Skip to content

Commit

Permalink
scsi: aacraid: Fix command send race condition
Browse files Browse the repository at this point in the history 
This fixes a potential race condition observed on Power systems.

Several places throughout the aacraid driver call aac_fib_send or
similar to send a command to the aacraid adapter, then check the return
code to determine if the command was actually sent to the adapter, then
update the phase field in the scsi command scratch pad area to track
that the firmware now owns this command.  However, there is nothing that
ensures that by the time the aac_fib_send function returns and we go to
write to the scsi command, that the command hasn't already completed and
the scsi command has been freed.  This was causing random crashes in the
TCP stack which was tracked down to be caused by memory that had been a
struct request + scsi_cmnd being now used for an skbuff. Memory
poisoning was enabled in the kernel to debug this which showed that the
last owner of the memory that had been freed was aacraid and that it was
a struct request.  The memory that was corrupted was the exact data
pattern of AAC_OWNER_FIRMWARE and it was at the same offset that aacraid
writes, which is scsicmd->SCp.phase. The patch below resolves this
issue.

Cc: <stable@vger.kernel.org>
Signed-off-by: Brian King <brking@linux.vnet.ibm.com>
Tested-by: Wen Xiong <wenxiong@linux.vnet.ibm.com>
Reviewed-by: Dave Carroll <david.carroll@microsemi.com>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
  • Loading branch information
Brian King authored and Martin K. Petersen committed Aug 30, 2017
1 parent fa2d9d6 commit 1ae948f
Showing 1 changed file with 21 additions and 33 deletions.
54 changes: 21 additions & 33 deletions drivers/scsi/aacraid/aachba.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -594,6 +594,7 @@ static int aac_get_container_name(struct scsi_cmnd * scsicmd)

aac_fib_init(cmd_fibcontext);
dinfo = (struct aac_get_name *) fib_data(cmd_fibcontext);
scsicmd->SCp.phase = AAC_OWNER_FIRMWARE;

dinfo->command = cpu_to_le32(VM_ContainerConfig);
dinfo->type = cpu_to_le32(CT_READ_NAME);
Expand All @@ -611,10 +612,8 @@ static int aac_get_container_name(struct scsi_cmnd * scsicmd)
/*
* Check that the command queued to the controller
*/
if (status == -EINPROGRESS) {
scsicmd->SCp.phase = AAC_OWNER_FIRMWARE;
if (status == -EINPROGRESS)
return 0;
}

printk(KERN_WARNING "aac_get_container_name: aac_fib_send failed with status: %d.\n", status);
aac_fib_complete(cmd_fibcontext);
Expand Down Expand Up @@ -725,6 +724,7 @@ static void _aac_probe_container1(void * context, struct fib * fibptr)

dinfo->count = cpu_to_le32(scmd_id(scsicmd));
dinfo->type = cpu_to_le32(FT_FILESYS);
scsicmd->SCp.phase = AAC_OWNER_FIRMWARE;

status = aac_fib_send(ContainerCommand,
fibptr,
Expand All @@ -736,9 +736,7 @@ static void _aac_probe_container1(void * context, struct fib * fibptr)
/*
* Check that the command queued to the controller
*/
if (status == -EINPROGRESS)
scsicmd->SCp.phase = AAC_OWNER_FIRMWARE;
else if (status < 0) {
if (status < 0 && status != -EINPROGRESS) {
/* Inherit results from VM_NameServe, if any */
dresp->status = cpu_to_le32(ST_OK);
_aac_probe_container2(context, fibptr);
Expand Down Expand Up @@ -766,6 +764,7 @@ static int _aac_probe_container(struct scsi_cmnd * scsicmd, int (*callback)(stru
dinfo->count = cpu_to_le32(scmd_id(scsicmd));
dinfo->type = cpu_to_le32(FT_FILESYS);
scsicmd->SCp.ptr = (char *)callback;
scsicmd->SCp.phase = AAC_OWNER_FIRMWARE;

status = aac_fib_send(ContainerCommand,
fibptr,
Expand All @@ -777,10 +776,9 @@ static int _aac_probe_container(struct scsi_cmnd * scsicmd, int (*callback)(stru
/*
* Check that the command queued to the controller
*/
if (status == -EINPROGRESS) {
scsicmd->SCp.phase = AAC_OWNER_FIRMWARE;
if (status == -EINPROGRESS)
return 0;
}

if (status < 0) {
scsicmd->SCp.ptr = NULL;
aac_fib_complete(fibptr);
Expand Down Expand Up @@ -1126,6 +1124,7 @@ static int aac_get_container_serial(struct scsi_cmnd * scsicmd)
dinfo->command = cpu_to_le32(VM_ContainerConfig);
dinfo->type = cpu_to_le32(CT_CID_TO_32BITS_UID);
dinfo->cid = cpu_to_le32(scmd_id(scsicmd));
scsicmd->SCp.phase = AAC_OWNER_FIRMWARE;

status = aac_fib_send(ContainerCommand,
cmd_fibcontext,
Expand All @@ -1138,10 +1137,8 @@ static int aac_get_container_serial(struct scsi_cmnd * scsicmd)
/*
* Check that the command queued to the controller
*/
if (status == -EINPROGRESS) {
scsicmd->SCp.phase = AAC_OWNER_FIRMWARE;
if (status == -EINPROGRESS)
return 0;
}

printk(KERN_WARNING "aac_get_container_serial: aac_fib_send failed with status: %d.\n", status);
aac_fib_complete(cmd_fibcontext);
Expand Down Expand Up @@ -2335,16 +2332,14 @@ static int aac_read(struct scsi_cmnd * scsicmd)
* Alocate and initialize a Fib
*/
cmd_fibcontext = aac_fib_alloc_tag(dev, scsicmd);

scsicmd->SCp.phase = AAC_OWNER_FIRMWARE;
status = aac_adapter_read(cmd_fibcontext, scsicmd, lba, count);

/*
* Check that the command queued to the controller
*/
if (status == -EINPROGRESS) {
scsicmd->SCp.phase = AAC_OWNER_FIRMWARE;
if (status == -EINPROGRESS)
return 0;
}

printk(KERN_WARNING "aac_read: aac_fib_send failed with status: %d.\n", status);
/*
Expand Down Expand Up @@ -2429,16 +2424,14 @@ static int aac_write(struct scsi_cmnd * scsicmd)
* Allocate and initialize a Fib then setup a BlockWrite command
*/
cmd_fibcontext = aac_fib_alloc_tag(dev, scsicmd);

scsicmd->SCp.phase = AAC_OWNER_FIRMWARE;
status = aac_adapter_write(cmd_fibcontext, scsicmd, lba, count, fua);

/*
* Check that the command queued to the controller
*/
if (status == -EINPROGRESS) {
scsicmd->SCp.phase = AAC_OWNER_FIRMWARE;
if (status == -EINPROGRESS)
return 0;
}

printk(KERN_WARNING "aac_write: aac_fib_send failed with status: %d\n", status);
/*
Expand Down Expand Up @@ -2588,6 +2581,7 @@ static int aac_synchronize(struct scsi_cmnd *scsicmd)
synchronizecmd->cid = cpu_to_le32(scmd_id(scsicmd));
synchronizecmd->count =
cpu_to_le32(sizeof(((struct aac_synchronize_reply *)NULL)->data));
scsicmd->SCp.phase = AAC_OWNER_FIRMWARE;

/*
* Now send the Fib to the adapter
Expand All @@ -2603,10 +2597,8 @@ static int aac_synchronize(struct scsi_cmnd *scsicmd)
/*
* Check that the command queued to the controller
*/
if (status == -EINPROGRESS) {
scsicmd->SCp.phase = AAC_OWNER_FIRMWARE;
if (status == -EINPROGRESS)
return 0;
}

printk(KERN_WARNING
"aac_synchronize: aac_fib_send failed with status: %d.\n", status);
Expand Down Expand Up @@ -2666,6 +2658,7 @@ static int aac_start_stop(struct scsi_cmnd *scsicmd)
pmcmd->cid = cpu_to_le32(sdev_id(sdev));
pmcmd->parm = (scsicmd->cmnd[1] & 1) ?
cpu_to_le32(CT_PM_UNIT_IMMEDIATE) : 0;
scsicmd->SCp.phase = AAC_OWNER_FIRMWARE;

/*
* Now send the Fib to the adapter
Expand All @@ -2681,10 +2674,8 @@ static int aac_start_stop(struct scsi_cmnd *scsicmd)
/*
* Check that the command queued to the controller
*/
if (status == -EINPROGRESS) {
scsicmd->SCp.phase = AAC_OWNER_FIRMWARE;
if (status == -EINPROGRESS)
return 0;
}

aac_fib_complete(cmd_fibcontext);
aac_fib_free(cmd_fibcontext);
Expand Down Expand Up @@ -3692,16 +3683,14 @@ static int aac_send_srb_fib(struct scsi_cmnd* scsicmd)
* Allocate and initialize a Fib then setup a BlockWrite command
*/
cmd_fibcontext = aac_fib_alloc_tag(dev, scsicmd);

scsicmd->SCp.phase = AAC_OWNER_FIRMWARE;
status = aac_adapter_scsi(cmd_fibcontext, scsicmd);

/*
* Check that the command queued to the controller
*/
if (status == -EINPROGRESS) {
scsicmd->SCp.phase = AAC_OWNER_FIRMWARE;
if (status == -EINPROGRESS)
return 0;
}

printk(KERN_WARNING "aac_srb: aac_fib_send failed with status: %d\n", status);
aac_fib_complete(cmd_fibcontext);
Expand Down Expand Up @@ -3739,15 +3728,14 @@ static int aac_send_hba_fib(struct scsi_cmnd *scsicmd)
if (!cmd_fibcontext)
return -1;

scsicmd->SCp.phase = AAC_OWNER_FIRMWARE;
status = aac_adapter_hba(cmd_fibcontext, scsicmd);

/*
* Check that the command queued to the controller
*/
if (status == -EINPROGRESS) {
scsicmd->SCp.phase = AAC_OWNER_FIRMWARE;
if (status == -EINPROGRESS)
return 0;
}

pr_warn("aac_hba_cmd_req: aac_fib_send failed with status: %d\n",
status);
Expand Down

0 comments on commit 1ae948f

Please sign in to comment.