diff mbox series

scsi: consider status is not good for offline devices

Message ID 20210330114727.234467-1-yanaijie@huawei.com (mailing list archive)
State Accepted
Headers show
Series scsi: consider status is not good for offline devices | expand

Commit Message

Jason Yan March 30, 2021, 11:47 a.m. UTC
When the scsi device status is offline, mode sense command will return a
result with only DID_NO_CONNECT set. Then in sd_read_write_protect_flag(),
only status byte of the result is checked, we still consider the command
returned good, and read sdkp->write_prot from the buffer. And because of
bug [1], garbage data is copied to the buffer, the disk sometimes
be set readonly. When the scsi device is set running again, users cannot
write data to the disk.

So return 0 in scsi_status_is_good() for offline devices to indicate
that the command has failed. And move scsi_status_is_good() down after
the definition of DID_NO_CONNECT so that we can compile fine.

[1] https://patchwork.kernel.org/project/linux-block/patch/20210318122621.330010-1-yanaijie@huawei.com/

Signed-off-by: Jason Yan <yanaijie@huawei.com>
---
 include/scsi/scsi.h | 54 +++++++++++++++++++++++----------------------
 1 file changed, 28 insertions(+), 26 deletions(-)

Comments

Martin K. Petersen May 11, 2021, 3:25 a.m. UTC | #1
On Tue, 30 Mar 2021 19:47:27 +0800, Jason Yan wrote:

> When the scsi device status is offline, mode sense command will return a
> result with only DID_NO_CONNECT set. Then in sd_read_write_protect_flag(),
> only status byte of the result is checked, we still consider the command
> returned good, and read sdkp->write_prot from the buffer. And because of
> bug [1], garbage data is copied to the buffer, the disk sometimes
> be set readonly. When the scsi device is set running again, users cannot
> write data to the disk.
> 
> [...]

Applied to 5.14/scsi-queue, thanks!

[1/1] scsi: consider status is not good for offline devices
      https://git.kernel.org/mkp/scsi/c/1ee275342234
diff mbox series

Patch

diff --git a/include/scsi/scsi.h b/include/scsi/scsi.h
index e75cca25338a..d7bfbfde0ea5 100644
--- a/include/scsi/scsi.h
+++ b/include/scsi/scsi.h
@@ -30,32 +30,6 @@  enum scsi_timeouts {
  */
 #define SCAN_WILD_CARD	~0
 
-/** scsi_status_is_good - check the status return.
- *
- * @status: the status passed up from the driver (including host and
- *          driver components)
- *
- * This returns true for known good conditions that may be treated as
- * command completed normally
- */
-static inline int scsi_status_is_good(int status)
-{
-	/*
-	 * FIXME: bit0 is listed as reserved in SCSI-2, but is
-	 * significant in SCSI-3.  For now, we follow the SCSI-2
-	 * behaviour and ignore reserved bits.
-	 */
-	status &= 0xfe;
-	return ((status == SAM_STAT_GOOD) ||
-		(status == SAM_STAT_CONDITION_MET) ||
-		/* Next two "intermediate" statuses are obsolete in SAM-4 */
-		(status == SAM_STAT_INTERMEDIATE) ||
-		(status == SAM_STAT_INTERMEDIATE_CONDITION_MET) ||
-		/* FIXME: this is obsolete in SAM-3 */
-		(status == SAM_STAT_COMMAND_TERMINATED));
-}
-
-
 /*
  * standard mode-select header prepended to all mode-select commands
  */
@@ -276,4 +250,32 @@  static inline int scsi_is_wlun(u64 lun)
 /* Used to obtain the PCI location of a device */
 #define SCSI_IOCTL_GET_PCI		0x5387
 
+/** scsi_status_is_good - check the status return.
+ *
+ * @status: the status passed up from the driver (including host and
+ *          driver components)
+ *
+ * This returns true for known good conditions that may be treated as
+ * command completed normally
+ */
+static inline int scsi_status_is_good(int status)
+{
+	if (host_byte(status) == DID_NO_CONNECT)
+	    return 0;
+
+	/*
+	 * FIXME: bit0 is listed as reserved in SCSI-2, but is
+	 * significant in SCSI-3.  For now, we follow the SCSI-2
+	 * behaviour and ignore reserved bits.
+	 */
+	status &= 0xfe;
+	return ((status == SAM_STAT_GOOD) ||
+		(status == SAM_STAT_CONDITION_MET) ||
+		/* Next two "intermediate" statuses are obsolete in SAM-4 */
+		(status == SAM_STAT_INTERMEDIATE) ||
+		(status == SAM_STAT_INTERMEDIATE_CONDITION_MET) ||
+		/* FIXME: this is obsolete in SAM-3 */
+		(status == SAM_STAT_COMMAND_TERMINATED));
+}
+
 #endif /* _SCSI_SCSI_H */