>-----Original Message----- >From: linux-scsi-owner@xxxxxxxxxxxxxxx [mailto:linux-scsi- >owner@xxxxxxxxxxxxxxx] On Behalf Of Hannes Reinecke >Sent: Wednesday, June 28, 2017 2:03 PM >To: Christoph Hellwig >Cc: Martin K. Petersen; James Bottomley; linux-scsi@xxxxxxxxxxxxxxx; Hannes >Reinecke; Hannes Reinecke >Subject: [PATCH 13/47] megaraid: pass in NULL scb for host reset > >When calling a host reset we shouldn't rely on the command triggering the >reset, so allow megaraid_abort_and_reset() to be called with a NULL scb. >And drop the pointless 'bus_reset' and 'target_reset' handlers, which just call >the same function as host_reset. If this patch address any functional issue, then we should consider this. If it's code optimization, can we ignore this as this is being very old driver and no more maintained by Broadcom/LSI ? > >Signed-off-by: Hannes Reinecke <hare@xxxxxxxx> >--- > drivers/scsi/megaraid.c | 42 ++++++++++++++++-------------------------- > 1 file changed, 16 insertions(+), 26 deletions(-) > >diff --git a/drivers/scsi/megaraid.c b/drivers/scsi/megaraid.c index >3c63c29..7e504d3 100644 >--- a/drivers/scsi/megaraid.c >+++ b/drivers/scsi/megaraid.c >@@ -1909,7 +1909,7 @@ static DEF_SCSI_QCMD(megaraid_queue) > > spin_lock_irq(&adapter->lock); > >- rval = megaraid_abort_and_reset(adapter, cmd, SCB_RESET); >+ rval = megaraid_abort_and_reset(adapter, NULL, SCB_RESET); If cmd=NULL is passed, it will crash inside function megaraid_abort_and_reset() while dereferencing "cmd" pointer. Below is the code of function megaraid_abort_and_reset() where it will crash- static int megaraid_abort_and_reset(adapter_t *adapter, Scsi_Cmnd *cmd, int aor) { struct list_head *pos, *next; scb_t *scb; dev_warn(&adapter->dev->dev, "%s cmd=%x <c=%d t=%d l=%d>\n", (aor == SCB_ABORT)? "ABORTING":"RESET", cmd->cmnd[0], cmd->device->channel,>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>it should crash here cmd->device->id, (u32)cmd->device->lun); Please correct if I am missing something here. > > /* > * This is required here to complete any completed requests @@ - >1948,7 +1948,7 @@ static DEF_SCSI_QCMD(megaraid_queue) > > scb = list_entry(pos, scb_t, list); > >- if (scb->cmd == cmd) { /* Found command */ >+ if (!cmd || scb->cmd == cmd) { /* Found command */ > > scb->state |= aor; > >@@ -1967,31 +1967,23 @@ static DEF_SCSI_QCMD(megaraid_queue) > > return FAILED; > } >- else { >- >- /* >- * Not yet issued! Remove from the pending >- * list >- */ >- dev_warn(&adapter->dev->dev, >- "%s-[%x], driver owner\n", >- (aor==SCB_ABORT) ? >"ABORTING":"RESET", >- scb->idx); >- >- mega_free_scb(adapter, scb); >- >- if( aor == SCB_ABORT ) { >- cmd->result = (DID_ABORT << 16); >- } >- else { >- cmd->result = (DID_RESET << 16); >- } >+ /* >+ * Not yet issued! Remove from the pending >+ * list >+ */ >+ dev_warn(&adapter->dev->dev, >+ "%s-[%x], driver owner\n", >+ (cmd) ? "ABORTING":"RESET", >+ scb->idx); >+ mega_free_scb(adapter, scb); > >+ if (cmd) { >+ cmd->result = (DID_ABORT << 16); > list_add_tail(SCSI_LIST(cmd), >- &adapter->completed_list); >- >- return SUCCESS; >+ &adapter->completed_list); > } >+ >+ return SUCCESS; > } > } > >@@ -4180,8 +4172,6 @@ static inline void mega_create_proc_entry(int index, >struct proc_dir_entry *pare > .cmd_per_lun = DEF_CMD_PER_LUN, > .use_clustering = ENABLE_CLUSTERING, > .eh_abort_handler = megaraid_abort, >- .eh_device_reset_handler = megaraid_reset, >- .eh_bus_reset_handler = megaraid_reset, > .eh_host_reset_handler = megaraid_reset, > .no_write_same = 1, > }; >-- >1.8.5.6
