[PATCH 0/6] scsi: Support ALUA unavailable state and INQUIRY changes

[PATCH 0/6] scsi: Support ALUA unavailable state and INQUIRY changes

[Brian Bunker]

The SCSI mid-layer fails to refresh standard INQUIRY data during
rescans. This prevents support for the ALUA "unavailable" state,
where the Peripheral Qualifier (PQ) toggles between 1 and 0.
When a device becomes available, the cached stale PQ prevents
upper-layer drivers from attaching.

Additionally, the Unit Attention for "INQUIRY DATA HAS CHANGED" is
effectively ignored for identification changes because the rescan
does not update standard INQUIRY fields.

Introduce scsi_update_inquiry_data() to refresh the PQ, device type,
and identification strings. Use a new inquiry_mutex to protect
concurrent sysfs access while the buffer is reallocated.

If a rescan detects a change in PQ or device type, trigger
device_reprobe() to automatically match and attach the correct
driver.

Fixes added:
- ALUA/UA Fix: Refreshes standard INQUIRY data on rescan.
- Safety: Uses kmemdup and a mutex to prevent Use-After-Free.
- Deadlock Avoidance: Drops device_lock before reprobing.

Brian Bunker (6):
  scsi: Add INQUIRY data field definitions and accessor helpers
  scsi: Protect INQUIRY sysfs attributes with mutex
  scsi: Add scsi_update_inquiry_data() for updating INQUIRY data
  scsi: Refactor scsi_add_lun() to use scsi_update_inquiry_data()
  scsi: Add device reprobe support to scsi_rescan_device()
  scsi: Handle reprobe for existing devices during SCSI scan

 drivers/scsi/scsi.c        | 164 +++++++++++++++++++
 drivers/scsi/scsi_scan.c   | 321 +++++++++++++++++++++++++------------
 drivers/scsi/scsi_sysfs.c  |  74 ++++++++-
 include/scsi/scsi.h        | 171 ++++++++++++++++++++
 include/scsi/scsi_device.h |  13 ++
 5 files changed, 635 insertions(+), 108 deletions(-)

-- 
2.50.1 (Apple Git-155)

[PATCH 1/6] scsi: Add INQUIRY data field definitions and accessor helpers

[Brian Bunker]

Add well-documented inline functions and macros to parse INQUIRY data
fields according to SPC-6 section 6.7.2. These helpers provide a
consistent interface for extracting:

- Peripheral qualifier and device type from byte 0
- Removable media bit from byte 1
- Response data format from byte 3
- Capability flags (WBUS16, SYNC, CMDQUE, SFTRE) from byte 7
- Vendor, product, and revision strings

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>
---
 include/scsi/scsi.h | 171 ++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 171 insertions(+)

diff --git a/include/scsi/scsi.h b/include/scsi/scsi.h
index 08ac3200b4a4b..8f38d9a884f91 100644
--- a/include/scsi/scsi.h
+++ b/include/scsi/scsi.h
@@ -172,6 +172,177 @@ enum scsi_qc_status {
 #define SCSI_INQ_PQ_NOT_CON     0x01
 #define SCSI_INQ_PQ_NOT_CAP     0x03
 
+/*
+ * INQUIRY data field offsets and lengths
+ */
+#define SCSI_INQ_STD_LEN		36	/* Min standard INQ len */
+#define SCSI_INQ_VENDOR_OFFSET		8
+#define SCSI_INQ_VENDOR_LEN		8
+#define SCSI_INQ_PRODUCT_OFFSET		16
+#define SCSI_INQ_PRODUCT_LEN		16
+#define SCSI_INQ_REVISION_OFFSET	32
+#define SCSI_INQ_REVISION_LEN		4
+
+/*
+ * INQUIRY data byte 0 bit masks
+ */
+#define SCSI_INQ_PERIPH_QUAL_MASK	0xe0	/* bits 5-7 */
+#define SCSI_INQ_PERIPH_QUAL_SHIFT	5
+#define SCSI_INQ_DEVICE_TYPE_MASK	0x1f	/* bits 0-4 */
+
+/*
+ * INQUIRY data byte 1 bit masks
+ */
+#define SCSI_INQ_RMB_MASK		0x80	/* bit 7 */
+
+/*
+ * INQUIRY data byte 3 bit masks
+ */
+#define SCSI_INQ_RESP_DATA_FMT_MASK	0x0f	/* bits 0-3 */
+
+/*
+ * INQUIRY data byte 7 bit masks
+ */
+#define SCSI_INQ_WBUS16			0x20	/* Wide Bus 16 (bit 5) */
+#define SCSI_INQ_SYNC			0x10	/* Synchronous (bit 4) */
+#define SCSI_INQ_CMDQUE			0x02	/* Command Queuing (bit 1) */
+#define SCSI_INQ_SFTRE			0x01	/* Soft Reset (bit 0) */
+
+/**
+ * scsi_inq_periph_qual - Extract peripheral qualifier from byte 0
+ * @inq_byte0: INQUIRY data byte 0
+ *
+ * Returns: Peripheral Qualifier (0-7)
+ */
+static inline unsigned char scsi_inq_periph_qual(unsigned char inq_byte0)
+{
+	return (inq_byte0 & SCSI_INQ_PERIPH_QUAL_MASK) >>
+		SCSI_INQ_PERIPH_QUAL_SHIFT;
+}
+
+/**
+ * scsi_inq_device_type - Extract device type from byte 0
+ * @inq_byte0: INQUIRY data byte 0
+ * @lun: Logical Unit Number
+ *
+ * Extracts the peripheral device type. For well-known logical units
+ * (W-LUNs), corrects the type to TYPE_WLUN if device reports wrong type.
+ *
+ * Returns: Peripheral Device Type (0-31)
+ */
+static inline unsigned char scsi_inq_device_type(unsigned char inq_byte0,
+						 u64 lun)
+{
+	unsigned char type = inq_byte0 & SCSI_INQ_DEVICE_TYPE_MASK;
+
+	/*
+	 * Some devices respond with wrong type for well-known logical
+	 * units. Force well-known type to enumerate them correctly.
+	 */
+	if (scsi_is_wlun(lun) && type != TYPE_WLUN)
+		type = TYPE_WLUN;
+
+	return type;
+}
+
+/**
+ * scsi_inq_removable - Extract removable media bit from byte 1
+ * @inq_byte1: INQUIRY data byte 1
+ *
+ * Returns: true if removable, false if not
+ */
+static inline bool scsi_inq_removable(unsigned char inq_byte1)
+{
+	return inq_byte1 & SCSI_INQ_RMB_MASK;
+}
+
+/**
+ * scsi_inq_resp_data_fmt - Extract response data format from byte 3
+ * @inq_byte3: INQUIRY data byte 3
+ *
+ * Returns: Response Data Format (0-15)
+ */
+static inline unsigned char scsi_inq_resp_data_fmt(unsigned char inq_byte3)
+{
+	return inq_byte3 & SCSI_INQ_RESP_DATA_FMT_MASK;
+}
+
+/**
+ * scsi_inq_wbus16 - Check wide bus support from byte 7
+ * @inq_byte7: INQUIRY data byte 7
+ *
+ * Returns: true if 16-bit wide bus supported, false if not
+ */
+static inline bool scsi_inq_wbus16(unsigned char inq_byte7)
+{
+	return inq_byte7 & SCSI_INQ_WBUS16;
+}
+
+/**
+ * scsi_inq_sync - Check synchronous transfer support from byte 7
+ * @inq_byte7: INQUIRY data byte 7
+ *
+ * Returns: true if synchronous transfers supported, false if not
+ */
+static inline bool scsi_inq_sync(unsigned char inq_byte7)
+{
+	return inq_byte7 & SCSI_INQ_SYNC;
+}
+
+/**
+ * scsi_inq_cmdque - Check command queuing support from byte 7
+ * @inq_byte7: INQUIRY data byte 7
+ *
+ * Returns: true if command queuing supported, false if not
+ */
+static inline bool scsi_inq_cmdque(unsigned char inq_byte7)
+{
+	return inq_byte7 & SCSI_INQ_CMDQUE;
+}
+
+/**
+ * scsi_inq_sftre - Check soft reset support from byte 7
+ * @inq_byte7: INQUIRY data byte 7
+ *
+ * Returns: true if soft reset supported, false if not
+ */
+static inline bool scsi_inq_sftre(unsigned char inq_byte7)
+{
+	return inq_byte7 & SCSI_INQ_SFTRE;
+}
+
+/**
+ * scsi_inq_vendor - Get pointer to vendor string
+ * @inq_data: Pointer to INQUIRY data buffer
+ *
+ * Returns: Pointer to 8-byte vendor string (not null-terminated)
+ */
+static inline const char *scsi_inq_vendor(const unsigned char *inq_data)
+{
+	return (const char *)(inq_data + SCSI_INQ_VENDOR_OFFSET);
+}
+
+/**
+ * scsi_inq_product - Get pointer to product string
+ * @inq_data: Pointer to INQUIRY data buffer
+ *
+ * Returns: Pointer to 16-byte product string (not null-terminated)
+ */
+static inline const char *scsi_inq_product(const unsigned char *inq_data)
+{
+	return (const char *)(inq_data + SCSI_INQ_PRODUCT_OFFSET);
+}
+
+/**
+ * scsi_inq_revision - Get pointer to revision string
+ * @inq_data: Pointer to INQUIRY data buffer
+ *
+ * Returns: Pointer to 4-byte revision string (not null-terminated)
+ */
+static inline const char *scsi_inq_revision(const unsigned char *inq_data)
+{
+	return (const char *)(inq_data + SCSI_INQ_REVISION_OFFSET);
+}
 
 /*
  * Here are some scsi specific ioctl commands which are sometimes useful.
-- 
2.50.1 (Apple Git-155)

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

[Brian Bunker]

The vendor, model, rev, and inquiry sysfs attributes read data directly
from the inquiry buffer. When INQUIRY data 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.

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>
---
 drivers/scsi/scsi_sysfs.c | 74 +++++++++++++++++++++++++++++++++++----
 1 file changed, 67 insertions(+), 7 deletions(-)

diff --git a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c
index dfc3559e7e04f..c34c69487205f 100644
--- a/drivers/scsi/scsi_sysfs.c
+++ b/drivers/scsi/scsi_sysfs.c
@@ -648,9 +648,63 @@ static DEVICE_ATTR(field, S_IRUGO, sdev_show_##field, NULL);
  */
 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");
+
+/*
+ * Custom show functions for INQUIRY strings that take the inquiry_mutex.
+ * These strings point into the inquiry buffer which can be updated during
+ * device rescan, so we need to protect against concurrent access.
+ */
+static ssize_t
+sdev_show_vendor(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", SCSI_INQ_VENDOR_LEN,
+			       scsi_inq_vendor(sdev->inquiry));
+	else
+		ret = snprintf(buf, 20, "\n");
+	mutex_unlock(&sdev->inquiry_mutex);
+	return ret;
+}
+static DEVICE_ATTR(vendor, S_IRUGO, sdev_show_vendor, NULL);
+
+static ssize_t
+sdev_show_model(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", SCSI_INQ_PRODUCT_LEN,
+			       scsi_inq_product(sdev->inquiry));
+	else
+		ret = snprintf(buf, 20, "\n");
+	mutex_unlock(&sdev->inquiry_mutex);
+	return ret;
+}
+static DEVICE_ATTR(model, S_IRUGO, sdev_show_model, NULL);
+
+static ssize_t
+sdev_show_rev(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", SCSI_INQ_REVISION_LEN,
+			       scsi_inq_revision(sdev->inquiry));
+	else
+		ret = snprintf(buf, 20, "\n");
+	mutex_unlock(&sdev->inquiry_mutex);
+	return ret;
+}
+static DEVICE_ATTR(rev, S_IRUGO, sdev_show_rev, NULL);
+
 sdev_rd_attr (cdl_supported, "%d\n");
 
 static ssize_t
@@ -915,12 +969,18 @@ 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;
 
-	if (!sdev->inquiry)
-		return -EINVAL;
+	mutex_lock(&sdev->inquiry_mutex);
+	if (!sdev->inquiry) {
+		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)

[PATCH 3/6] scsi: Add scsi_update_inquiry_data() for updating INQUIRY data

[Brian Bunker]

Add a new function scsi_update_inquiry_data() that can safely update all
INQUIRY-derived fields for an existing SCSI device:

- Vendor, model, revision strings
- Peripheral qualifier and device type
- Capability flags (removable, lockable, tagged queuing support, etc.)
- ATA device detection and allow_restart setting

The function:
- Takes the inquiry_mutex to protect against concurrent sysfs reads
- Respects BLIST_ISROM and BLIST_NOTQ blacklist flags
- Returns 1 if device type or peripheral qualifier changed, indicating
  the caller should call device_reprobe() to re-match drivers
- Returns 0 on success with no changes requiring reprobe
- Returns negative errno on failure

This is the core infrastructure needed for updating INQUIRY data during
device rescan operations, which is required for proper ALUA unavailable
state handling.

Signed-off-by: Brian Bunker <brian@purestorage.com>
Signed-off-by: Krishna Kant <krishna.kant@purestorage.com>
---
 drivers/scsi/scsi.c        | 164 +++++++++++++++++++++++++++++++++++++
 include/scsi/scsi_device.h |  13 +++
 2 files changed, 177 insertions(+)

diff --git a/drivers/scsi/scsi.c b/drivers/scsi/scsi.c
index 76cdad063f7bc..82231c6970587 100644
--- a/drivers/scsi/scsi.c
+++ b/drivers/scsi/scsi.c
@@ -61,6 +61,7 @@
 #include <scsi/scsi_cmnd.h>
 #include <scsi/scsi_dbg.h>
 #include <scsi/scsi_device.h>
+#include <scsi/scsi_devinfo.h>
 #include <scsi/scsi_driver.h>
 #include <scsi/scsi_eh.h>
 #include <scsi/scsi_host.h>
@@ -549,6 +550,169 @@ void scsi_attach_vpd(struct scsi_device *sdev)
 	kfree(vpd_buf);
 }
 
+/**
+ * scsi_update_inquiry_data - Update standard INQUIRY data for a SCSI device
+ * @sdev: The device to update
+ * @inq_result: Buffer containing new INQUIRY data
+ * @inq_len: Length of inquiry data
+ *
+ * Updates the standard INQUIRY data (vendor, model, rev, peripheral qualifier,
+ * device type, removable media flag) and capability flags derived from INQUIRY
+ * data for a SCSI device. This is used during both initial device setup and
+ * when reprobing a device to get fresh INQUIRY information. The old inquiry
+ * buffer is freed and replaced with the new data under the protection of
+ * inquiry_mutex.
+ *
+ * Blacklist flags (BLIST_ISROM, BLIST_NOTQ) are respected when updating
+ * device properties.
+ *
+ * Returns:
+ *   SCSI_INQ_UNCHANGED on success
+ *   SCSI_INQ_REPROBE_NEEDED if type or PQ changed (caller should reprobe)
+ *  -ENOMEM on allocation failure
+ *  -EINVAL if inquiry data is too short
+ */
+int scsi_update_inquiry_data(struct scsi_device *sdev,
+			     unsigned char *inq_result, size_t inq_len)
+{
+	unsigned char *new_inquiry;
+	unsigned char old_type;
+	unsigned char old_periph_qual;
+	bool had_prior_inquiry;
+
+	/*
+	 * Ensure we have at least the minimum standard INQUIRY data (36 bytes)
+	 * to safely access device type, vendor, model, rev, and capability flags.
+	 */
+	if (inq_len < SCSI_INQ_STD_LEN) {
+		sdev_printk(KERN_WARNING, sdev,
+			    "INQUIRY data too short (%zu bytes), need at least %d\n",
+			    inq_len, SCSI_INQ_STD_LEN);
+		return -EINVAL;
+	}
+
+	/* Allocate new inquiry buffer */
+	new_inquiry = kmemdup(inq_result, max_t(size_t, inq_len, SCSI_INQ_STD_LEN),
+			      GFP_KERNEL);
+	if (!new_inquiry)
+		return -ENOMEM;
+
+	/* Update inquiry data under mutex protection */
+	mutex_lock(&sdev->inquiry_mutex);
+
+	/*
+	 * Save old values to detect changes that require reprobe.
+	 * Only meaningful if we had prior inquiry data; during initial
+	 * setup sdev->inquiry is NULL and the old values are just
+	 * zero-initialized defaults.
+	 */
+	had_prior_inquiry = (sdev->inquiry != NULL);
+	old_type = sdev->type;
+	old_periph_qual = sdev->inq_periph_qual;
+
+	kfree(sdev->inquiry);
+	sdev->inquiry = new_inquiry;
+	sdev->vendor = scsi_inq_vendor(sdev->inquiry);
+	sdev->model = scsi_inq_product(sdev->inquiry);
+	sdev->rev = scsi_inq_revision(sdev->inquiry);
+	sdev->inq_periph_qual = scsi_inq_periph_qual(inq_result[0]);
+
+	/*
+	 * Check if this is an ATA device (SATA emulation layer).
+	 * ATA devices need allow_restart set to work around SATL power
+	 * management specifications.
+	 */
+	if (strncmp(sdev->vendor, "ATA     ", 8) == 0) {
+		sdev->is_ata = 1;
+		sdev->allow_restart = 1;
+	} else
+		sdev->is_ata = 0;
+
+	/*
+	 * Update device type from INQUIRY byte 0.
+	 * BLIST_ISROM is a quirk for devices that report wrong type but should
+	 * be treated as (removable) CD-ROM. Override to TYPE_ROM as exception.
+	 */
+	if (sdev->sdev_bflags & BLIST_ISROM)
+		sdev->type = TYPE_ROM;
+	else
+		sdev->type = scsi_inq_device_type(inq_result[0], sdev->lun);
+
+	/*
+	 * Update removable flag from INQUIRY byte 1.
+	 * BLIST_ISROM devices are always removable (exception/quirk).
+	 */
+	if (sdev->sdev_bflags & BLIST_ISROM)
+		sdev->removable = 1;
+	else
+		sdev->removable = scsi_inq_removable(inq_result[1]);
+
+	/*
+	 * Set lockable to match removable. Devices with removable media
+	 * can typically have their media locked/unlocked via the
+	 * ALLOW_MEDIUM_REMOVAL command.
+	 */
+	sdev->lockable = sdev->removable;
+
+	/* Update capability flags from INQUIRY byte 7 */
+	sdev->soft_reset = (scsi_inq_sftre(inq_result[7]) &&
+			    (scsi_inq_resp_data_fmt(inq_result[3]) == 2)) ? 1 : 0;
+
+	/*
+	 * Update protocol support flags.
+	 * Only update ppr if we have enough INQUIRY data (>56 bytes) to check
+	 * byte 56, or if scsi_level indicates SCSI-3+ support. If we don't have
+	 * enough data, leave ppr unchanged to avoid incorrectly clearing it
+	 * during rescan with short INQUIRY.
+	 */
+	if (sdev->scsi_level >= SCSI_3 || inq_len > 56)
+		sdev->ppr = (sdev->scsi_level >= SCSI_3 ||
+			     (inq_len > 56 && inq_result[56] & 0x04)) ? 1 : 0;
+	sdev->wdtr = scsi_inq_wbus16(inq_result[7]);
+	sdev->sdtr = scsi_inq_sync(inq_result[7]);
+
+	/*
+	 * Update tagged queuing support from INQUIRY byte 7.
+	 * BLIST_NOTQ is an exception to force tagged queuing off.
+	 */
+	if (sdev->sdev_bflags & BLIST_NOTQ)
+		sdev->tagged_supported = 0;
+	else
+		sdev->tagged_supported = (sdev->scsi_level >= SCSI_2) &&
+					  scsi_inq_cmdque(inq_result[7]);
+	sdev->simple_tags = sdev->tagged_supported;
+
+	mutex_unlock(&sdev->inquiry_mutex);
+
+	/*
+	 * If device type or peripheral qualifier changed, return a special
+	 * code to indicate that caller should trigger device_reprobe() to
+	 * re-match with appropriate upper-layer driver.
+	 *
+	 * - Type changes require different drivers (sd vs sr vs st, etc.)
+	 * - PQ changes affect scsi_bus_match() which only matches PQ == 0
+	 *
+	 * Note: We check this AFTER updating all fields and releasing the
+	 * mutex, so all INQUIRY-derived data is current regardless of whether
+	 * reprobe is needed.
+	 */
+	if (had_prior_inquiry &&
+	    (old_type != sdev->type || old_periph_qual != sdev->inq_periph_qual)) {
+		if (old_type != sdev->type)
+			sdev_printk(KERN_NOTICE, sdev,
+				    "device type changed from %d to %d\n",
+				    old_type, sdev->type);
+		if (old_periph_qual != sdev->inq_periph_qual)
+			sdev_printk(KERN_NOTICE, sdev,
+				    "peripheral qualifier changed from %d to %d\n",
+				    old_periph_qual, sdev->inq_periph_qual);
+		return SCSI_INQ_REPROBE_NEEDED;
+	}
+
+	return SCSI_INQ_UNCHANGED;
+}
+EXPORT_SYMBOL(scsi_update_inquiry_data);
+
 /**
  * scsi_report_opcode - Find out if a given command is supported
  * @sdev:	scsi device to query
diff --git a/include/scsi/scsi_device.h b/include/scsi/scsi_device.h
index 9c2a7bbe5891e..356396b85869a 100644
--- a/include/scsi/scsi_device.h
+++ b/include/scsi/scsi_device.h
@@ -407,6 +407,19 @@ void scsi_attach_vpd(struct scsi_device *sdev);
 void scsi_cdl_check(struct scsi_device *sdev);
 int scsi_cdl_enable(struct scsi_device *sdev, bool enable);
 
+/**
+ * enum scsi_inq_update_result - Return values for scsi_update_inquiry_data()
+ * @SCSI_INQ_UNCHANGED: INQUIRY data updated, no reprobe needed
+ * @SCSI_INQ_REPROBE_NEEDED: INQUIRY data updated, device type or PQ changed
+ */
+enum scsi_inq_update_result {
+	SCSI_INQ_UNCHANGED = 0,
+	SCSI_INQ_REPROBE_NEEDED = 1,
+};
+
+int scsi_update_inquiry_data(struct scsi_device *sdev,
+			     unsigned char *inq_result, size_t inq_len);
+
 extern struct scsi_device *scsi_device_from_queue(struct request_queue *q);
 extern int __must_check scsi_device_get(struct scsi_device *);
 extern void scsi_device_put(struct scsi_device *);
-- 
2.50.1 (Apple Git-155)

[PATCH 4/6] scsi: Refactor scsi_add_lun() to use scsi_update_inquiry_data()

[Brian Bunker]

Refactor scsi_add_lun() to use the new scsi_update_inquiry_data()
function instead of inline INQUIRY parsing code. This consolidates
INQUIRY data handling in one place and ensures consistent behavior
between initial device setup and device rescan operations.

The following fields are now set by scsi_update_inquiry_data():
- inquiry buffer, vendor, model, rev pointers
- type, removable, lockable
- inq_periph_qual
- soft_reset, ppr, wdtr, sdtr
- tagged_supported, simple_tags
- is_ata, allow_restart

This patch maintains identical behavior to the previous code.
scsi_add_lun() continues to handle the remaining BLIST flags and
device-specific setup that doesn't come directly from INQUIRY data.

Signed-off-by: Brian Bunker <brian@purestorage.com>
Signed-off-by: Krishna Kant <krishna.kant@purestorage.com>
---
 drivers/scsi/scsi_scan.c | 99 +++++++---------------------------------
 1 file changed, 17 insertions(+), 82 deletions(-)

diff --git a/drivers/scsi/scsi_scan.c b/drivers/scsi/scsi_scan.c
index ef22a4228b855..554409300746f 100644
--- a/drivers/scsi/scsi_scan.c
+++ b/drivers/scsi/scsi_scan.c
@@ -879,17 +879,6 @@ static int scsi_add_lun(struct scsi_device *sdev, unsigned char *inq_result,
 	struct queue_limits lim;
 	int ret;
 
-	/*
-	 * XXX do not save the inquiry, since it can change underneath us,
-	 * save just vendor/model/rev.
-	 *
-	 * Rather than save it and have an ioctl that retrieves the saved
-	 * value, have an ioctl that executes the same INQUIRY code used
-	 * in scsi_probe_lun, let user level programs doing INQUIRY
-	 * scanning run at their own risk, or supply a user level program
-	 * that can correctly scan.
-	 */
-
 	/*
 	 * Copy at least 36 bytes of INQUIRY data, so that we don't
 	 * dereference unallocated memory when accessing the Vendor,
@@ -898,48 +887,28 @@ static int scsi_add_lun(struct scsi_device *sdev, unsigned char *inq_result,
 	 * these strings are invalid, but often they contain plausible data
 	 * nonetheless.  It doesn't matter if the device sent < 36 bytes
 	 * total, since scsi_probe_lun() initializes inq_result with 0s.
+	 *
+	 * Set sdev_bflags before calling scsi_update_inquiry_data() so it
+	 * can use the correct blacklist flags (especially BLIST_ISROM).
 	 */
-	sdev->inquiry = kmemdup(inq_result,
-				max_t(size_t, sdev->inquiry_len, 36),
-				GFP_KERNEL);
-	if (sdev->inquiry == NULL)
-		return SCSI_SCAN_NO_RESPONSE;
+	sdev->sdev_bflags = *bflags;
 
-	sdev->vendor = (char *) (sdev->inquiry + 8);
-	sdev->model = (char *) (sdev->inquiry + 16);
-	sdev->rev = (char *) (sdev->inquiry + 32);
+	if (!sdev->inquiry &&
+	    scsi_update_inquiry_data(sdev, inq_result, sdev->inquiry_len) < 0)
+		return SCSI_SCAN_NO_RESPONSE;
 
-	sdev->is_ata = strncmp(sdev->vendor, "ATA     ", 8) == 0;
-	if (sdev->is_ata) {
-		/*
-		 * sata emulation layer device.  This is a hack to work around
-		 * the SATL power management specifications which state that
-		 * when the SATL detects the device has gone into standby
-		 * mode, it shall respond with NOT READY.
-		 */
-		sdev->allow_restart = 1;
+	/* Sanity check that inquiry data was set up */
+	if (!sdev->inquiry) {
+		sdev_printk(KERN_ERR, sdev, "inquiry data not set\n");
+		return SCSI_SCAN_NO_RESPONSE;
 	}
 
-	if (*bflags & BLIST_ISROM) {
-		sdev->type = TYPE_ROM;
-		sdev->removable = 1;
-	} else {
-		sdev->type = (inq_result[0] & 0x1f);
-		sdev->removable = (inq_result[1] & 0x80) >> 7;
-
-		/*
-		 * some devices may respond with wrong type for
-		 * well-known logical units. Force well-known type
-		 * to enumerate them correctly.
-		 */
-		if (scsi_is_wlun(sdev->lun) && sdev->type != TYPE_WLUN) {
-			sdev_printk(KERN_WARNING, sdev,
-				"%s: correcting incorrect peripheral device type 0x%x for W-LUN 0x%16xhN\n",
-				__func__, sdev->type, (unsigned int)sdev->lun);
-			sdev->type = TYPE_WLUN;
-		}
-
-	}
+	/*
+	 * scsi_update_inquiry_data() has already set type, removable, lockable,
+	 * inq_periph_qual, soft_reset, ppr, wdtr, sdtr, tagged_supported,
+	 * simple_tags, is_ata, and allow_restart from INQUIRY data. Handle
+	 * special cases that need the raw inq_result or additional logic.
+	 */
 
 	if (sdev->type == TYPE_RBC || sdev->type == TYPE_ROM) {
 		/* RBC and MMC devices can return SCSI-3 compliance and yet
@@ -950,46 +919,12 @@ static int scsi_add_lun(struct scsi_device *sdev, unsigned char *inq_result,
 			*bflags |= BLIST_NOREPORTLUN;
 	}
 
-	/*
-	 * For a peripheral qualifier (PQ) value of 1 (001b), the SCSI
-	 * spec says: The device server is capable of supporting the
-	 * specified peripheral device type on this logical unit. However,
-	 * the physical device is not currently connected to this logical
-	 * unit.
-	 *
-	 * The above is vague, as it implies that we could treat 001 and
-	 * 011 the same. Stay compatible with previous code, and create a
-	 * scsi_device for a PQ of 1
-	 *
-	 * Don't set the device offline here; rather let the upper
-	 * level drivers eval the PQ to decide whether they should
-	 * attach. So remove ((inq_result[0] >> 5) & 7) == 1 check.
-	 */ 
-
-	sdev->inq_periph_qual = (inq_result[0] >> 5) & 7;
-	sdev->lockable = sdev->removable;
-	sdev->soft_reset = (inq_result[7] & 1) && ((inq_result[3] & 7) == 2);
-
-	if (sdev->scsi_level >= SCSI_3 ||
-			(sdev->inquiry_len > 56 && inq_result[56] & 0x04))
-		sdev->ppr = 1;
-	if (inq_result[7] & 0x60)
-		sdev->wdtr = 1;
-	if (inq_result[7] & 0x10)
-		sdev->sdtr = 1;
-
 	sdev_printk(KERN_NOTICE, sdev, "%s %.8s %.16s %.4s PQ: %d "
 			"ANSI: %d%s\n", scsi_device_type(sdev->type),
 			sdev->vendor, sdev->model, sdev->rev,
 			sdev->inq_periph_qual, inq_result[2] & 0x07,
 			(inq_result[3] & 0x0f) == 1 ? " CCS" : "");
 
-	if ((sdev->scsi_level >= SCSI_2) && (inq_result[7] & 2) &&
-	    !(*bflags & BLIST_NOTQ)) {
-		sdev->tagged_supported = 1;
-		sdev->simple_tags = 1;
-	}
-
 	/*
 	 * Some devices (Texel CD ROM drives) have handshaking problems
 	 * when used with the Seagate controllers. borken is initialized
-- 
2.50.1 (Apple Git-155)

[PATCH 5/6] scsi: Add device reprobe support to scsi_rescan_device()

[Brian Bunker]

Update INQUIRY data on rescan and call device_reprobe() if PQ or type
changed. Critical for ALUA unavailable state handling (SPC-4 5.15.2.4.4).

Signed-off-by: Brian Bunker <brian@purestorage.com>
Signed-off-by: Krishna Kant <krishna.kant@purestorage.com>
---
 drivers/scsi/scsi_scan.c | 131 +++++++++++++++++++++++++++++++++++----
 1 file changed, 120 insertions(+), 11 deletions(-)

diff --git a/drivers/scsi/scsi_scan.c b/drivers/scsi/scsi_scan.c
index 554409300746f..d54f64ae80f61 100644
--- a/drivers/scsi/scsi_scan.c
+++ b/drivers/scsi/scsi_scan.c
@@ -1091,6 +1091,12 @@ static unsigned char *scsi_inq_str(unsigned char *buf, unsigned char *inq,
 }
 #endif
 
+/* Forward declaration for use in scsi_probe_and_add_lun */
+static int __scsi_reprobe_inquiry(struct scsi_device *sdev,
+				  unsigned char *inq_result,
+				  size_t inq_len,
+				  bool *need_reprobe);
+
 /**
  * scsi_probe_and_add_lun - probe a LUN, if a LUN is found add it
  * @starget:	pointer to target device structure
@@ -1647,10 +1653,94 @@ int scsi_resume_device(struct scsi_device *sdev)
 }
 EXPORT_SYMBOL(scsi_resume_device);
 
+/**
+ * __scsi_reprobe_inquiry - Update INQUIRY data and reprobe device if needed
+ * @sdev: The SCSI device to reprobe
+ * @inq_result: Buffer containing fresh INQUIRY data
+ * @inq_len: Length of INQUIRY data
+ * @need_reprobe: Pointer to store whether device_reprobe() is needed
+ *
+ * Updates the device's INQUIRY data, attaches VPD pages, checks CDL support,
+ * and determines if the device needs to be reprobed due to type or peripheral
+ * qualifier changes. If no reprobe is needed, calls driver rescan functions.
+ *
+ * This function does NOT take device_lock - caller must hold it.
+ *
+ * Returns:
+ *   SCSI_INQ_UNCHANGED on success (no reprobe needed)
+ *   SCSI_INQ_REPROBE_NEEDED if type or PQ changed (reprobe needed)
+ *  -ENOMEM on allocation failure
+ *  -EINVAL if INQUIRY data is too short
+ */
+static int __scsi_reprobe_inquiry(struct scsi_device *sdev,
+				  unsigned char *inq_result,
+				  size_t inq_len,
+				  bool *need_reprobe)
+{
+	struct device *dev = &sdev->sdev_gendev;
+	int ret;
+
+	/* Update INQUIRY data */
+	ret = scsi_update_inquiry_data(sdev, inq_result, inq_len);
+	if (ret < 0) {
+		sdev_printk(KERN_ERR, sdev,
+			    "failed to update inquiry data: %d\n", ret);
+		return ret;
+	}
+
+	SCSI_LOG_SCAN_BUS(3, sdev_printk(KERN_INFO, sdev,
+		"updated inquiry data (type %d, PQ %d)\n",
+		sdev->type, sdev->inq_periph_qual));
+
+	/* Update VPD pages and CDL support */
+	scsi_attach_vpd(sdev);
+	scsi_cdl_check(sdev);
+
+	/*
+	 * If peripheral qualifier or device type changed, caller should
+	 * reprobe to update driver attachment. scsi_update_inquiry_data()
+	 * returns 1 when either changes.
+	 *
+	 * The scsi_bus_match() function only matches devices with PQ == 0,
+	 * so PQ changes cause driver attach/detach.
+	 *
+	 * Device type changes require reprobe to match the correct upper-layer
+	 * driver (e.g., sd for TYPE_DISK, sr for TYPE_ROM).
+	 */
+	if (ret == SCSI_INQ_REPROBE_NEEDED) {
+		SCSI_LOG_SCAN_BUS(3, sdev_printk(KERN_INFO, sdev,
+			"type or PQ changed, reprobe needed\n"));
+		*need_reprobe = true;
+		return ret;
+	}
+
+	/*
+	 * PQ and type unchanged, call driver's rescan functions to update
+	 * device properties (capacity, etc.)
+	 */
+	if (sdev->handler && sdev->handler->rescan)
+		sdev->handler->rescan(sdev);
+
+	if (dev->driver && try_module_get(dev->driver->owner)) {
+		struct scsi_driver *drv = to_scsi_driver(dev->driver);
+
+		if (drv->rescan)
+			drv->rescan(dev);
+		module_put(dev->driver->owner);
+	}
+
+	*need_reprobe = false;
+	return ret;
+}
+
 int scsi_rescan_device(struct scsi_device *sdev)
 {
 	struct device *dev = &sdev->sdev_gendev;
+	unsigned char *inq_result;
+	blist_flags_t bflags;
+	int result_len = 256;
 	int ret = 0;
+	bool need_reprobe = false;
 
 	device_lock(dev);
 
@@ -1666,18 +1756,37 @@ int scsi_rescan_device(struct scsi_device *sdev)
 		goto unlock;
 	}
 
-	scsi_attach_vpd(sdev);
-	scsi_cdl_check(sdev);
-
-	if (sdev->handler && sdev->handler->rescan)
-		sdev->handler->rescan(sdev);
-
-	if (dev->driver && try_module_get(dev->driver->owner)) {
-		struct scsi_driver *drv = to_scsi_driver(dev->driver);
+	/*
+	 * Rescan standard INQUIRY data to detect changes in device
+	 * properties (vendor, model, rev, peripheral qualifier, device type, etc.)
+	 */
+	inq_result = kmalloc(result_len, GFP_KERNEL);
+	if (inq_result) {
+		if (scsi_probe_lun(sdev, inq_result, result_len,
+				   &bflags) == 0) {
+			/* Successfully got fresh INQUIRY data, reprobe if needed */
+			ret = __scsi_reprobe_inquiry(sdev, inq_result,
+						     sdev->inquiry_len,
+						     &need_reprobe);
+			if (ret < 0) {
+				/* Critical failure, bail out */
+				kfree(inq_result);
+				goto unlock;
+			}
+		}
+		kfree(inq_result);
+	}
 
-		if (drv->rescan)
-			drv->rescan(dev);
-		module_put(dev->driver->owner);
+	/*
+	 * If type or PQ changed, reprobe to update driver attachment.
+	 * Must unlock device before calling device_reprobe() to avoid deadlock.
+	 */
+	if (need_reprobe) {
+		device_unlock(dev);
+		if (device_reprobe(dev) < 0)
+			sdev_printk(KERN_WARNING, sdev,
+				    "device reprobe failed\n");
+		device_lock(dev);
 	}
 
 unlock:
-- 
2.50.1 (Apple Git-155)

[PATCH 6/6] scsi: Handle reprobe for existing devices during SCSI scan

[Brian Bunker]

Complement scsi_rescan_device() reprobe by handling the scan path.
Update INQUIRY data and reprobe existing devices when PQ or type changed.

Signed-off-by: Brian Bunker <brian@purestorage.com>
Signed-off-by: Krishna Kant <krishna.kant@purestorage.com>
---
 drivers/scsi/scsi_scan.c | 91 ++++++++++++++++++++++++++++++++++++----
 1 file changed, 83 insertions(+), 8 deletions(-)

diff --git a/drivers/scsi/scsi_scan.c b/drivers/scsi/scsi_scan.c
index d54f64ae80f61..514b633d7182e 100644
--- a/drivers/scsi/scsi_scan.c
+++ b/drivers/scsi/scsi_scan.c
@@ -1129,6 +1129,7 @@ static int scsi_probe_and_add_lun(struct scsi_target *starget,
 	blist_flags_t bflags;
 	int res = SCSI_SCAN_NO_RESPONSE, result_len = 256;
 	struct Scsi_Host *shost = dev_to_shost(starget->dev.parent);
+	bool is_reprobe = false;
 
 	/*
 	 * The rescan flag is used as an optimization, the first scan of a
@@ -1136,7 +1137,32 @@ static int scsi_probe_and_add_lun(struct scsi_target *starget,
 	 */
 	sdev = scsi_device_lookup_by_target(starget, lun);
 	if (sdev) {
-		if (rescan != SCSI_SCAN_INITIAL || !scsi_device_created(sdev)) {
+		if (rescan == SCSI_SCAN_INITIAL && scsi_device_created(sdev)) {
+			/*
+			 * Initial scan found device in CREATED state (being probed
+			 * by another thread). Drop reference and allocate new -
+			 * the other thread will complete setup of the original.
+			 */
+			scsi_device_put(sdev);
+			sdev = scsi_alloc_sdev(starget, lun, hostdata);
+			if (!sdev)
+				goto out;
+		} else if (rescan != SCSI_SCAN_INITIAL && !scsi_device_created(sdev)) {
+			/*
+			 * Manual rescan of fully initialized device.
+			 * Reprobe to detect peripheral qualifier or device type
+			 * changes (e.g., ALUA state transitions).
+			 */
+			SCSI_LOG_SCAN_BUS(3, sdev_printk(KERN_INFO, sdev,
+				"scsi scan: device exists (type %d, PQ %d), reprobing\n",
+				sdev->type, sdev->inq_periph_qual));
+			is_reprobe = true;
+		} else {
+			/*
+			 * Either initial scan with fully initialized device,
+			 * or manual rescan with device still in CREATED state.
+			 * Return that device exists.
+			 */
 			SCSI_LOG_SCAN_BUS(3, sdev_printk(KERN_INFO, sdev,
 				"scsi scan: device exists on %s\n",
 				dev_name(&sdev->sdev_gendev)));
@@ -1151,11 +1177,11 @@ static int scsi_probe_and_add_lun(struct scsi_target *starget,
 								 sdev->model);
 			return SCSI_SCAN_LUN_PRESENT;
 		}
-		scsi_device_put(sdev);
-	} else
+	} else {
 		sdev = scsi_alloc_sdev(starget, lun, hostdata);
-	if (!sdev)
-		goto out;
+		if (!sdev)
+			goto out;
+	}
 
 	if (scsi_device_is_pseudo_dev(sdev)) {
 		if (bflagsp)
@@ -1170,6 +1196,40 @@ static int scsi_probe_and_add_lun(struct scsi_target *starget,
 	if (scsi_probe_lun(sdev, result, result_len, &bflags))
 		goto out_free_result;
 
+	/*
+	 * For reprobe scenarios, update the inquiry data with fresh
+	 * INQUIRY results. The device already exists in sysfs, so we
+	 * don't call scsi_add_lun() which would try to add it again.
+	 */
+	if (is_reprobe) {
+		bool need_reprobe = false;
+		int update_ret = __scsi_reprobe_inquiry(sdev, result, result_len,
+							&need_reprobe);
+
+		if (update_ret < 0) {
+			res = SCSI_SCAN_NO_RESPONSE;
+			goto out_free_result;
+		}
+
+		if (bflagsp)
+			*bflagsp = bflags;
+
+		/*
+		 * If type or PQ changed, reprobe to update driver attachment.
+		 * Reprobe failure is not fatal - device exists, just may have
+		 * wrong driver attached.
+		 */
+		if (need_reprobe) {
+			if (device_reprobe(&sdev->sdev_gendev) < 0)
+				sdev_printk(KERN_WARNING, sdev,
+					    "device reprobe failed\n");
+		}
+
+		/* Device already exists, just return success */
+		res = SCSI_SCAN_LUN_PRESENT;
+		goto out_free_result;
+	}
+
 	if (bflagsp)
 		*bflagsp = bflags;
 	/*
@@ -1252,12 +1312,27 @@ static int scsi_probe_and_add_lun(struct scsi_target *starget,
 			if (scsi_device_get(sdev) == 0) {
 				*sdevp = sdev;
 			} else {
-				__scsi_remove_device(sdev);
+				if (!is_reprobe)
+					__scsi_remove_device(sdev);
 				res = SCSI_SCAN_NO_RESPONSE;
 			}
 		}
-	} else
-		__scsi_remove_device(sdev);
+		/*
+		 * For reprobe case, we held a reference from
+		 * scsi_device_lookup_by_target(), release it now.
+		 */
+		if (is_reprobe)
+			scsi_device_put(sdev);
+	} else {
+		/*
+		 * For reprobe, device already exists - don't remove it.
+		 * Just release the reference we got from lookup.
+		 */
+		if (is_reprobe)
+			scsi_device_put(sdev);
+		else
+			__scsi_remove_device(sdev);
+	}
  out:
 	return res;
 }
-- 
2.50.1 (Apple Git-155)

Re: [PATCH 1/6] scsi: Add INQUIRY data field definitions and accessor helpers

[Hannes Reinecke]

On 4/24/26 23:53, Brian Bunker wrote:
> Add well-documented inline functions and macros to parse INQUIRY data
> fields according to SPC-6 section 6.7.2. These helpers provide a
> consistent interface for extracting:
> 
> - Peripheral qualifier and device type from byte 0
> - Removable media bit from byte 1
> - Response data format from byte 3
> - Capability flags (WBUS16, SYNC, CMDQUE, SFTRE) from byte 7
> - Vendor, product, and revision strings
> 
> 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>
> ---
>   include/scsi/scsi.h | 171 ++++++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 171 insertions(+)
> 
Reviewed-by: Hannes Reinecke <hare@suse.de>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke                  Kernel Storage Architect
hare@suse.de                                +49 911 74053 688
SUSE Software Solutions GmbH, Frankenstr. 146, 90461 Nürnberg
HRB 36809 (AG Nürnberg), GF: I. Totev, A. McDonald, W. Knoblich

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

[Hannes Reinecke]

On 4/24/26 23:53, Brian Bunker wrote:
> The vendor, model, rev, and inquiry sysfs attributes read data directly
> from the inquiry buffer. When INQUIRY data 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.
> 
> 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>
> ---
>   drivers/scsi/scsi_sysfs.c | 74 +++++++++++++++++++++++++++++++++++----
>   1 file changed, 67 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c
> index dfc3559e7e04f..c34c69487205f 100644
> --- a/drivers/scsi/scsi_sysfs.c
> +++ b/drivers/scsi/scsi_sysfs.c
> @@ -648,9 +648,63 @@ static DEVICE_ATTR(field, S_IRUGO, sdev_show_##field, NULL);
>    */
>   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");

Feels a bit odd, protecting 'vendor', 'model' etc, but leaving out
'type' and 'level'.
Any particular reason for this?
If we are prepared to accept that the inquiry data changes, we cannot
assume which fields will be changed, rather we have to expect that
_all_ fields can be changed.

Cheers,

Hannes
-- 
Dr. Hannes Reinecke                  Kernel Storage Architect
hare@suse.de                                +49 911 74053 688
SUSE Software Solutions GmbH, Frankenstr. 146, 90461 Nürnberg
HRB 36809 (AG Nürnberg), GF: I. Totev, A. McDonald, W. Knoblich

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

[Brian Bunker]

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)

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

[Damien Le Moal]

On 4/29/26 10:27, Brian Bunker wrote:
> 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);			\

sysfs_emit() ?

> +	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");				\

Same here.

> +	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 = {


-- 
Damien Le Moal
Western Digital Research

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

[Bart Van Assche]

On 4/28/26 6:27 PM, Brian Bunker wrote:
> +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;							\
> +}									\

Please use guard()() and sysfs_emit() in new code and remove the "ret"
variable.

> +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;							\
> +}									\

Same comment here.

> @@ -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;
>   }
Also here, please use guard()() and remove the "ret" variable.

Thanks,

Bart.

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

[Brian Bunker]

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>
---
v3:
  - Use sysfs_emit() instead of snprintf() in the new show functions.
  - Use guard(mutex)() for scoped lock acquisition and drop the local
    ret variable.

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 | 52 ++++++++++++++++++++++++++++++---------
 1 file changed, 41 insertions(+), 11 deletions(-)

diff --git a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c
index dfc3559e7e04f..9201f1f04d6b4 100644
--- a/drivers/scsi/scsi_sysfs.c
+++ b/drivers/scsi/scsi_sysfs.c
@@ -636,22 +636,51 @@ 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_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);			\
+									\
+	guard(mutex)(&sdev->inquiry_mutex);				\
+	return sysfs_emit(buf, "%d\n", sdev->field);			\
+}									\
+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_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_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);			\
+									\
+	guard(mutex)(&sdev->inquiry_mutex);				\
+	if (sdev->inquiry)						\
+		return sysfs_emit(buf, "%.*s\n", len,			\
+				  accessor(sdev->inquiry));		\
+	return sysfs_emit(buf, "\n");					\
+}									\
+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,
@@ -916,6 +945,7 @@ 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);
 
+	guard(mutex)(&sdev->inquiry_mutex);
 	if (!sdev->inquiry)
 		return -EINVAL;
 
-- 
2.50.1 (Apple Git-155)

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

[Hannes Reinecke]

On 4/30/26 00:49, Brian Bunker wrote:
> 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>
> ---
> v3:
>    - Use sysfs_emit() instead of snprintf() in the new show functions.
>    - Use guard(mutex)() for scoped lock acquisition and drop the local
>      ret variable.
> 
> 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.
> 
Reviewed-by: Hannes Reinecke <hare@suse.de>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke                  Kernel Storage Architect
hare@suse.de                                +49 911 74053 688
SUSE Software Solutions GmbH, Frankenstr. 146, 90461 Nürnberg
HRB 36809 (AG Nürnberg), GF: I. Totev, A. McDonald, W. Knoblich

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

[Bart Van Assche]

On 4/29/26 3:49 PM, Brian Bunker wrote:
> 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>

Shouldn't your signed-off-by come last since you are the person posting
this patch?

> -#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_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);			\
> +									\
> +	guard(mutex)(&sdev->inquiry_mutex);				\
> +	if (sdev->inquiry)						\
> +		return sysfs_emit(buf, "%.*s\n", len,			\
> +				  accessor(sdev->inquiry));		\
> +	return sysfs_emit(buf, "\n");					\
> +}									\
> +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);

I'm not aware of any other example of "accessor" functions in the Linux
kernel for character data that is not '\0' terminated. Accessor
functions like scsi_inq_vendor() only hide the offset of the vendor data
but not the length. I prefer that all accessor functions would be 
removed for strings that are not '\0'-terminated. sdev_show_##field()
can be modified to accept an offset and a length instead of an accessor
function and a length.

Thanks,

Bart.

Re: [PATCH 1/6] scsi: Add INQUIRY data field definitions and accessor helpers

[Bart Van Assche]

On 4/24/26 2:53 PM, Brian Bunker wrote:
> +/**
> + * scsi_inq_removable - Extract removable media bit from byte 1
> + * @inq_byte1: INQUIRY data byte 1
> + *
> + * Returns: true if removable, false if not
> + */
> +static inline bool scsi_inq_removable(unsigned char inq_byte1)
> +{
> +	return inq_byte1 & SCSI_INQ_RMB_MASK;
> +}

This type of functions is highly unusual in the Linux kernel. We
typically open-code functions like the above instead of introducing a
function with a name that is almost longer than the function body.

Thanks,

Bart.

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

[Brian Bunker]

Bart, Hannes,

Before I respin, I'd like to get your alignment on the API design.

Hannes, in your review of the earlier single-patch version of this
work last year, you wrote:

> +	sdev->vendor = (char *)(sdev->inquiry + 8);
> +	sdev->model = (char *)(sdev->inquiry + 16);
> +	sdev->rev = (char *)(sdev->inquiry + 32);
>
> I really hate these.
> Can't we replace them with accessor functions and drop the pointers?

That's the direction I want to take. Bart's feedback on v3 2/6 then
pushed back specifically on accessor functions that hand out pointers
to character data which is not '\0' terminated, and suggested that
sdev_show_##field() could take an offset and length instead.

Those two pieces of feedback converge: drop the cached pointers, but
don't replace them with anything that exposes a pointer to
non-NUL-terminated bytes. Before I respin around that, I want to make
sure I have the scope of Bart's objection right.

To help frame it, I audited every call site in the tree that touches
sdev->vendor, sdev->model, or sdev->rev - both the direct field name
and the alias forms callers actually use (cp->device->vendor,
cd->device->vendor, scsidp->vendor, ch->dt[elem]->vendor, SDp->vendor,
STp->device->..., etc.). For each hit I checked two failure modes:

  - bare "%s" in a format string (the qla2xxx pattern); and
  - NUL-assuming string functions (strlen, strcpy, strcat, strdup,
    strstr, strchr, strrchr, strcmp, strcasecmp, strscpy, strlcpy).

Result, across all three fields:

  Site                                              Field(s)   NUL-term?
  --------------------------------------------------  ---------  ----
  drivers/hwmon/drivetemp.c            (strncmp)       model       no
  drivers/scsi/aacraid/linit.c         (strncmp)       vendor      no
  drivers/scsi/advansys.c              (strncmp)       vendor      no
  drivers/scsi/scsi_dh.c               (strncmp)       v, m        no
  drivers/scsi/scsi_proc.c             (bounded loop)  v, m, r     no
  drivers/scsi/scsi_scan.c             (strncmp/%.Ns)  v, m, r     no
  drivers/scsi/sg.c                    (%8.8s)         v, m, r     no
  drivers/scsi/sr_vendor.c             (strncmp)       v, m        no
  drivers/scsi/scsi_sysfs.c            (%.Ns)          v, m, r     no
  drivers/scsi/st.c                    (strncmp)       v, m, r     no
  drivers/scsi/ch.c                    (%8.8s)         v, m, r     no
  drivers/scsi/storvsc_drv.c           (strncmp)       vendor      no
  drivers/target/target_core_pscsi.c   (snprintf %.Ns) v, m, r     no
  drivers/scsi/qla2xxx/qla_isr.c:3664  ("%s")          vendor      YES

Of 18 distinct call sites, 17 are bounded by either an explicit
length or a precision specifier. Exactly one - qla2xxx - passes
cp->device->vendor to a "%s" format, which walks past the 8-byte
vendor field into the model field until it finds a NUL byte. That's
a real over-read today.

I also checked every strlen/strcpy/strcat/etc. call site in the tree
and confirmed none of them runs on sdev->vendor / sdev->model /
sdev->rev directly. The two scsi_dh.c and st.c sites that look like
they might (strncmp(sdev->vendor, blacklist->vendor,
strlen(blacklist->vendor))) actually call strlen on the blacklist
entry, which is a NUL-terminated literal - the strncmp is bounded by
that length on the sdev side. Safe.

So there are no NUL-termination misuses on model or rev anywhere in
the tree, and exactly one on vendor (qla2xxx).

That means the work has two motivations that are worth separating:

  (a) qla2xxx is a bug today and worth fixing on its own with a
      Fixes: tag, independent of any larger refactoring.

  (b) The remaining 17 sites work today but only because each author
      knew the field is space-padded with no terminator. The
      conversion series doesn't fix bugs at those sites; it removes
      the opportunity for the next person to write "%s" somewhere and
      reintroduce a qla2xxx-class bug, and it's preparatory for
      INQUIRY refresh on rescan (where the buffer can be reallocated
      and a raw pointer into it becomes a UAF risk).

For (b), I had been heading toward an API built around copy-out and
match helpers - i.e. nothing in the public API hands back a pointer
to non-NUL-terminated bytes:

  size_t scsi_device_vendor(struct scsi_device *sdev,
                            char *buf, size_t len);
  size_t scsi_device_model(struct scsi_device *sdev,
                           char *buf, size_t len);
  size_t scsi_device_rev(struct scsi_device *sdev,
                         char *buf, size_t len);

  bool   scsi_device_vendor_match(struct scsi_device *sdev,
                                  const char *prefix);
  bool   scsi_device_model_match(struct scsi_device *sdev,
                                 const char *prefix);
  bool   scsi_device_rev_match(struct scsi_device *sdev,
                               const char *prefix);

The copy helpers take a caller buffer, copy the field, and
NUL-terminate. The match helpers take a NUL-terminated prefix string
from the caller and do a bounded comparison against the
(non-terminated) field bytes. The earlier scsi_inq_vendor() /
scsi_inq_product() / scsi_inq_revision() helpers that returned a
const char * into the inquiry buffer have been dropped.

A few questions before I respin:

  1. Bart, does the copy-out / match API above address the "no
     accessors for non-NUL-terminated data" concern, or is the
     objection broader?

  2. Hannes, would you prefer the conversion patches and the
     removal of sdev->vendor / sdev->model / sdev->rev to land as
     one series, or split into two - conversions first, removal as a
     follow-up once the conversions have settled?

I'd rather get your input on the API before sending another
12+ patch respin.

Thanks,
Brian

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

[Bart Van Assche]

On 5/1/26 3:11 PM, Brian Bunker wrote:
>> +	sdev->vendor = (char *)(sdev->inquiry + 8);
>> +	sdev->model = (char *)(sdev->inquiry + 16);
>> +	sdev->rev = (char *)(sdev->inquiry + 32);
>>
>> I really hate these.
>> Can't we replace them with accessor functions and drop the pointers?

Hannes, what type of accessor functions do you have in mind? I don't
like accessor functions that only do half of the job (returning the
start pointer but not the length). Or are you perhaps suggesting to
define accessor functions that return a struct with both the start
pointer and the length, something that is uncommon in the Linux kernel?

Another possibility is to change sdev->vendor, sdev->model and
sdev->rev from pointers to fixed size strings into '\0'-terminated char
arrays.

Thanks,

Bart.

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

[Bart Van Assche]

On 5/2/26 6:37 PM, Bart Van Assche wrote:
> Another possibility is to change sdev->vendor, sdev->model and
> sdev->rev from pointers to fixed size strings into '\0'-terminated char
> arrays.

(replying to my own email)

How about the untested patch below?

scsi: core: Convert inquiry information

Currently the vendor, model, and revision members of struct scsi_device
are pointers to fixed-length strings that are not NUL-terminated.
Fixed-precision format specifiers (e.g., "%.8s") are required whenever
they are printed and strncmp() must be used to compare these fields.
This is error-prone.

Convert these fields to fixed-size character arrays within struct
scsi_device and remove trailing white space at initialization time.

This patch fixes a bug in the qla2xxx driver. It makes the following
code safe:

                 if (state_flags & BIT_4)
                         scmd_printk(KERN_WARNING, cp,
                             "Unsupported device '%s' found.\n",
                             cp->device->vendor);

diff --git a/drivers/hwmon/drivetemp.c b/drivers/hwmon/drivetemp.c
index 002e0660a0b8..e71b99f70076 100644
--- a/drivers/hwmon/drivetemp.c
+++ b/drivers/hwmon/drivetemp.c
@@ -306,7 +306,7 @@ static bool drivetemp_sct_avoid(struct 
drivetemp_data *st)
  	struct scsi_device *sdev = st->sdev;
  	unsigned int ctr;

-	if (!sdev->model)
+	if (!sdev->model[0])
  		return false;

  	/*
diff --git a/drivers/scsi/advansys.c b/drivers/scsi/advansys.c
index fcf059bf41e8..f92ad396ac9c 100644
--- a/drivers/scsi/advansys.c
+++ b/drivers/scsi/advansys.c
@@ -7204,7 +7204,7 @@ static void AscAsyncFix(ASC_DVC_VAR *asc_dvc, 
struct scsi_device *sdev)
  	if (asc_dvc->init_sdtr & tid_bits)
  		return;

-	if ((type == TYPE_ROM) && (strncmp(sdev->vendor, "HP ", 3) == 0))
+	if (type == TYPE_ROM && strcmp(sdev->vendor, "HP") == 0)
  		asc_dvc->pci_fix_asyn_xfer_always |= tid_bits;

  	asc_dvc->pci_fix_asyn_xfer |= tid_bits;
diff --git a/drivers/scsi/ch.c b/drivers/scsi/ch.c
index 4010fdbf813c..fb59ca035843 100644
--- a/drivers/scsi/ch.c
+++ b/drivers/scsi/ch.c
@@ -408,7 +408,7 @@ ch_readconfig(scsi_changer *ch)
  				/* should not happen */
  				VPRINTK(KERN_CONT, "Huh? device not found!\n");
  			} else {
-				VPRINTK(KERN_CONT, "name: %8.8s %16.16s %4.4s\n",
+				VPRINTK(KERN_CONT, "name: %8s %16s %4s\n",
  					ch->dt[elem]->vendor,
  					ch->dt[elem]->model,
  					ch->dt[elem]->rev);
diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c
index a1b116cd4723..bdec827b82a1 100644
--- a/drivers/scsi/hpsa.c
+++ b/drivers/scsi/hpsa.c
@@ -1262,7 +1262,7 @@ static void hpsa_show_dev_msg(const char *level, 
struct ctlr_info *h,
  	}

  	dev_printk(level, &h->pdev->dev,
-			"scsi %d:%d:%d:%d: %s %s %.8s %.16s %s SSDSmartPathCap%c En%c Exp=%d\n",
+			"scsi %d:%d:%d:%d: %s %s %s %s %s SSDSmartPathCap%c En%c Exp=%d\n",
  			h->scsi_host->host_no, dev->bus, dev->target, dev->lun,
  			description,
  			scsi_device_type(dev->devtype),
diff --git a/drivers/scsi/scsi_scan.c b/drivers/scsi/scsi_scan.c
index ef22a4228b85..242bd46df3ba 100644
--- a/drivers/scsi/scsi_scan.c
+++ b/drivers/scsi/scsi_scan.c
@@ -292,9 +292,9 @@ static struct scsi_device *scsi_alloc_sdev(struct 
scsi_target *starget,
  	if (!sdev)
  		goto out;

-	sdev->vendor = scsi_null_device_strs;
-	sdev->model = scsi_null_device_strs;
-	sdev->rev = scsi_null_device_strs;
+	strscpy(sdev->vendor, scsi_null_device_strs);
+	strscpy(sdev->model, scsi_null_device_strs);
+	strscpy(sdev->rev, scsi_null_device_strs);
  	sdev->host = shost;
  	sdev->queue_ramp_up_period = SCSI_DEFAULT_RAMP_UP_PERIOD;
  	sdev->id = starget->id;
@@ -857,6 +857,21 @@ static int scsi_probe_lun(struct scsi_device *sdev, 
unsigned char *inq_result,
  	return 0;
  }

+static void strip_trailing_spaces(char *s)
+{
+	size_t size;
+	char *end;
+
+	size = strlen(s);
+	if (!size)
+		return;
+
+	end = s + size - 1;
+	while (end >= s && isspace(*end))
+		end--;
+	*(end + 1) = '\0';
+}
+
  /**
   * scsi_add_lun - allocate and fully initialze a scsi_device
   * @sdev:	holds information to be stored in the new scsi_device
@@ -905,11 +920,17 @@ static int scsi_add_lun(struct scsi_device *sdev, 
unsigned char *inq_result,
  	if (sdev->inquiry == NULL)
  		return SCSI_SCAN_NO_RESPONSE;

-	sdev->vendor = (char *) (sdev->inquiry + 8);
-	sdev->model = (char *) (sdev->inquiry + 16);
-	sdev->rev = (char *) (sdev->inquiry + 32);
-
-	sdev->is_ata = strncmp(sdev->vendor, "ATA     ", 8) == 0;
+	memcpy(sdev->vendor, sdev->inquiry + 8, 8);
+	sdev->vendor[8] = 0;
+	strip_trailing_spaces(sdev->vendor);
+	memcpy(sdev->model, sdev->inquiry + 16, 16);
+	sdev->model[16] = 0;
+	strip_trailing_spaces(sdev->model);
+	memcpy(sdev->rev, sdev->inquiry + 32, 4);
+	sdev->rev[4] = 0;
+	strip_trailing_spaces(sdev->rev);
+
+	sdev->is_ata = strcmp(sdev->vendor, "ATA") == 0;
  	if (sdev->is_ata) {
  		/*
  		 * sata emulation layer device.  This is a hack to work around
@@ -978,7 +999,7 @@ static int scsi_add_lun(struct scsi_device *sdev, 
unsigned char *inq_result,
  	if (inq_result[7] & 0x10)
  		sdev->sdtr = 1;

-	sdev_printk(KERN_NOTICE, sdev, "%s %.8s %.16s %.4s PQ: %d "
+	sdev_printk(KERN_NOTICE, sdev, "%s %s %s %s PQ: %d "
  			"ANSI: %d%s\n", scsi_device_type(sdev->type),
  			sdev->vendor, sdev->model, sdev->rev,
  			sdev->inq_periph_qual, inq_result[2] & 0x07,
diff --git a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c
index dfc3559e7e04..3282ad956d1f 100644
--- a/drivers/scsi/scsi_sysfs.c
+++ b/drivers/scsi/scsi_sysfs.c
@@ -648,9 +648,9 @@ static DEVICE_ATTR(field, S_IRUGO, 
sdev_show_##field, NULL);
   */
  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 (vendor, "%s\n");
+sdev_rd_attr (model, "%s\n");
+sdev_rd_attr (rev, "%s\n");
  sdev_rd_attr (cdl_supported, "%d\n");

  static ssize_t
diff --git a/drivers/scsi/sg.c b/drivers/scsi/sg.c
index 2b4b2a1a8e44..b6c248e69b3a 100644
--- a/drivers/scsi/sg.c
+++ b/drivers/scsi/sg.c
@@ -2507,7 +2507,7 @@ static int sg_proc_seq_show_devstrs(struct 
seq_file *s, void *v)
  	sdp = it ? sg_lookup_dev(it->index) : NULL;
  	scsidp = sdp ? sdp->device : NULL;
  	if (sdp && scsidp && (!atomic_read(&sdp->detaching)))
-		seq_printf(s, "%8.8s\t%16.16s\t%4.4s\n",
+		seq_printf(s, "%8s\t%16s\t%4s\n",
  			   scsidp->vendor, scsidp->model, scsidp->rev);
  	else
  		seq_puts(s, "<no active device>\n");
diff --git a/drivers/target/target_core_pscsi.c 
b/drivers/target/target_core_pscsi.c
index 9ed2818c3028..1718d1620f24 100644
--- a/drivers/target/target_core_pscsi.c
+++ b/drivers/target/target_core_pscsi.c
@@ -169,14 +169,11 @@ pscsi_set_inquiry_info(struct scsi_device *sdev, 
struct t10_wwn *wwn)
  	 * Use sdev->inquiry data from drivers/scsi/scsi_scan.c:scsi_add_lun()
  	 */
  	BUILD_BUG_ON(sizeof(wwn->vendor) != INQUIRY_VENDOR_LEN + 1);
-	snprintf(wwn->vendor, sizeof(wwn->vendor),
-		 "%." __stringify(INQUIRY_VENDOR_LEN) "s", sdev->vendor);
+	strscpy(wwn->vendor, sdev->vendor);
  	BUILD_BUG_ON(sizeof(wwn->model) != INQUIRY_MODEL_LEN + 1);
-	snprintf(wwn->model, sizeof(wwn->model),
-		 "%." __stringify(INQUIRY_MODEL_LEN) "s", sdev->model);
+	strscpy(wwn->model, sdev->model);
  	BUILD_BUG_ON(sizeof(wwn->revision) != INQUIRY_REVISION_LEN + 1);
-	snprintf(wwn->revision, sizeof(wwn->revision),
-		 "%." __stringify(INQUIRY_REVISION_LEN) "s", sdev->rev);
+	strscpy(wwn->revision, sdev->rev);
  }

  static int
diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
index 4805e40ed4d7..70315f7fdafe 100644
--- a/drivers/ufs/core/ufshcd.c
+++ b/drivers/ufs/core/ufshcd.c
@@ -702,7 +702,7 @@ static void ufshcd_print_host_state(struct ufs_hba *hba)
  	dev_err(hba->dev, "quirks=0x%x, dev. quirks=0x%x\n", hba->quirks,
  		hba->dev_quirks);
  	if (sdev_ufs)
-		dev_err(hba->dev, "UFS dev info: %.8s %.16s rev %.4s\n",
+		dev_err(hba->dev, "UFS dev info: %s %s rev %s\n",
  			sdev_ufs->vendor, sdev_ufs->model, sdev_ufs->rev);

  	ufshcd_print_clk_freqs(hba);
diff --git a/include/scsi/scsi_device.h b/include/scsi/scsi_device.h
index 9c2a7bbe5891..cce4587b4b80 100644
--- a/include/scsi/scsi_device.h
+++ b/include/scsi/scsi_device.h
@@ -137,9 +137,9 @@ struct scsi_device {
  	struct mutex inquiry_mutex;
  	unsigned char inquiry_len;	/* valid bytes in 'inquiry' */
  	unsigned char * inquiry;	/* INQUIRY response data */
-	const char * vendor;		/* [back_compat] point into 'inquiry' ... */
-	const char * model;		/* ... after scan; point to static string */
-	const char * rev;		/* ... "nullnullnullnull" before scan */
+	char vendor[9];
+	char model[17];
+	char rev[5];

  #define SCSI_DEFAULT_VPD_LEN	255	/* default SCSI VPD page size (max) */
  	struct scsi_vpd __rcu *vpd_pg0;

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

[Brian Bunker]

On 5/3/26 8:44 AM, Bart Van Assche wrote:
> scsi: core: Convert inquiry information
>
> Convert these fields to fixed-size character arrays within struct
> scsi_device and remove trailing white space at initialization time.
>
> This patch fixes a bug in the qla2xxx driver.

Thanks for the patch - picking up the qla2xxx fix as a side effect of
the conversion is a nice win, and the call-site cleanup (no more
%.Ns / strncmp) is genuinely more readable.

> +static void strip_trailing_spaces(char *s)
> +{
> +	size_t size;
> +	char *end;
> +
> +	size = strlen(s);
> +	if (!size)
> +		return;
> +
> +	end = s + size - 1;
> +	while (end >= s && isspace(*end))
> +		end--;
> +	*(end + 1) = '\0';
> +}

This changes the sysfs ABI for /sys/.../vendor, /sys/.../model and
/sys/.../rev. Today these files contain the raw 8/16/4 bytes from
the INQUIRY response, space-padded as the SCSI spec requires for
short identifiers, followed by a newline. For example, on a Pure
array today:

  $ od -x /sys/.../vendor
  0000000 5550 4552 2020 2020 000a
  0000011

i.e. "PURE    \n" - 9 bytes. After the patch the same file would
contain "PURE\n" - 5 bytes. That's spec-correct (per SPC, trailing
bytes ARE padding, not data) but it's a userspace-visible change
that's been stable for some time. The places I'd worry about are
udev rules using ATTRS{vendor}=="PURE    " (literal match, no
whitespace handling) and shell scripts comparing against the padded
form. None of those are in-tree.

> -	if ((type == TYPE_ROM) && (strncmp(sdev->vendor, "HP ", 3) == 0))
> +	if (type == TYPE_ROM && strcmp(sdev->vendor, "HP") == 0)

A few of the strncmp -> strcmp conversions narrow the match in ways
that are worth a second look even after the strip - this one matches
only literal "HP" post-strip, where the original would have matched
"HP COMPAQ" too.

> -	const char * vendor;		/* [back_compat] point into 'inquiry' ... */
> -	const char * model;		/* ... after scan; point to static string */
> -	const char * rev;		/* ... "nullnullnullnull" before scan */
> +	char vendor[9];
> +	char model[17];
> +	char rev[5];

Going back to Hannes' original suggestion - dropping
sdev->vendor / sdev->model / sdev->rev from struct scsi_device
entirely - is another way to get the qla2xxx bug out of the tree
without touching the sysfs output. The shape I had in mind is small
helpers against the (still raw, still space-padded) inquiry buffer:

  bool   scsi_device_vendor_match(struct scsi_device *sdev,
                                  const char *prefix);
  bool   scsi_device_model_match(struct scsi_device *sdev,
                                 const char *prefix);
  bool   scsi_device_rev_match(struct scsi_device *sdev,
                               const char *prefix);

  size_t scsi_device_vendor(struct scsi_device *sdev,
                            char *buf, size_t len);
  size_t scsi_device_model(struct scsi_device *sdev,
                           char *buf, size_t len);
  size_t scsi_device_rev(struct scsi_device *sdev,
                         char *buf, size_t len);

Match helpers do a bounded comparison against the inquiry bytes
under inquiry_mutex; copy helpers copy and NUL-terminate into a
caller buffer. Sysfs reads stay byte-exact (no strip); qla2xxx
becomes a small stack buffer + scsi_device_vendor() and the
over-read goes away. Scope is comparable to your conversion -
~18 call sites across ~8 files, each migrating separately - but
no userspace ABI delta and no cached pointers left in
struct scsi_device.

Happy either way - just wanted to surface the ABI concern and
remind you of the other direction before you spend more time on the
conversion patch.

Thanks,
Brian

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

[Bart Van Assche]

On 5/4/26 8:36 PM, Brian Bunker wrote:
> This changes the sysfs ABI for /sys/.../vendor, /sys/.../model and
> /sys/.../rev.

Not necessarily. If the format specifiers %-8s / %-16s / %-4s are used 
for reporting these member variables via sysfs, the sysfs output should
remain the same.

Thanks,

Bart.

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

[Brian Bunker]

On 5/5/26 1:24 AM, Bart Van Assche wrote:
>> This changes the sysfs ABI for /sys/.../vendor, /sys/.../model and
>> /sys/.../rev.
>
> Not necessarily. If the format specifiers %-8s / %-16s / %-4s are used
> for reporting these member variables via sysfs, the sysfs output should
> remain the same.

Agreed - left-justified width specifiers would re-pad on the way out
and keep the byte-for-byte sysfs output. The current diff uses plain
%s for the sysfs attributes:

  -sdev_rd_attr (vendor, "%.8s\n");
  -sdev_rd_attr (model,  "%.16s\n");
  -sdev_rd_attr (rev,    "%.4s\n");
  +sdev_rd_attr (vendor, "%s\n");
  +sdev_rd_attr (model,  "%s\n");
  +sdev_rd_attr (rev,    "%s\n");

so as posted "PURE    \n" still shrinks to "PURE\n". If those become
%-8s / %-16s / %-4s in the next revision the ABI concern goes away.

Thanks,
Brian