| From: Brian King <brking@linux.vnet.ibm.com> |
| Date: Tue, 29 Aug 2017 10:00:29 -0500 |
| Subject: scsi: aacraid: Fix command send race condition |
| |
| commit 1ae948fa4f00f3a2823e7cb19a3049ef27dd6947 upstream. |
| |
| 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. |
| |
| 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> |
| [bwh: Backported to 3.16: |
| - Drop changes to aac_send_hba_fib() |
| - Adjust context] |
| Signed-off-by: Ben Hutchings <ben@decadent.org.uk> |
| --- |
| --- a/drivers/scsi/aacraid/aachba.c |
| +++ b/drivers/scsi/aacraid/aachba.c |
| @@ -480,6 +480,7 @@ static int aac_get_container_name(struct |
| |
| 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); |
| @@ -497,10 +498,8 @@ static int aac_get_container_name(struct |
| /* |
| * 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); |
| @@ -589,6 +588,7 @@ static void _aac_probe_container1(void * |
| dinfo->command = cpu_to_le32(VM_NameServe64); |
| 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, |
| @@ -600,9 +600,7 @@ static void _aac_probe_container1(void * |
| /* |
| * 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); |
| @@ -625,6 +623,7 @@ static int _aac_probe_container(struct s |
| 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, |
| @@ -636,10 +635,9 @@ static int _aac_probe_container(struct s |
| /* |
| * 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); |
| @@ -873,6 +871,7 @@ static int aac_get_container_serial(stru |
| 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, |
| @@ -885,10 +884,8 @@ static int aac_get_container_serial(stru |
| /* |
| * 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); |
| @@ -1774,16 +1771,14 @@ static int aac_read(struct scsi_cmnd * s |
| printk(KERN_WARNING "aac_read: fib allocation failed\n"); |
| return -1; |
| } |
| - |
| + 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); |
| /* |
| @@ -1877,16 +1872,14 @@ static int aac_write(struct scsi_cmnd * |
| printk(KERN_WARNING "aac_write: fib allocation failed\n"); |
| return -1; |
| } |
| - |
| + 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); |
| /* |
| @@ -2036,6 +2029,7 @@ static int aac_synchronize(struct scsi_c |
| 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 |
| @@ -2051,10 +2045,8 @@ static int aac_synchronize(struct scsi_c |
| /* |
| * 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); |
| @@ -2116,6 +2108,7 @@ static int aac_start_stop(struct scsi_cm |
| 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 |
| @@ -2131,10 +2124,8 @@ static int aac_start_stop(struct scsi_cm |
| /* |
| * 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); |
| @@ -2889,15 +2880,14 @@ static int aac_send_srb_fib(struct scsi_ |
| if (!(cmd_fibcontext = aac_fib_alloc(dev))) { |
| return -1; |
| } |
| + 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); |