Skip to content

Commit

Permalink
ipr: Fix invalid array indexing for HRRQ
Browse files Browse the repository at this point in the history
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.

Cc: <[email protected]>
Tested-by: Wen Xiong <[email protected]>
Reviewed-by: Wen Xiong <[email protected]>
Reviewed-by: Gabriel Krisman Bertazi <[email protected]>
Signed-off-by: Brian King <[email protected]>
Reviewed-by: Martin K. Petersen <[email protected]>
Signed-off-by: James Bottomley <[email protected]>
  • Loading branch information
bjking1 authored and James Bottomley committed Jul 30, 2015
1 parent bb7c543 commit 3f1c058
Showing 1 changed file with 8 additions and 3 deletions.
11 changes: 8 additions & 3 deletions drivers/scsi/ipr.c
Original file line number Diff line number Diff line change
Expand Up @@ -1052,10 +1052,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;
}

/**
Expand Down

0 comments on commit 3f1c058

Please sign in to comment.