linux-kernel - Re: [PATCH] scsi: core: Add 'serial' sysfs attribute for SCSI/SATA

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <ce30b2fe-8225-47fb-b581-251a1b9cd2cf@acm.org>
Date: Wed, 14 Jan 2026 12:40:16 -0800
From: Bart Van Assche <bvanassche@....org>
To: Igor Pylypiv <ipylypiv@...gle.com>,
 "James E.J. Bottomley" <James.Bottomley@...senPartnership.com>,
 "Martin K. Petersen" <martin.petersen@...cle.com>
Cc: linux-scsi@...r.kernel.org, linux-ide@...r.kernel.org,
 linux-kernel@...r.kernel.org
Subject: Re: [PATCH] scsi: core: Add 'serial' sysfs attribute for SCSI/SATA

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.