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: <20250916145357.00007ebb@huawei.com>
Date: Tue, 16 Sep 2025 14:53:57 +0100
From: Jonathan Cameron <jonathan.cameron@...wei.com>
To: Keith Busch <kbusch@...nel.org>
CC: Krzysztof Wilczy´nski <kw@...ux.com>, Matthew Wood
	<thepacketgeek@...il.com>, Bjorn Helgaas <bhelgaas@...gle.com>, "Mario
 Limonciello" <superm1@...nel.org>, 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, 15 Sep 2025 16:23:07 -0600
Keith Busch <kbusch@...nel.org> wrote:

> On Sat, Sep 13, 2025 at 03:20:41PM +0900, Krzysztof Wilczy´nski wrote:
> > Hello,
> >   
> > > @@ -1749,10 +1767,13 @@ static umode_t pcie_dev_attrs_are_visible(struct kobject *kobj,
> > >  	struct device *dev = kobj_to_dev(kobj);
> > >  	struct pci_dev *pdev = to_pci_dev(dev);
> > >  
> > > -	if (pci_is_pcie(pdev))
> > > -		return a->mode;
> > > +	if (!pci_is_pcie(pdev))
> > > +		return 0;
> > >  
> > > -	return 0;
> > > +	if (a == &dev_attr_serial_number.attr && !pci_get_dsn(pdev))
> > > +		return 0;
> > > +
> > > +	return a->mode;  
> > 
> > It would be fine to have this sysfs attribute present all the time, and
> > simply return error when the serial number is not available.  Not sure if
> > hiding it adds a lot of value.  This is how some of the existing attributes
> > currently behave.
> > 
> > But it does add extra code to pcie_dev_attrs_are_visible() where it is now
> > a special case, somewhat.  
> 
> You bring up a good point, but I think it seems odd that the existing
> pcie attributes are visible even if we know reading it will fail.

Perhaps historical. The is_visible infrastructure is I think somewhat
newer than a lot of that ABI.

> Perhaps the pcie link status visibility should be changed to follow this
> patch's example to hide when they don't exist. Applications might notice
> a different error, ENOENT vs EINVAL, if the device doesn't support the
> capability, but that is a more accurate errno.

As it is sysfs we can never be sure someone hasn't assumed existing files
are present even when they aren't useful.

So I doubt we can improve the existing cases without something breaking.
If we want to give it a go and see who screams I'm fine with that :)

Jonathan