linux-kernel - Re: [RESEND PATCH v7 1/1] PCI/sysfs: Expose PCIe device serial number

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: <aMnmTMsUWwTwnlWV@kbusch-mbp>
Date: Tue, 16 Sep 2025 16:35:56 -0600
From: Keith Busch <kbusch@...nel.org>
To: Bjorn Helgaas <helgaas@...nel.org>
Cc: Matthew Wood <thepacketgeek@...il.com>,
	Bjorn Helgaas <bhelgaas@...gle.com>,
	Mario Limonciello <superm1@...nel.org>,
	Jonathan Cameron <Jonathan.Cameron@...wei.com>,
	Thomas Weißschuh <thomas.weissschuh@...utronix.de>,
	linux-kernel@...r.kernel.org, linux-pci@...r.kernel.org
Subject: Re: [RESEND PATCH v7 1/1] PCI/sysfs: Expose PCIe device serial number

On Mon, Sep 15, 2025 at 02:39:04PM -0500, Bjorn Helgaas wrote:
> > @@ -660,6 +677,7 @@ static struct attribute *pcie_dev_attrs[] = {
> >  	&dev_attr_current_link_width.attr,
> >  	&dev_attr_max_link_width.attr,
> >  	&dev_attr_max_link_speed.attr,
> > +	&dev_attr_serial_number.attr,
> 
> I can see that the PCI r3.0 (conventional PCI) spec doesn't include
> the Device Serial Number Capability and the PCIe spec does include it,
> but this seems like it would fit better in the pci_dev_dev_attrs[],
> and the visibility check would be parallel to the
> dev_attr_boot_vga.attr check there.

I'm not sure I agree. The pci_dev_dev_attrs apply to all pci devices,
but DSN only exists in PCIe Extended Capability space. Conventional pci
config requests couldn't even describe it, so seems okay to fence it off
using the PCI-Express attribute group that already has that visibility
barrier.

I also don't like Krzysztof's suggestion to make it visible even if we
know you can't read it. The exisiting attributes that behave that way
shouldn't do that, IMO. It's a waste of resources to provide a handle
just to say the capability doesn't exist when the handle could just not
exist instead.