scsi: fnic: Replace sgreset tag with max_tag_id

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@cisco.com>
Tested-by: Karan Tilak Kumar <kartilak@cisco.com>
Signed-off-by: Karan Tilak Kumar <kartilak@cisco.com>
Link: https://lore.kernel.org/r/20230817182146.229059-1-kartilak@cisco.com
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>

diff --git a/drivers/scsi/fnic/fnic.h b/drivers/scsi/fnic/fnic.h
index d82de34..93c6893 100644 (file)
--- 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.54"
+#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 26dbd34..8ce3e3c 100644 (file)
--- a/drivers/scsi/fnic/fnic_scsi.c
+++ b/drivers/scsi/fnic/fnic_scsi.c
@@ -2220,7 +2220,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 */
@@ -2250,17 +2249,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);
@@ -2432,9 +2431,8 @@ fnic_device_reset_end:
                  (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",