Re: [PATCH 06/12] qla4xxx: fixed NULL pointer dereference in eh_device_reset

[Mike Christie <michaelc@cs.wisc.edu>]

On 04/06/2010 10:54 PM, Mike Christie wrote:
> On 04/06/2010 05:14 AM, Ravi Anand wrote:
>> From: Vikas Chaudhary<vikas.chaudhary@qlogic.com>
>>
>> Created variables to reference h, b, t, l, because if
>> scsi passthru command completes within eh_device_reset,
>> the cmd structure may no longer be valid.
>
>
>> @@ -1587,16 +1587,34 @@ static int qla4xxx_eh_wait_for_commands(struct
>> scsi_qla_host *ha,
>> **/
>> static int qla4xxx_eh_device_reset(struct scsi_cmnd *cmd)
>> {
>> - struct scsi_qla_host *ha = to_qla_host(cmd->device->host);
>> - struct ddb_entry *ddb_entry = cmd->device->hostdata;
>> + struct scsi_qla_host *ha;
>> + struct ddb_entry *ddb_entry;
>> int ret = FAILED, stat;
>> + struct Scsi_Host *h;
>> + unsigned int b, t, l;
>> +
>> + if (cmd == NULL) {
>> + DEBUG2(printk(KERN_INFO "%s: **** SCSI mid-layer passing in"
>> + " NULL cmd DEVICE RESET - cmd already"
>> + " completed.\n", __func__));
>> + return SUCCESS;
>> + }
>>
>> - if (!ddb_entry)
>> - return ret;
>> + h = cmd->device->host;
>> + b = cmd->device->channel;
>> + t = cmd->device->id;
>> + l = cmd->device->lun;
>> + ha = to_qla_host(h);
>> + ddb_entry = cmd->device->hostdata;
>
>
> Could it complete normally while you are accessing the cmd above still?

Actually, I do not think it matters. For pass through did you mean 
scsi_reset_provider? If so the scmd passed to you for the 
scsi_reset_provider code path is allocated by scsi_reset_provider, so it 
is fine. For the normal code path (the scsi_unjam_host path), the scsi 
completion code is supposed to prevent the scsi cmd from getting 
completed when that code is running. If it is not then I think we are 
going to have other problems.