* [PATCH v3] scsi: core: Add 'serial' sysfs attribute for SCSI/SATA
@ 2026-02-09 21:21 Igor Pylypiv
2026-02-09 21:36 ` Bart Van Assche
` (3 more replies)
0 siblings, 4 replies; 9+ messages in thread
From: Igor Pylypiv @ 2026-02-09 21:21 UTC (permalink / raw)
To: James E.J. Bottomley, Martin K. Petersen
Cc: Bart Van Assche, linux-scsi, linux-ide, linux-kernel,
Igor Pylypiv
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>
---
v2->v3 changes:
- Replaced sysfs_emit(buf, "%s\n", buf) with a manual newline placement
to avoid undefined behavior of passing the output buffer as an input.
v1->v2 changes:
- Reordered declarations in scsi_vpd_lun_serial() from longest to shortest.
- Replaced rcu_read_lock()/rcu_read_unlock() with guard(rcu)().
drivers/scsi/scsi_lib.c | 47 ++++++++++++++++++++++++++++++++++++++
drivers/scsi/scsi_sysfs.c | 16 +++++++++++++
include/scsi/scsi_device.h | 1 +
3 files changed, 64 insertions(+)
diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index 4a902c9dfd8b..c17fbe4dd845 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>
@@ -3459,6 +3460,52 @@ 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)
+{
+ const struct scsi_vpd *vpd_pg80;
+ const unsigned char *d;
+ int len;
+
+ guard(rcu)();
+ vpd_pg80 = rcu_dereference(sdev->vpd_pg80);
+ if (!vpd_pg80)
+ 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)
+ return -EINVAL;
+
+ memcpy(sn, d, len);
+ sn[len] = '\0';
+
+ 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..9c4f47e7a298 100644
--- a/drivers/scsi/scsi_sysfs.c
+++ b/drivers/scsi/scsi_sysfs.c
@@ -1013,6 +1013,21 @@ 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;
+
+ buf[ret] = '\n';
+ return ret + 1;
+}
+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 +1272,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.53.0.rc2.204.g2597b5adb4-goog
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH v3] scsi: core: Add 'serial' sysfs attribute for SCSI/SATA
2026-02-09 21:21 [PATCH v3] scsi: core: Add 'serial' sysfs attribute for SCSI/SATA Igor Pylypiv
@ 2026-02-09 21:36 ` Bart Van Assche
2026-02-10 11:38 ` Hannes Reinecke
` (2 subsequent siblings)
3 siblings, 0 replies; 9+ messages in thread
From: Bart Van Assche @ 2026-02-09 21:36 UTC (permalink / raw)
To: Igor Pylypiv, James E.J. Bottomley, Martin K. Petersen
Cc: linux-scsi, linux-ide, linux-kernel
On 2/9/26 1:21 PM, 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.
Reviewed-by: Bart Van Assche <bvanassche@acm.org>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v3] scsi: core: Add 'serial' sysfs attribute for SCSI/SATA
2026-02-09 21:21 [PATCH v3] scsi: core: Add 'serial' sysfs attribute for SCSI/SATA Igor Pylypiv
2026-02-09 21:36 ` Bart Van Assche
@ 2026-02-10 11:38 ` Hannes Reinecke
2026-02-10 16:51 ` Igor Pylypiv
2026-03-01 1:06 ` Martin K. Petersen
2026-03-11 2:06 ` Martin K. Petersen
3 siblings, 1 reply; 9+ messages in thread
From: Hannes Reinecke @ 2026-02-10 11:38 UTC (permalink / raw)
To: Igor Pylypiv, James E.J. Bottomley, Martin K. Petersen
Cc: Bart Van Assche, linux-scsi, linux-ide, linux-kernel
On 2/9/26 22:21, 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.
>
> Signed-off-by: Igor Pylypiv <ipylypiv@google.com>
> ---
>
> v2->v3 changes:
> - Replaced sysfs_emit(buf, "%s\n", buf) with a manual newline placement
> to avoid undefined behavior of passing the output buffer as an input.
>
> v1->v2 changes:
> - Reordered declarations in scsi_vpd_lun_serial() from longest to shortest.
> - Replaced rcu_read_lock()/rcu_read_unlock() with guard(rcu)().
>
>
> drivers/scsi/scsi_lib.c | 47 ++++++++++++++++++++++++++++++++++++++
> drivers/scsi/scsi_sysfs.c | 16 +++++++++++++
> include/scsi/scsi_device.h | 1 +
> 3 files changed, 64 insertions(+)
>
> diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
> index 4a902c9dfd8b..c17fbe4dd845 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>
> @@ -3459,6 +3460,52 @@ 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)
> +{
> + const struct scsi_vpd *vpd_pg80;
> + const unsigned char *d;
> + int len;
> +
> + guard(rcu)();
> + vpd_pg80 = rcu_dereference(sdev->vpd_pg80);
> + if (!vpd_pg80)
> + 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--;
> +
Please use 'strim()' instead.
> + if (sn_size < len + 1)
> + return -EINVAL;
> +
> + memcpy(sn, d, len);
'len' might well be '0' after 'strim()', please check
before calling 'memcpy'.
> + sn[len] = '\0';
> +
> + 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..9c4f47e7a298 100644
> --- a/drivers/scsi/scsi_sysfs.c
> +++ b/drivers/scsi/scsi_sysfs.c
> @@ -1013,6 +1013,21 @@ 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;
> +
> + buf[ret] = '\n';
> + return ret + 1;
> +}
> +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 +1272,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
Otherwise looks okay.
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
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v3] scsi: core: Add 'serial' sysfs attribute for SCSI/SATA
2026-02-10 11:38 ` Hannes Reinecke
@ 2026-02-10 16:51 ` Igor Pylypiv
2026-02-17 16:58 ` Igor Pylypiv
2026-02-17 19:25 ` Bart Van Assche
0 siblings, 2 replies; 9+ messages in thread
From: Igor Pylypiv @ 2026-02-10 16:51 UTC (permalink / raw)
To: Hannes Reinecke
Cc: James E.J. Bottomley, Martin K. Petersen, Bart Van Assche,
linux-scsi, linux-ide, linux-kernel
On Tue, Feb 10, 2026 at 12:38:51PM +0100, Hannes Reinecke wrote:
> On 2/9/26 22:21, 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.
> >
> > Signed-off-by: Igor Pylypiv <ipylypiv@google.com>
> > ---
> >
> > v2->v3 changes:
> > - Replaced sysfs_emit(buf, "%s\n", buf) with a manual newline placement
> > to avoid undefined behavior of passing the output buffer as an input.
> >
> > v1->v2 changes:
> > - Reordered declarations in scsi_vpd_lun_serial() from longest to shortest.
> > - Replaced rcu_read_lock()/rcu_read_unlock() with guard(rcu)().
> >
> >
> > drivers/scsi/scsi_lib.c | 47 ++++++++++++++++++++++++++++++++++++++
> > drivers/scsi/scsi_sysfs.c | 16 +++++++++++++
> > include/scsi/scsi_device.h | 1 +
> > 3 files changed, 64 insertions(+)
> >
> > diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
> > index 4a902c9dfd8b..c17fbe4dd845 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>
> > @@ -3459,6 +3460,52 @@ 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)
> > +{
> > + const struct scsi_vpd *vpd_pg80;
> > + const unsigned char *d;
> > + int len;
> > +
> > + guard(rcu)();
> > + vpd_pg80 = rcu_dereference(sdev->vpd_pg80);
> > + if (!vpd_pg80)
> > + 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--;
> > +
>
> Please use 'strim()' instead.
Hi Hannes,
Bart pointed this out in V1 as well. I'll copy-paste my reply from V1:
"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()."
Please let me know your thoughts on this.
>
> > + if (sn_size < len + 1)
> > + return -EINVAL;
> > +
> > + memcpy(sn, d, len);
>
> 'len' might well be '0' after 'strim()', please check
> before calling 'memcpy'.
It looks like calling a memcpy() with zero length is a no-op. Is checking
for len > 0 really necessary in this case?
Thank you,
Igor
> > + sn[len] = '\0';
> > +
> > + 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..9c4f47e7a298 100644
> > --- a/drivers/scsi/scsi_sysfs.c
> > +++ b/drivers/scsi/scsi_sysfs.c
> > @@ -1013,6 +1013,21 @@ 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;
> > +
> > + buf[ret] = '\n';
> > + return ret + 1;
> > +}
> > +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 +1272,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
>
> Otherwise looks okay.
>
> 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
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v3] scsi: core: Add 'serial' sysfs attribute for SCSI/SATA
2026-02-10 16:51 ` Igor Pylypiv
@ 2026-02-17 16:58 ` Igor Pylypiv
2026-02-18 8:28 ` Hannes Reinecke
2026-02-17 19:25 ` Bart Van Assche
1 sibling, 1 reply; 9+ messages in thread
From: Igor Pylypiv @ 2026-02-17 16:58 UTC (permalink / raw)
To: Hannes Reinecke
Cc: James E.J. Bottomley, Martin K. Petersen, Bart Van Assche,
linux-scsi, linux-ide, linux-kernel
On Tue, Feb 10, 2026 at 08:51:24AM -0800, Igor Pylypiv wrote:
> On Tue, Feb 10, 2026 at 12:38:51PM +0100, Hannes Reinecke wrote:
> > On 2/9/26 22:21, 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.
> > >
> > > Signed-off-by: Igor Pylypiv <ipylypiv@google.com>
> > > ---
> > >
> > > v2->v3 changes:
> > > - Replaced sysfs_emit(buf, "%s\n", buf) with a manual newline placement
> > > to avoid undefined behavior of passing the output buffer as an input.
> > >
> > > v1->v2 changes:
> > > - Reordered declarations in scsi_vpd_lun_serial() from longest to shortest.
> > > - Replaced rcu_read_lock()/rcu_read_unlock() with guard(rcu)().
> > >
> > >
> > > drivers/scsi/scsi_lib.c | 47 ++++++++++++++++++++++++++++++++++++++
> > > drivers/scsi/scsi_sysfs.c | 16 +++++++++++++
> > > include/scsi/scsi_device.h | 1 +
> > > 3 files changed, 64 insertions(+)
> > >
> > > diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
> > > index 4a902c9dfd8b..c17fbe4dd845 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>
> > > @@ -3459,6 +3460,52 @@ 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)
> > > +{
> > > + const struct scsi_vpd *vpd_pg80;
> > > + const unsigned char *d;
> > > + int len;
> > > +
> > > + guard(rcu)();
> > > + vpd_pg80 = rcu_dereference(sdev->vpd_pg80);
> > > + if (!vpd_pg80)
> > > + 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--;
> > > +
> >
> > Please use 'strim()' instead.
>
> Hi Hannes,
>
> Bart pointed this out in V1 as well. I'll copy-paste my reply from V1:
>
> "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()."
>
> Please let me know your thoughts on this.
Hi Hannes,
Ping for a feedback.
Sending this in case my previous reply fell through the cracks.
Thank you!
Igor
>
> >
> > > + if (sn_size < len + 1)
> > > + return -EINVAL;
> > > +
> > > + memcpy(sn, d, len);
> >
> > 'len' might well be '0' after 'strim()', please check
> > before calling 'memcpy'.
>
> It looks like calling a memcpy() with zero length is a no-op. Is checking
> for len > 0 really necessary in this case?
>
> Thank you,
> Igor
>
> > > + sn[len] = '\0';
> > > +
> > > + 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..9c4f47e7a298 100644
> > > --- a/drivers/scsi/scsi_sysfs.c
> > > +++ b/drivers/scsi/scsi_sysfs.c
> > > @@ -1013,6 +1013,21 @@ 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;
> > > +
> > > + buf[ret] = '\n';
> > > + return ret + 1;
> > > +}
> > > +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 +1272,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
> >
> > Otherwise looks okay.
> >
> > 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
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v3] scsi: core: Add 'serial' sysfs attribute for SCSI/SATA
2026-02-10 16:51 ` Igor Pylypiv
2026-02-17 16:58 ` Igor Pylypiv
@ 2026-02-17 19:25 ` Bart Van Assche
1 sibling, 0 replies; 9+ messages in thread
From: Bart Van Assche @ 2026-02-17 19:25 UTC (permalink / raw)
To: Igor Pylypiv, Hannes Reinecke
Cc: James E.J. Bottomley, Martin K. Petersen, linux-scsi, linux-ide,
linux-kernel
On 2/10/26 8:51 AM, Igor Pylypiv wrote:
> On Tue, Feb 10, 2026 at 12:38:51PM +0100, Hannes Reinecke wrote:
>> On 2/9/26 22:21, Igor Pylypiv wrote:
>>> + if (sn_size < len + 1)
>>> + return -EINVAL;
>>> +
>>> + memcpy(sn, d, len);
>>
>> 'len' might well be '0' after 'strim()', please check
>> before calling 'memcpy'.
>
> It looks like calling a memcpy() with zero length is a no-op. Is checking
> for len > 0 really necessary in this case?
It seems to me that the Linux kernel memcpy() implementation handles len
== 0 fine. From lib/string.c:
void *memcpy(void *dest, const void *src, size_t count)
{
char *tmp = dest;
const char *s = src;
while (count--)
*tmp++ = *s++;
return dest;
}
EXPORT_SYMBOL(memcpy);
Bart.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v3] scsi: core: Add 'serial' sysfs attribute for SCSI/SATA
2026-02-17 16:58 ` Igor Pylypiv
@ 2026-02-18 8:28 ` Hannes Reinecke
0 siblings, 0 replies; 9+ messages in thread
From: Hannes Reinecke @ 2026-02-18 8:28 UTC (permalink / raw)
To: Igor Pylypiv
Cc: James E.J. Bottomley, Martin K. Petersen, Bart Van Assche,
linux-scsi, linux-ide, linux-kernel
On 2/17/26 17:58, Igor Pylypiv wrote:
> On Tue, Feb 10, 2026 at 08:51:24AM -0800, Igor Pylypiv wrote:
>> On Tue, Feb 10, 2026 at 12:38:51PM +0100, Hannes Reinecke wrote:
>>> On 2/9/26 22:21, 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.
>>>>
>>>> Signed-off-by: Igor Pylypiv <ipylypiv@google.com>
>>>> ---
>>>>
>>>> v2->v3 changes:
>>>> - Replaced sysfs_emit(buf, "%s\n", buf) with a manual newline placement
>>>> to avoid undefined behavior of passing the output buffer as an input.
>>>>
>>>> v1->v2 changes:
>>>> - Reordered declarations in scsi_vpd_lun_serial() from longest to shortest.
>>>> - Replaced rcu_read_lock()/rcu_read_unlock() with guard(rcu)().
>>>>
>>>>
>>>> drivers/scsi/scsi_lib.c | 47 ++++++++++++++++++++++++++++++++++++++
>>>> drivers/scsi/scsi_sysfs.c | 16 +++++++++++++
>>>> include/scsi/scsi_device.h | 1 +
>>>> 3 files changed, 64 insertions(+)
>>>>
>>>> diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
>>>> index 4a902c9dfd8b..c17fbe4dd845 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>
>>>> @@ -3459,6 +3460,52 @@ 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)
>>>> +{
>>>> + const struct scsi_vpd *vpd_pg80;
>>>> + const unsigned char *d;
>>>> + int len;
>>>> +
>>>> + guard(rcu)();
>>>> + vpd_pg80 = rcu_dereference(sdev->vpd_pg80);
>>>> + if (!vpd_pg80)
>>>> + 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--;
>>>> +
>>>
>>> Please use 'strim()' instead.
>>
>> Hi Hannes,
>>
>> Bart pointed this out in V1 as well. I'll copy-paste my reply from V1:
>>
>> "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()."
>>
>> Please let me know your thoughts on this.
>
> Hi Hannes,
>
> Ping for a feedback.
> Sending this in case my previous reply fell through the cracks.
>
Hmm. Okay, sounds reasonable.
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
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v3] scsi: core: Add 'serial' sysfs attribute for SCSI/SATA
2026-02-09 21:21 [PATCH v3] scsi: core: Add 'serial' sysfs attribute for SCSI/SATA Igor Pylypiv
2026-02-09 21:36 ` Bart Van Assche
2026-02-10 11:38 ` Hannes Reinecke
@ 2026-03-01 1:06 ` Martin K. Petersen
2026-03-11 2:06 ` Martin K. Petersen
3 siblings, 0 replies; 9+ messages in thread
From: Martin K. Petersen @ 2026-03-01 1:06 UTC (permalink / raw)
To: Igor Pylypiv
Cc: James E.J. Bottomley, Martin K. Petersen, Bart Van Assche,
linux-scsi, linux-ide, linux-kernel
Igor,
> 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.
Applied to 7.1/scsi-staging, thanks!
--
Martin K. Petersen
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v3] scsi: core: Add 'serial' sysfs attribute for SCSI/SATA
2026-02-09 21:21 [PATCH v3] scsi: core: Add 'serial' sysfs attribute for SCSI/SATA Igor Pylypiv
` (2 preceding siblings ...)
2026-03-01 1:06 ` Martin K. Petersen
@ 2026-03-11 2:06 ` Martin K. Petersen
3 siblings, 0 replies; 9+ messages in thread
From: Martin K. Petersen @ 2026-03-11 2:06 UTC (permalink / raw)
To: James E.J. Bottomley, Igor Pylypiv
Cc: Martin K . Petersen, Bart Van Assche, linux-scsi, linux-ide,
linux-kernel
On Mon, 09 Feb 2026 13:21:51 -0800, 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."
>
> [...]
Applied to 7.1/scsi-queue, thanks!
[1/1] scsi: core: Add 'serial' sysfs attribute for SCSI/SATA
https://git.kernel.org/mkp/scsi/c/94c125bafa00
--
Martin K. Petersen
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2026-03-11 2:06 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-02-09 21:21 [PATCH v3] scsi: core: Add 'serial' sysfs attribute for SCSI/SATA Igor Pylypiv
2026-02-09 21:36 ` Bart Van Assche
2026-02-10 11:38 ` Hannes Reinecke
2026-02-10 16:51 ` Igor Pylypiv
2026-02-17 16:58 ` Igor Pylypiv
2026-02-18 8:28 ` Hannes Reinecke
2026-02-17 19:25 ` Bart Van Assche
2026-03-01 1:06 ` Martin K. Petersen
2026-03-11 2:06 ` Martin K. Petersen
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox