[PATCH v6 5/6] scsi: core: Protect host state changes with the host lock

Some but not all SCSI host state changes are protected with the SCSI
host lock. Annotate the SCSI host state with __guarded_by(host_lock)
and protect all SCSI host state changes with the SCSI host lock. This
patch prevents that KCSAN complains about data races when accessing the
SCSI host state.

Reported-by: Jianzhou Zhao <[email protected]>
Closes: https://lore.kernel.org/all/[email protected]/
Signed-off-by: Bart Van Assche <[email protected]>
---
 drivers/scsi/hosts.c                      | 19 ++++++++++--------
 drivers/scsi/leapraid/leapraid_func.c     |  2 +-
 drivers/scsi/leapraid/leapraid_os.c       |  2 +-
 drivers/scsi/megaraid/megaraid_sas_base.c |  2 +-
 drivers/scsi/mpi3mr/mpi3mr_os.c           |  2 +-
 drivers/scsi/mpt3sas/mpt3sas_scsih.c      |  2 +-
 drivers/scsi/qla4xxx/ql4_os.c             |  6 ++----
 drivers/scsi/scsi_lib.c                   |  3 +--
 drivers/scsi/scsi_sysfs.c                 |  7 ++++---
 include/scsi/scsi_host.h                  | 24 ++++++++++++++++-------
 10 files changed, 40 insertions(+), 29 deletions(-)

diff --git a/drivers/scsi/hosts.c b/drivers/scsi/hosts.c
index d512080268af..9610dd7aa85f 100644
--- a/drivers/scsi/hosts.c
+++ b/drivers/scsi/hosts.c
@@ -73,8 +73,9 @@ static struct class shost_class = {
  *	transition is illegal.
  **/
 int scsi_host_set_state(struct Scsi_Host *shost, enum scsi_host_state state)
+	__must_hold(shost->host_lock)
 {
-	enum scsi_host_state oldstate = shost->shost_state;
+	enum scsi_host_state oldstate = READ_ONCE(shost->shost_state);
 
 	if (state == oldstate)
 		return 0;
@@ -145,7 +146,7 @@ int scsi_host_set_state(struct Scsi_Host *shost, enum scsi_host_state state)
 		}
 		break;
 	}
-	shost->shost_state = state;
+	WRITE_ONCE(shost->shost_state, state);
 	return 0;
 
  illegal:
@@ -276,7 +277,8 @@ int scsi_add_host_with_dma(struct Scsi_Host *shost, struct device *dev,
 	if (error)
 		goto out_disable_runtime_pm;
 
-	scsi_host_set_state(shost, SHOST_RUNNING);
+	scoped_guard(spinlock_irq, shost->host_lock)
+		scsi_host_set_state(shost, SHOST_RUNNING);
 	get_device(shost->shost_gendev.parent);
 
 	device_enable_async_suspend(&shost->shost_dev);
@@ -350,6 +352,7 @@ EXPORT_SYMBOL(scsi_add_host_with_dma);
 static void scsi_host_dev_release(struct device *dev)
 {
 	struct Scsi_Host *shost = dev_to_shost(dev);
+	enum scsi_host_state state = scsi_get_host_state(shost);
 	struct device *parent = dev->parent;
 
 	/* Wait for functions invoked through call_rcu(&scmd->rcu, ...) */
@@ -362,7 +365,7 @@ static void scsi_host_dev_release(struct device *dev)
 	if (shost->work_q)
 		destroy_workqueue(shost->work_q);
 
-	if (shost->shost_state == SHOST_CREATED) {
+	if (state == SHOST_CREATED) {
 		/*
 		 * Free the shost_dev device name and remove the proc host dir
 		 * here if scsi_host_{alloc,put}() have been called but neither
@@ -378,7 +381,7 @@ static void scsi_host_dev_release(struct device *dev)
 
 	ida_free(&host_index_ida, shost->host_no);
 
-	if (shost->shost_state != SHOST_CREATED)
+	if (state != SHOST_CREATED)
 		put_device(parent);
 	kfree(shost);
 }
@@ -411,8 +414,8 @@ struct Scsi_Host *scsi_host_alloc(const struct scsi_host_template *sht, int priv
 		return NULL;
 
 	shost->host_lock = &shost->default_lock;
-	spin_lock_init(shost->host_lock);
-	shost->shost_state = SHOST_CREATED;
+	scoped_guard(spinlock_init, shost->host_lock)
+		shost->shost_state = SHOST_CREATED;
 	INIT_LIST_HEAD(&shost->__devices);
 	INIT_LIST_HEAD(&shost->__targets);
 	INIT_LIST_HEAD(&shost->eh_abort_list);
@@ -598,7 +601,7 @@ EXPORT_SYMBOL(scsi_host_lookup);
  **/
 struct Scsi_Host *scsi_host_get(struct Scsi_Host *shost)
 {
-	if ((shost->shost_state == SHOST_DEL) ||
+	if (scsi_get_host_state(shost) == SHOST_DEL ||
 		!get_device(&shost->shost_gendev))
 		return NULL;
 	return shost;
diff --git a/drivers/scsi/leapraid/leapraid_func.c b/drivers/scsi/leapraid/leapraid_func.c
index 3e1aeceab994..089d0810bd13 100644
--- a/drivers/scsi/leapraid/leapraid_func.c
+++ b/drivers/scsi/leapraid/leapraid_func.c
@@ -37,7 +37,7 @@ static noinline bool leapraid_shost_in_recovery(struct Scsi_Host *shost)
 {
 	enum scsi_host_state state;
 
-	state = READ_ONCE(shost->shost_state);
+	state = scsi_get_host_state(shost);
 	return state == SHOST_RECOVERY ||
 	       state == SHOST_CANCEL_RECOVERY ||
 	       state == SHOST_DEL_RECOVERY ||
diff --git a/drivers/scsi/leapraid/leapraid_os.c b/drivers/scsi/leapraid/leapraid_os.c
index 2f3d4ac7490c..a8e1c9f33896 100644
--- a/drivers/scsi/leapraid/leapraid_os.c
+++ b/drivers/scsi/leapraid/leapraid_os.c
@@ -808,7 +808,7 @@ static bool leapraid_should_queuecommand(struct leapraid_adapter *adapter,
 		goto no_connect;
 
 	if (sdev_priv->block &&
-	    scmd->device->host->shost_state == SHOST_RECOVERY &&
+	    scsi_get_host_state(scmd->device->host) == SHOST_RECOVERY &&
 	    scmd->cmnd[0] == TEST_UNIT_READY) {
 		scsi_build_sense(scmd, 0, UNIT_ATTENTION,
 				 LEAPRAID_SCSI_ASC_POWER_ON_RESET,
diff --git a/drivers/scsi/megaraid/megaraid_sas_base.c b/drivers/scsi/megaraid/megaraid_sas_base.c
index ecd365d78ae3..f0152b043e18 100644
--- a/drivers/scsi/megaraid/megaraid_sas_base.c
+++ b/drivers/scsi/megaraid/megaraid_sas_base.c
@@ -3072,7 +3072,7 @@ static int megasas_reset_bus_host(struct scsi_cmnd *scmd)
 
 	scmd_printk(KERN_INFO, scmd,
 		"SCSI host state: %d  SCSI host busy: %d  FW outstanding: %d\n",
-		scmd->device->host->shost_state,
+		scsi_get_host_state(scmd->device->host),
 		scsi_host_busy(scmd->device->host),
 		atomic_read(&instance->fw_outstanding));
 	/*
diff --git a/drivers/scsi/mpi3mr/mpi3mr_os.c b/drivers/scsi/mpi3mr/mpi3mr_os.c
index 402d1f35d214..f80a21ec161b 100644
--- a/drivers/scsi/mpi3mr/mpi3mr_os.c
+++ b/drivers/scsi/mpi3mr/mpi3mr_os.c
@@ -5172,7 +5172,7 @@ static enum scsi_qc_status mpi3mr_qcmd(struct Scsi_Host *shost,
 
 	/* Avoid error handling escalation when device is removed or blocked */
 
-	if (scmd->device->host->shost_state == SHOST_RECOVERY &&
+	if (scsi_get_host_state(scmd->device->host) == SHOST_RECOVERY &&
 		scmd->cmnd[0] == TEST_UNIT_READY &&
 		(stgt_priv_data->dev_removed || (dev_handle == MPI3MR_INVALID_DEV_HANDLE))) {
 		scsi_build_sense(scmd, 0, UNIT_ATTENTION, 0x29, 0x07);
diff --git a/drivers/scsi/mpt3sas/mpt3sas_scsih.c b/drivers/scsi/mpt3sas/mpt3sas_scsih.c
index dea78688cc9b..0e12009a87f6 100644
--- a/drivers/scsi/mpt3sas/mpt3sas_scsih.c
+++ b/drivers/scsi/mpt3sas/mpt3sas_scsih.c
@@ -5472,7 +5472,7 @@ static enum scsi_qc_status scsih_qcmd(struct Scsi_Host *shost,
 	 * Avoid error handling escallation when device is disconnected
 	 */
 	if (handle == MPT3SAS_INVALID_DEVICE_HANDLE || sas_device_priv_data->block) {
-		if (scmd->device->host->shost_state == SHOST_RECOVERY &&
+		if (scsi_get_host_state(scmd->device->host) == SHOST_RECOVERY &&
 		    scmd->cmnd[0] == TEST_UNIT_READY) {
 			scsi_build_sense(scmd, 0, UNIT_ATTENTION, 0x29, 0x07);
 			scsi_done(scmd);
diff --git a/drivers/scsi/qla4xxx/ql4_os.c b/drivers/scsi/qla4xxx/ql4_os.c
index d598ab4126f8..c9d9fc7c81fb 100644
--- a/drivers/scsi/qla4xxx/ql4_os.c
+++ b/drivers/scsi/qla4xxx/ql4_os.c
@@ -9411,11 +9411,9 @@ static int qla4xxx_eh_target_reset(struct scsi_cmnd *cmd)
  * This routine finds that if reset host is called in EH
  * scenario or from some application like sg_reset
  **/
-static int qla4xxx_is_eh_active(struct Scsi_Host *shost)
+static bool qla4xxx_is_eh_active(struct Scsi_Host *shost)
 {
-	if (shost->shost_state == SHOST_RECOVERY)
-		return 1;
-	return 0;
+	return scsi_get_host_state(shost) == SHOST_RECOVERY;
 }
 
 /**
diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index 686cef2406b3..2e07b686090f 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -1663,10 +1663,9 @@ static enum scsi_qc_status scsi_dispatch_cmd(struct scsi_cmnd *cmd)
 		goto done;
 	}
 
-	if (unlikely(host->shost_state == SHOST_DEL)) {
+	if (unlikely(scsi_get_host_state(host) == SHOST_DEL)) {
 		cmd->result = (DID_NO_CONNECT << 16);
 		goto done;
-
 	}
 
 	trace_scsi_dispatch_cmd_start(cmd);
diff --git a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c
index dfc3559e7e04..9480432f650b 100644
--- a/drivers/scsi/scsi_sysfs.c
+++ b/drivers/scsi/scsi_sysfs.c
@@ -214,8 +214,9 @@ store_shost_state(struct device *dev, struct device_attribute *attr,
 	if (!state)
 		return -EINVAL;
 
-	if (scsi_host_set_state(shost, state))
-		return -EINVAL;
+	scoped_guard(spinlock_irq, shost->host_lock)
+		if (scsi_host_set_state(shost, state))
+			return -EINVAL;
 	return count;
 }
 
@@ -223,7 +224,7 @@ static ssize_t
 show_shost_state(struct device *dev, struct device_attribute *attr, char *buf)
 {
 	struct Scsi_Host *shost = class_to_shost(dev);
-	const char *name = scsi_host_state_name(shost->shost_state);
+	const char *name = scsi_host_state_name(scsi_get_host_state(shost));
 
 	if (!name)
 		return -EINVAL;
diff --git a/include/scsi/scsi_host.h b/include/scsi/scsi_host.h
index 7e2011830ba4..c9754771bf29 100644
--- a/include/scsi/scsi_host.h
+++ b/include/scsi/scsi_host.h
@@ -2,6 +2,7 @@
 #ifndef _SCSI_SCSI_HOST_H
 #define _SCSI_SCSI_HOST_H
 
+#include <linux/cleanup.h>
 #include <linux/device.h>
 #include <linux/list.h>
 #include <linux/types.h>
@@ -727,7 +728,7 @@ struct Scsi_Host {
 	unsigned int  irq;
 	
 
-	enum scsi_host_state shost_state;
+	enum scsi_host_state shost_state __guarded_by(host_lock);
 
 	/* ldm bits */
 	struct device		shost_gendev, shost_dev;
@@ -785,11 +786,18 @@ static inline struct Scsi_Host *dev_to_shost(struct device *dev)
 	return container_of(dev, struct Scsi_Host, shost_gendev);
 }
 
+static inline enum scsi_host_state scsi_get_host_state(struct Scsi_Host *shost)
+{
+	return context_unsafe(READ_ONCE(shost->shost_state));
+}
+
 static inline int scsi_host_in_recovery(struct Scsi_Host *shost)
 {
-	return shost->shost_state == SHOST_RECOVERY ||
-		shost->shost_state == SHOST_CANCEL_RECOVERY ||
-		shost->shost_state == SHOST_DEL_RECOVERY ||
+	enum scsi_host_state state = scsi_get_host_state(shost);
+
+	return state == SHOST_RECOVERY ||
+		state == SHOST_CANCEL_RECOVERY ||
+		state == SHOST_DEL_RECOVERY ||
 		shost->tmf_in_progress;
 }
 
@@ -835,8 +843,9 @@ static inline struct device *scsi_get_device(struct Scsi_Host *shost)
  **/
 static inline int scsi_host_scan_allowed(struct Scsi_Host *shost)
 {
-	return shost->shost_state == SHOST_RUNNING ||
-	       shost->shost_state == SHOST_RECOVERY;
+	enum scsi_host_state state = scsi_get_host_state(shost);
+
+	return state == SHOST_RUNNING || state == SHOST_RECOVERY;
 }
 
 extern void scsi_unblock_requests(struct Scsi_Host *);
@@ -940,6 +949,7 @@ static inline unsigned char scsi_host_get_guard(struct Scsi_Host *shost)
 	return shost->prot_guard_type;
 }
 
-extern int scsi_host_set_state(struct Scsi_Host *, enum scsi_host_state);
+int scsi_host_set_state(struct Scsi_Host *shost, enum scsi_host_state state)
+	__must_hold(shost->host_lock);
 
 #endif /* _SCSI_SCSI_HOST_H */