[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <d61a1830-d9d0-4697-b547-80106ce57023@suse.de>
Date: Tue, 10 Feb 2026 12:38:51 +0100
From: Hannes Reinecke <hare@...e.de>
To: Igor Pylypiv <ipylypiv@...gle.com>,
"James E.J. Bottomley" <James.Bottomley@...senPartnership.com>,
"Martin K. Petersen" <martin.petersen@...cle.com>
Cc: Bart Van Assche <bvanassche@....org>, linux-scsi@...r.kernel.org,
linux-ide@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3] scsi: core: Add 'serial' sysfs attribute for SCSI/SATA
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@...gle.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@...e.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
Powered by blists - more mailing lists