On Thu, 2018-04-05 at 11:46 -0400, Chaitra P B wrote: > Check scsi tracker 'st' for NULL and st->smid for zero (as driver uses smid > starting from one) before accessing it. > These checks are added as there are possibilities for getting valid > scsi_cmd when driver calls scsi_host_find_tag() API when it loops using > smid(i.e tag) from one to hba queue depth but still scsi tracker st for > this corresponding scsi_cmd is not yet initialized. > > For example below are such scenario: > Sometimes it is possible that scsi_cmd might have created at SML but it > might not be issued to the driver (or driver might have returned the > command with Host busy status) as the host reset operation / TMs is in > progress.In such case where the scsi_cmd is not yet processed by driver > then the scsi tracker 'st' of that scsi_cmd & the fields of this 'st' will > be uninitialized. > And hence this patch add checks for 'st' in IOCTL path for TMs issued from > applications and also in host reset path where driver flushes all the > outstanding commands as part of host reset operation.
What is needed is an explanation about which mechanism serializes the execution of scsih_qcmd() and mpt3sas_base_hard_reset_handler(), at least if such a mechanism exists. The above text does not mention anything about such a synchronization mechanism. > scmd = mpt3sas_scsih_scsi_lookup_get(ioc, smid); > - if (!scmd) > + if (scmd == NULL || scmd->device == NULL || > + scmd->device->hostdata == NULL) As Christoph explained before, scmd->device is never NULL. Additionally, the scmd->device->hostdata check looks very suspicious. That check should either be left out or the race that causes a SCSI device to be removed concurrently with this function should be fixed. If you are unable to motivate why this patch is correct, please repost this series without this patch. Thanks, Bart.
