[PATCH v2 2/6] scsi: Protect INQUIRY sysfs attributes with mutex

[Brian Bunker <brian@purestorage.com>]

All INQUIRY-derived sysfs attributes (type, scsi_level, vendor, model,
rev, cdl_supported, and the binary inquiry attribute) read data that
can be updated during device rescan. These reads must be protected
against concurrent updates.

Use the existing inquiry_mutex to protect access to these sysfs
attributes. This ensures that userspace always sees consistent INQUIRY
data, even if a rescan is updating the buffer concurrently.

Replace the sdev_rd_attr macro with two new helpers,
sdev_rd_inquiry_attr_int and sdev_rd_inquiry_attr_str, which generate
the show functions for INQUIRY-derived integer and string fields and
take the inquiry_mutex around the field access.

This is preparatory work for adding INQUIRY data update support during
device rescan operations.

Signed-off-by: Brian Bunker <brian@purestorage.com>
Signed-off-by: Krishna Kant <krishna.kant@purestorage.com>
---
v2:
  - Protect all INQUIRY-derived fields (type, scsi_level, cdl_supported),
    not just the string fields (vendor, model, rev) and binary inquiry
    attribute. If we accept that INQUIRY data can change, we cannot assume
    which fields will change.
  - Replace the sdev_rd_attr macro with sdev_rd_inquiry_attr_int and
    sdev_rd_inquiry_attr_str helpers to avoid duplicating the lock/unlock
    boilerplate across each show function.

 drivers/scsi/scsi_sysfs.c | 69 +++++++++++++++++++++++++++++++--------
 1 file changed, 55 insertions(+), 14 deletions(-)

diff --git a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c
index dfc3559e7e04f..4a2b89e13b7bf 100644
--- a/drivers/scsi/scsi_sysfs.c
+++ b/drivers/scsi/scsi_sysfs.c
@@ -636,22 +636,58 @@ sdev_show_##field (struct device *dev, struct device_attribute *attr,	\
 }									\
 
 /*
- * sdev_rd_attr: macro to create a function and attribute variable for a
- * read only field.
+ * sdev_rd_inquiry_attr_int: macro to create a function and attribute for a
+ * read-only INQUIRY-derived integer field. The inquiry_mutex protects
+ * against concurrent updates during device rescan.
  */
-#define sdev_rd_attr(field, format_string)				\
-	sdev_show_function(field, format_string)			\
-static DEVICE_ATTR(field, S_IRUGO, sdev_show_##field, NULL);
+#define sdev_rd_inquiry_attr_int(field)					\
+static ssize_t								\
+sdev_show_##field(struct device *dev, struct device_attribute *attr,	\
+		  char *buf)						\
+{									\
+	struct scsi_device *sdev = to_scsi_device(dev);			\
+	ssize_t ret;							\
+									\
+	mutex_lock(&sdev->inquiry_mutex);				\
+	ret = snprintf(buf, 20, "%d\n", sdev->field);			\
+	mutex_unlock(&sdev->inquiry_mutex);				\
+	return ret;							\
+}									\
+static DEVICE_ATTR(field, S_IRUGO, sdev_show_##field, NULL)
+
+/*
+ * sdev_rd_inquiry_attr_str: macro to create a function and attribute for a
+ * read-only INQUIRY-derived string field. The inquiry_mutex protects
+ * against concurrent updates during device rescan.
+ */
+#define sdev_rd_inquiry_attr_str(field, accessor, len)			\
+static ssize_t								\
+sdev_show_##field(struct device *dev, struct device_attribute *attr,	\
+		  char *buf)						\
+{									\
+	struct scsi_device *sdev = to_scsi_device(dev);			\
+	ssize_t ret;							\
+									\
+	mutex_lock(&sdev->inquiry_mutex);				\
+	if (sdev->inquiry)						\
+		ret = snprintf(buf, 20, "%.*s\n", len,			\
+			       accessor(sdev->inquiry));		\
+	else								\
+		ret = snprintf(buf, 20, "\n");				\
+	mutex_unlock(&sdev->inquiry_mutex);				\
+	return ret;							\
+}									\
+static DEVICE_ATTR(field, S_IRUGO, sdev_show_##field, NULL)
 
 /*
  * Create the actual show/store functions and data structures.
  */
-sdev_rd_attr (type, "%d\n");
-sdev_rd_attr (scsi_level, "%d\n");
-sdev_rd_attr (vendor, "%.8s\n");
-sdev_rd_attr (model, "%.16s\n");
-sdev_rd_attr (rev, "%.4s\n");
-sdev_rd_attr (cdl_supported, "%d\n");
+sdev_rd_inquiry_attr_int(type);
+sdev_rd_inquiry_attr_int(scsi_level);
+sdev_rd_inquiry_attr_int(cdl_supported);
+sdev_rd_inquiry_attr_str(vendor, scsi_inq_vendor, SCSI_INQ_VENDOR_LEN);
+sdev_rd_inquiry_attr_str(model, scsi_inq_product, SCSI_INQ_PRODUCT_LEN);
+sdev_rd_inquiry_attr_str(rev, scsi_inq_revision, SCSI_INQ_REVISION_LEN);
 
 static ssize_t
 sdev_show_device_busy(struct device *dev, struct device_attribute *attr,
@@ -915,12 +951,17 @@ static ssize_t show_inquiry(struct file *filep, struct kobject *kobj,
 {
 	struct device *dev = kobj_to_dev(kobj);
 	struct scsi_device *sdev = to_scsi_device(dev);
+	ssize_t ret;
 
+	mutex_lock(&sdev->inquiry_mutex);
 	if (!sdev->inquiry)
-		return -EINVAL;
+		ret = -EINVAL;
+	else
+		ret = memory_read_from_buffer(buf, count, &off, sdev->inquiry,
+					       sdev->inquiry_len);
+	mutex_unlock(&sdev->inquiry_mutex);
 
-	return memory_read_from_buffer(buf, count, &off, sdev->inquiry,
-				       sdev->inquiry_len);
+	return ret;
 }
 
 static const struct bin_attribute dev_attr_inquiry = {
-- 
2.50.1 (Apple Git-155)