linux-kernel - Re: [PATCH] PCI: s390: Expose the UID as an arch specific PCI slot attribute

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 for Android: free password hash cracker in your pocket

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

Message-ID: <0a3936872d93511ac76cba04edc47f598950d7ee.camel@linux.ibm.com>
Date: Wed, 08 Oct 2025 13:05:13 +0200
From: Niklas Schnelle <schnelle@...ux.ibm.com>
To: Benjamin Block <bblock@...ux.ibm.com>
Cc: Bjorn Helgaas <bhelgaas@...gle.com>, Lukas Wunner <lukas@...ner.de>,
        Gerald Schaefer <gerald.schaefer@...ux.ibm.com>,
        Christian Borntraeger
 <borntraeger@...ux.ibm.com>,
        Gerd Bayer	 <gbayer@...ux.ibm.com>,
        Matthew
 Rosato <mjrosato@...ux.ibm.com>,
        Heiko Carstens	 <hca@...ux.ibm.com>, Vasily Gorbik <gor@...ux.ibm.com>,
        Alexander Gordeev	
 <agordeev@...ux.ibm.com>,
        Sven Schnelle <svens@...ux.ibm.com>,
        Julian Ruess
	 <julianr@...ux.ibm.com>,
        Peter Oberparleiter <oberpar@...ux.ibm.com>,
        Ramesh Errabolu <ramesh@...ux.ibm.com>, linux-s390@...r.kernel.org,
        linux-kernel@...r.kernel.org, linux-pci@...r.kernel.org
Subject: Re: [PATCH] PCI: s390: Expose the UID as an arch specific PCI slot
 attribute

On Tue, 2025-09-30 at 11:09 +0200, Benjamin Block wrote:
> On Fri, Sep 26, 2025 at 08:25:07PM +0200, Niklas Schnelle wrote:
> > On Fri, 2025-09-26 at 16:27 +0200, Benjamin Block wrote:
> > > > +extern const struct attribute_group zpci_slot_attr_group;
> > > > +
> > > > +#define ARCH_PCI_SLOT_GROUPS (&zpci_slot_attr_group)
> > > 
> > > I don't know the history exactly, but this can't be easily extended like the
> > > other group above `ARCH_PCI_DEV_GROUPS`.
> > > 
> > >     (&zpci_slot_attr_group,  \
> > >      &zpci_slot_attr_group_b)
> > > 
> > > Won't compile. The way `ARCH_PCI_DEV_GROUPS` does it, attaching a different
> > > group is just adding a new line.
> > 
> > Without the parenthesis it should. I only added them because otherwise
> > checkpatch complains and it's still valid for a single item to have
> > parenthesis.
> 
> It's not like checkpatch is the last arbiter of truth here, especially sind we
> already have a case without parenthesis; but I guess if we ever need to extend
> the list, we can remove the parenthesis again.

Yes that was my thought too.

> 
> > > > diff --git a/arch/s390/pci/pci_sysfs.c b/arch/s390/pci/pci_sysfs.c
> > > > index 0ee0924cfab7e5d22468fb197ee78cac679d8c13..997dff3796094680d9a3f0b6eb27a89fa1ed30b2 100644
> > > > --- a/arch/s390/pci/pci_sysfs.c
> > > > +++ b/arch/s390/pci/pci_sysfs.c
> > > > @@ -178,6 +178,17 @@ static ssize_t index_show(struct device *dev,
> > > >  }
> > > >  static DEVICE_ATTR_RO(index);
> > > >  
> > > > --- snip ---
> > > > +{
> > > > +	struct zpci_dev *zdev = container_of(slot->hotplug, struct zpci_dev,
> > > > +					     hotplug_slot);
> > > > +
> > > > +	return sysfs_emit(buf, "0x%x\n", zdev->uid);
> > > 
> > > Do we need a special case for when `uid` is 0? That would imply the uid is
> > > invalid, right? Would we want to return an error in that case (-EINVAL, or
> > > smth)? 
> > > 
> > > Also, do we want to use the same format as in `zpci_setup_bus_resources()`
> > > with the 4 leading 0's (`0x%04x`)?
> > 
> > I chose to match the "uid" device attribute here ("zpci_attr(uid,
> > "0x%x\n", uid)" in the beginning of the same file).
> 
> Ah right, fair enough.
> 
> > This doesn't
> > special case UID 0. You are right that this is an invalid UID though.
> > It also still exposes the UID even if it is not guaranteed to be
> > unique. But we'll make that setting known to user-space separately.
> > I feel like knowing the UIDs can still be helpful even when they are
> > not unique, for example to check that they've been set correctly from
> > within Linux before enabling UID Checking.
> 
> I don't mind the case where the UID is not checked for uniqueness (the naming
> is confusing in any case, since the U doesn't stand for unique), I was just
> wondering whether printing an invalid UID makes sense. I think those are
> distinct cases. 
> It might be easier to 'encode' this knowledge about an UID being "invalid"
> here, rather than 'encoding' it in every single user that might read this
> attribute.
> 
> I guess the same can be said for the old `uid` attribute that is attached to
> the PCI device, but that was introduced by Sebastian a long time ago.

My thought here is that any program that deals with this new uid
attribute would potentially also deal with the existing per-device
attribute. I'd not want to change the existing attribute so then I
think it is best if applications just have to deal with a single "UID
as '0x' prefixed hex with 0 meaning unset" encoding. It's also that
this invalid/unset case does happen as the machine configuration does
allow UIDs to be unset, so it's not really an error case.

Thanks,
Niklas