[3.13.y-ckt stable] Patch "ipr: Fix invalid array indexing for HRRQ" has been added to staging queue

Kamal Mostafa kamal at canonical.com
Wed Sep 2 00:55:50 UTC 2015


This is a note to let you know that I have just added a patch titled

    ipr: Fix invalid array indexing for HRRQ

to the linux-3.13.y-queue branch of the 3.13.y-ckt extended stable tree 
which can be found at:

    http://kernel.ubuntu.com/git/ubuntu/linux.git/log/?h=linux-3.13.y-queue

This patch is scheduled to be released in version 3.13.11-ckt26.

If you, or anyone else, feels it should not be added to this tree, please 
reply to this email.

For more information about the 3.13.y-ckt tree, see
https://wiki.ubuntu.com/Kernel/Dev/ExtendedStable

Thanks.
-Kamal

------

>From fb39899003927552c2f13e1034434bd564089f29 Mon Sep 17 00:00:00 2001
From: Brian King <brking at linux.vnet.ibm.com>
Date: Tue, 14 Jul 2015 11:41:33 -0500
Subject: ipr: Fix invalid array indexing for HRRQ

commit 3f1c0581310d5d94bd72740231507e763a6252a4 upstream.

Fixes another signed / unsigned array indexing bug in the ipr driver.
Currently, when hrrq_index wraps, it becomes a negative number. We
do the modulo, but still have a negative number, so we end up indexing
backwards in the array. Given where the hrrq array is located in memory,
we probably won't actually reference memory we don't own, but nonetheless
ipr is still looking at data within struct ipr_ioa_cfg and interpreting it as
struct ipr_hrr_queue data, so bad things could certainly happen.

Each ipr adapter has anywhere from 1 to 16 HRRQs. By default, we use 2 on new
adapters.  Let's take an example:

Assume ioa_cfg->hrrq_index=0x7fffffffe and ioa_cfg->hrrq_num=4:

The atomic_add_return will then return -1. We mod this with 3 and get -2, add
one and get -1 for an array index.

On adapters which support more than a single HRRQ, we dedicate HRRQ to adapter
initialization and error interrupts so that we can optimize the other queues
for fast path I/O. So all normal I/O uses HRRQ 1-15. So we want to spread the
I/O requests across those HRRQs.

With the default module parameter settings, this bug won't hit, only when
someone sets the ipr.number_of_msix parameter to a value larger than 3 is when
bad things start to happen.

Tested-by: Wen Xiong <wenxiong at linux.vnet.ibm.com>
Reviewed-by: Wen Xiong <wenxiong at linux.vnet.ibm.com>
Reviewed-by: Gabriel Krisman Bertazi <krisman at linux.vnet.ibm.com>
Signed-off-by: Brian King <brking at linux.vnet.ibm.com>
Reviewed-by: Martin K. Petersen <martin.petersen at oracle.com>
Signed-off-by: James Bottomley <JBottomley at Odin.com>
Signed-off-by: Kamal Mostafa <kamal at canonical.com>
---
 drivers/scsi/ipr.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/drivers/scsi/ipr.c b/drivers/scsi/ipr.c
index b0f5836..430583c 100644
--- a/drivers/scsi/ipr.c
+++ b/drivers/scsi/ipr.c
@@ -1045,10 +1045,15 @@ static void ipr_send_blocking_cmd(struct ipr_cmnd *ipr_cmd,

 static int ipr_get_hrrq_index(struct ipr_ioa_cfg *ioa_cfg)
 {
+	unsigned int hrrq;
+
 	if (ioa_cfg->hrrq_num == 1)
-		return 0;
-	else
-		return (atomic_add_return(1, &ioa_cfg->hrrq_index) % (ioa_cfg->hrrq_num - 1)) + 1;
+		hrrq = 0;
+	else {
+		hrrq = atomic_add_return(1, &ioa_cfg->hrrq_index);
+		hrrq = (hrrq % (ioa_cfg->hrrq_num - 1)) + 1;
+	}
+	return hrrq;
 }

 /**
--
1.9.1





More information about the kernel-team mailing list