qla4xxx: fixed NULL pointer dereference in eh_device_reset
Subsystems (2)
PATCH 6/12 qla4xxx: fixed NULL pointer dereference in eh_device_reset
From: Vikas Chaudhary <[email protected]>
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.
--- drivers/scsi/qla4xxx/ql4_os.c | 55 +++++++++++++++++++++++++++++++--------- 1 files changed, 42 insertions(+), 13 deletions(-)
drivers/scsi/qla4xxx/ql4_os.c +42 -13
@@ -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; -dev_info (&ha->pdev->dev, -"scsi%ld:%d:%d:%d: DEVICE RESET ISSUED.\n" , ha->host_no, - cmd->device->channel, cmd->device->id, cmd->device->lun); +if (!ddb_entry) { +DEBUG2 (printk ("scsi%ld: DEVICE RESET - NULL ddb entry.\n" + , ha->host_no)); +return FAILED ; + } + +dev_info (&ha->pdev->dev,"scsi%ld:%d:%d:%d: DEVICE RESET ISSUED.\n" + , ha->host_no, b, t, l); DEBUG2(printk(KERN_INFO "scsi%ld: DEVICE_RESET cmd=%p jiffies = 0x%lx, to=%x,"
@@ -1604,8 +1622,13 @@ static int qla4xxx_eh_device_reset(struct scsi_cmnd *cmd) cmd, jiffies, cmd->request->timeout /HZ , ha->dpc_flags, cmd->result, cmd->allowed)); -/* FIXME: wait for hba to go online */ - stat =qla4xxx_reset_lun (ha, ddb_entry, cmd->device->lun); +/* wait for hba to go online */ +if (qla4xxx_wait_for_hba_online (ha) !=QLA_SUCCESS ) { +dev_info (&ha->pdev->dev,"%s: DEVICE RESET." +"Adapter Offline.\n" , __func__); +return FAILED ; + } + stat =qla4xxx_reset_lun (ha, ddb_entry, l);if (stat !=QLA_SUCCESS ) {dev_info (&ha->pdev->dev,"DEVICE RESET FAILED. %d\n" , stat); goto eh_dev_reset_done;
@@ -1620,14 +1643,13 @@ static int qla4xxx_eh_device_reset(struct scsi_cmnd *cmd) }/* Send marker. */ -if (qla4xxx_send_marker_iocb (ha, ddb_entry, cmd->device->lun, -MM_LUN_RESET ) !=QLA_SUCCESS ) +if (qla4xxx_send_marker_iocb (ha, ddb_entry, l,MM_LUN_RESET ) + !=QLA_SUCCESS ) goto eh_dev_reset_done;dev_info (&ha->pdev->dev,"scsi(%ld:%d:%d:%d): DEVICE RESET SUCCEEDED.\n" , - ha->host_no, cmd->device->channel, cmd->device->id, - cmd->device->lun); + ha->host_no, b, t, l); ret =SUCCESS ;
@@ -1701,6 +1723,13 @@ static int qla4xxx_eh_host_reset(struct scsi_cmnd *cmd)int return_status =FAILED ;struct scsi_qla_host *ha; +if (cmd ==NULL ) { +DEBUG2 (printk (KERN_INFO "%s: **** SCSI mid-layer passing in" +" NULL cmd HOST RESET - cmd already" +" completed.\n" , __func__)); +return SUCCESS ; + } + ha = (struct scsi_qla_host *) cmd->device->host->hostdata; dev_info(&ha->pdev->dev,
Signature · 3 lines
1.6.0.2 ----- End forwarded message -----
Quoted 24 lines · Ravi Anand
From: Vikas Chaudhary<[email protected]>
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.drivers/scsi/qla4xxx/ql4_os.c +10
-2@@ -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 ;
drivers/scsi/qla4xxx/ql4_os.c static int qla4xxx_eh_wait_for_commands(struct scsi_qla_host *ha, · line 1608
Quoted 10 lines
drivers/scsi/qla4xxx/ql4_os.c +7
-2+ } -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?
Quoted 28 lines · Mike Christie
Quoted 27 lines · Ravi Anand
From: Vikas Chaudhary<[email protected]>
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.drivers/scsi/qla4xxx/ql4_os.c +11
-2@@ -1587,16 +1587,34 @@ static int qla4xxx_eh_wait_for_commands(structscsi_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 ; + }
Quoted 11 lines
drivers/scsi/qla4xxx/ql4_os.c static int qla4xxx_eh_wait_for_commands(struct scsi_qla_host *ha, · line 1608
Quoted 8 lines
drivers/scsi/qla4xxx/ql4_os.c +6
-2-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.