[PATCH] scsi: core: Add 'serial' sysfs attribute for SCSI/SATA

Igor Pylypiv posted 1 patch 3 weeks, 2 days ago
There is a newer version of this series
drivers/scsi/scsi_lib.c    | 53 ++++++++++++++++++++++++++++++++++++++
drivers/scsi/scsi_sysfs.c  | 14 ++++++++++
include/scsi/scsi_device.h |  1 +
3 files changed, 68 insertions(+)
[PATCH] scsi: core: Add 'serial' sysfs attribute for SCSI/SATA
Posted by Igor Pylypiv 3 weeks, 2 days ago
Add a 'serial' sysfs attribute for SCSI and SATA devices. This attribute
exposes the Unit Serial Number, which is derived from the Device
Identification Vital Product Data (VPD) page 0x80.

Whitespace is stripped from the retrieved serial number to handle
the different alignment (right-aligned for SCSI, potentially
left-aligned for SATA). As noted in SAT-5 10.5.3, "Although SPC-5 defines
the PRODUCT SERIAL NUMBER field as right-aligned, ACS-5 does not require
its SERIAL NUMBER field to be right-aligned. Therefore, right-alignment
of the PRODUCT SERIAL NUMBER field for the translation is not assured."

This attribute is used by tools such as lsblk to display the serial
number of block devices.

Signed-off-by: Igor Pylypiv <ipylypiv@google.com>
---
 drivers/scsi/scsi_lib.c    | 53 ++++++++++++++++++++++++++++++++++++++
 drivers/scsi/scsi_sysfs.c  | 14 ++++++++++
 include/scsi/scsi_device.h |  1 +
 3 files changed, 68 insertions(+)

diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index 93031326ac3e..dc09785c050c 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -13,6 +13,7 @@
 #include <linux/bitops.h>
 #include <linux/blkdev.h>
 #include <linux/completion.h>
+#include <linux/ctype.h>
 #include <linux/kernel.h>
 #include <linux/export.h>
 #include <linux/init.h>
@@ -3451,6 +3452,58 @@ int scsi_vpd_lun_id(struct scsi_device *sdev, char *id, size_t id_len)
 }
 EXPORT_SYMBOL(scsi_vpd_lun_id);
 
+/**
+ * scsi_vpd_lun_serial - return a unique device serial number
+ * @sdev: SCSI device
+ * @sn:   buffer for the serial number
+ * @sn_size: size of the buffer
+ *
+ * Copies the device serial number into @sn based on the information in
+ * the VPD page 0x80 of the device. The string will be null terminated
+ * and have leading and trailing whitespace stripped.
+ *
+ * Returns the length of the serial number or error on failure.
+ */
+int scsi_vpd_lun_serial(struct scsi_device *sdev, char *sn, size_t sn_size)
+{
+	int len;
+	const unsigned char *d;
+	const struct scsi_vpd *vpd_pg80;
+
+	rcu_read_lock();
+	vpd_pg80 = rcu_dereference(sdev->vpd_pg80);
+	if (!vpd_pg80) {
+		rcu_read_unlock();
+		return -ENXIO;
+	}
+
+	len = vpd_pg80->len - 4;
+	d = vpd_pg80->data + 4;
+
+	/* Skip leading spaces */
+	while (len > 0 && isspace(*d)) {
+		len--;
+		d++;
+	}
+
+	/* Skip trailing spaces */
+	while (len > 0 && isspace(d[len - 1]))
+		len--;
+
+	if (sn_size < len + 1) {
+		rcu_read_unlock();
+		return -EINVAL;
+	}
+
+	memcpy(sn, d, len);
+	sn[len] = '\0';
+
+	rcu_read_unlock();
+
+	return len;
+}
+EXPORT_SYMBOL(scsi_vpd_lun_serial);
+
 /**
  * scsi_vpd_tpg_id - return a target port group identifier
  * @sdev: SCSI device
diff --git a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c
index 99eb0a30df61..d80a546f54c2 100644
--- a/drivers/scsi/scsi_sysfs.c
+++ b/drivers/scsi/scsi_sysfs.c
@@ -1013,6 +1013,19 @@ sdev_show_wwid(struct device *dev, struct device_attribute *attr,
 }
 static DEVICE_ATTR(wwid, S_IRUGO, sdev_show_wwid, NULL);
 
+static ssize_t
+sdev_show_serial(struct device *dev, struct device_attribute *attr, char *buf)
+{
+	struct scsi_device *sdev = to_scsi_device(dev);
+	ssize_t ret;
+
+	ret = scsi_vpd_lun_serial(sdev, buf, PAGE_SIZE);
+	if (ret < 0)
+		return ret;
+	return sysfs_emit(buf, "%s\n", buf);
+}
+static DEVICE_ATTR(serial, S_IRUGO, sdev_show_serial, NULL);
+
 #define BLIST_FLAG_NAME(name)					\
 	[const_ilog2((__force __u64)BLIST_##name)] = #name
 static const char *const sdev_bflags_name[] = {
@@ -1257,6 +1270,7 @@ static struct attribute *scsi_sdev_attrs[] = {
 	&dev_attr_device_busy.attr,
 	&dev_attr_vendor.attr,
 	&dev_attr_model.attr,
+	&dev_attr_serial.attr,
 	&dev_attr_rev.attr,
 	&dev_attr_rescan.attr,
 	&dev_attr_delete.attr,
diff --git a/include/scsi/scsi_device.h b/include/scsi/scsi_device.h
index d32f5841f4f8..9c2a7bbe5891 100644
--- a/include/scsi/scsi_device.h
+++ b/include/scsi/scsi_device.h
@@ -571,6 +571,7 @@ void scsi_put_internal_cmd(struct scsi_cmnd *scmd);
 extern void sdev_disable_disk_events(struct scsi_device *sdev);
 extern void sdev_enable_disk_events(struct scsi_device *sdev);
 extern int scsi_vpd_lun_id(struct scsi_device *, char *, size_t);
+extern int scsi_vpd_lun_serial(struct scsi_device *, char *, size_t);
 extern int scsi_vpd_tpg_id(struct scsi_device *, int *);
 
 #ifdef CONFIG_PM
-- 
2.52.0.457.g6b5491de43-goog
Re: [PATCH] scsi: core: Add 'serial' sysfs attribute for SCSI/SATA
Posted by Bart Van Assche 3 weeks, 2 days ago
On 1/14/26 10:51 AM, Igor Pylypiv wrote:
> Add a 'serial' sysfs attribute for SCSI and SATA devices. This attribute
> exposes the Unit Serial Number, which is derived from the Device
> Identification Vital Product Data (VPD) page 0x80.
> 
> Whitespace is stripped from the retrieved serial number to handle
> the different alignment (right-aligned for SCSI, potentially
> left-aligned for SATA). As noted in SAT-5 10.5.3, "Although SPC-5 defines
> the PRODUCT SERIAL NUMBER field as right-aligned, ACS-5 does not require
> its SERIAL NUMBER field to be right-aligned. Therefore, right-alignment
> of the PRODUCT SERIAL NUMBER field for the translation is not assured."
> 
> This attribute is used by tools such as lsblk to display the serial
> number of block devices.

How can existing user space tools use a sysfs attribute that has not yet
been implemented? Please explain.

> +int scsi_vpd_lun_serial(struct scsi_device *sdev, char *sn, size_t sn_size)
> +{
> +	int len;
> +	const unsigned char *d;
> +	const struct scsi_vpd *vpd_pg80;

The current convention for declarations in Linux kernel code is to order
these from longest to shortest. In other words, the opposite order of
the above order.

> +	rcu_read_lock();

Please use guard(rcu)() in new code.

> +	vpd_pg80 = rcu_dereference(sdev->vpd_pg80);
> +	if (!vpd_pg80) {
> +		rcu_read_unlock();
> +		return -ENXIO;
> +	}
> +
> +	len = vpd_pg80->len - 4;
> +	d = vpd_pg80->data + 4;
> +
> +	/* Skip leading spaces */
> +	while (len > 0 && isspace(*d)) {
> +		len--;
> +		d++;
> +	}
> +
> +	/* Skip trailing spaces */
> +	while (len > 0 && isspace(d[len - 1]))
> +		len--;
> +
> +	if (sn_size < len + 1) {
> +		rcu_read_unlock();
> +		return -EINVAL;
> +	}

Has it been considered to call strim() instead of implementing 
functionality that is very similar to strim()?

> +	return sysfs_emit(buf, "%s\n", buf);

The C99 standard says that passing the output buffer pointer as an 
argument to sprintf()/snprintf() triggers undefined behavior. I'm not 
sure whether this also applies to the kernel equivalents of these
functions but it's probably better to be careful.

Thanks,

Bart.
Re: [PATCH] scsi: core: Add 'serial' sysfs attribute for SCSI/SATA
Posted by Igor Pylypiv 3 weeks, 1 day ago
On Wed, Jan 14, 2026 at 12:40:16PM -0800, Bart Van Assche wrote:
> On 1/14/26 10:51 AM, Igor Pylypiv wrote:
> > Add a 'serial' sysfs attribute for SCSI and SATA devices. This attribute
> > exposes the Unit Serial Number, which is derived from the Device
> > Identification Vital Product Data (VPD) page 0x80.
> > 
> > Whitespace is stripped from the retrieved serial number to handle
> > the different alignment (right-aligned for SCSI, potentially
> > left-aligned for SATA). As noted in SAT-5 10.5.3, "Although SPC-5 defines
> > the PRODUCT SERIAL NUMBER field as right-aligned, ACS-5 does not require
> > its SERIAL NUMBER field to be right-aligned. Therefore, right-alignment
> > of the PRODUCT SERIAL NUMBER field for the translation is not assured."
> > 
> > This attribute is used by tools such as lsblk to display the serial
> > number of block devices.
> 
> How can existing user space tools use a sysfs attribute that has not yet
> been implemented? Please explain.


The 'serial' sysfs attribute is implemented for NVMe. Since 'serial' sysfs
attribute is missing for SCSI/SATA devices, lsblk reports an empty string
instead of a serial number.

# lsblk --nodeps -o name,serial
NAME    SERIAL
sda     
sdb     

> 
> > +int scsi_vpd_lun_serial(struct scsi_device *sdev, char *sn, size_t sn_size)
> > +{
> > +	int len;
> > +	const unsigned char *d;
> > +	const struct scsi_vpd *vpd_pg80;
> 
> The current convention for declarations in Linux kernel code is to order
> these from longest to shortest. In other words, the opposite order of
> the above order.
>

Thanks for pointing this out. I'll reorder the declarations in V2.
 
> > +	rcu_read_lock();
> 
> Please use guard(rcu)() in new code.

Thanks. I'll fix this in V2.
 
> > +	vpd_pg80 = rcu_dereference(sdev->vpd_pg80);
> > +	if (!vpd_pg80) {
> > +		rcu_read_unlock();
> > +		return -ENXIO;
> > +	}
> > +
> > +	len = vpd_pg80->len - 4;
> > +	d = vpd_pg80->data + 4;
> > +
> > +	/* Skip leading spaces */
> > +	while (len > 0 && isspace(*d)) {
> > +		len--;
> > +		d++;
> > +	}
> > +
> > +	/* Skip trailing spaces */
> > +	while (len > 0 && isspace(d[len - 1]))
> > +		len--;
> > +
> > +	if (sn_size < len + 1) {
> > +		rcu_read_unlock();
> > +		return -EINVAL;
> > +	}
> 
> Has it been considered to call strim() instead of implementing functionality
> that is very similar to strim()?

Yes, I considered using strim(). strim() modifies the input buffer by
replacing first trailing whitespace with '\0' so we can't use it directly
on the vpd_pg80->data. The solution would be to copy the whole vpd page
data into the sn buffer and call strim() on the sn buffer. strim() returns
a pointer to the first non-whitespace character so we would also need to
memmove the serial number to the beginning of the sn buffer. All this extra
copying seems to be redundant so I went ahead with a simpler solution
that does a single memcpy().

> > +	return sysfs_emit(buf, "%s\n", buf);
> 
> The C99 standard says that passing the output buffer pointer as an argument
> to sprintf()/snprintf() triggers undefined behavior. I'm not sure whether
> this also applies to the kernel equivalents of these
> functions but it's probably better to be careful.

Kernel implementation of sysfs_emit() seems to be fine. The documentation
states that sysfs_emit()/sysfs_emit_at() should be used for show() methods.

https://github.com/torvalds/linux/blob/603c05a1639f60e0c52c5fdd25cf5e0b44b9bd8e/Documentation/filesystems/sysfs.rst?plain=1#L246-L247

Thanks,
Igor

> Thanks,
> 
> Bart.