sgreset is issued with a scsi command pointer. The device reset code assumes that it was issued on a hardware queue, and calls block multiqueue layer. However, the assumption is broken, and there is no hardware queue associated with the sgreset, and this leads to a crash due to a null pointer exception. Fix the code to use the max_tag_id as a tag which does not overlap with the other tags issued by mid layer. Tested by running FC traffic for a few minutes, and by issuing sgreset on the device in parallel. Without the fix, the crash is observed right away. With this fix, no crash is observed. Reviewed-by: Sesidhar Baddela <sebaddel@xxxxxxxxx> Tested-by: Karan Tilak Kumar <kartilak@xxxxxxxxx> Signed-off-by: Karan Tilak Kumar <kartilak@xxxxxxxxx> --- drivers/scsi/fnic/fnic.h | 3 ++- drivers/scsi/fnic/fnic_scsi.c | 20 +++++++++----------- 2 files changed, 11 insertions(+), 12 deletions(-) diff --git a/drivers/scsi/fnic/fnic.h b/drivers/scsi/fnic/fnic.h index e51e92f932fa..93c68931a593 100644 --- a/drivers/scsi/fnic/fnic.h +++ b/drivers/scsi/fnic/fnic.h @@ -27,7 +27,7 @@ #define DRV_NAME "fnic" #define DRV_DESCRIPTION "Cisco FCoE HBA Driver" -#define DRV_VERSION "1.6.0.55" +#define DRV_VERSION "1.6.0.56" #define PFX DRV_NAME ": " #define DFX DRV_NAME "%d: " @@ -236,6 +236,7 @@ struct fnic { unsigned int wq_count; unsigned int cq_count; + struct mutex sgreset_mutex; struct dentry *fnic_stats_debugfs_host; struct dentry *fnic_stats_debugfs_file; struct dentry *fnic_reset_debugfs_file; diff --git a/drivers/scsi/fnic/fnic_scsi.c b/drivers/scsi/fnic/fnic_scsi.c index be89ce96df46..185142efee3d 100644 --- a/drivers/scsi/fnic/fnic_scsi.c +++ b/drivers/scsi/fnic/fnic_scsi.c @@ -2222,7 +2222,6 @@ int fnic_device_reset(struct scsi_cmnd *sc) struct reset_stats *reset_stats; int tag = rq->tag; DECLARE_COMPLETION_ONSTACK(tm_done); - int tag_gen_flag = 0; /*to track tags allocated by fnic driver*/ bool new_sc = 0; /* Wait for rport to unblock */ @@ -2252,17 +2251,17 @@ int fnic_device_reset(struct scsi_cmnd *sc) } fnic_priv(sc)->flags = FNIC_DEVICE_RESET; - /* Allocate tag if not present */ if (unlikely(tag < 0)) { /* - * Really should fix the midlayer to pass in a proper - * request for ioctls... + * For device reset issued through sg3utils, we let + * only one LUN_RESET to go through and use a special + * tag equal to max_tag_id so that we don't have to allocate + * or free it. It won't interact with tags + * allocated by mid layer. */ - tag = fnic_scsi_host_start_tag(fnic, sc); - if (unlikely(tag == SCSI_NO_TAG)) - goto fnic_device_reset_end; - tag_gen_flag = 1; + mutex_lock(&fnic->sgreset_mutex); + tag = fnic->fnic_max_tag_id; new_sc = 1; } io_lock = fnic_io_lock_hash(fnic, sc); @@ -2434,9 +2433,8 @@ int fnic_device_reset(struct scsi_cmnd *sc) (u64)sc->cmnd[4] << 8 | sc->cmnd[5]), fnic_flags_and_state(sc)); - /* free tag if it is allocated */ - if (unlikely(tag_gen_flag)) - fnic_scsi_host_end_tag(fnic, sc); + if (new_sc) + mutex_unlock(&fnic->sgreset_mutex); FNIC_SCSI_DBG(KERN_DEBUG, fnic->lport->host, "Returning from device reset %s\n", -- 2.31.1