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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <20250930090949.GA15786@p1gen4-pw042f0m.boeblingen.de.ibm.com>
Date: Tue, 30 Sep 2025 11:09:49 +0200
From: Benjamin Block <bblock@...ux.ibm.com>
To: Niklas Schnelle <schnelle@...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 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.

> > > 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.

-- 
Best Regards, Benjamin Block        /        Linux on IBM Z Kernel Development
IBM Deutschland Research & Development GmbH    /   https://www.ibm.com/privacy
Vors. Aufs.-R.: Wolfgang Wendt         /        Geschäftsführung: David Faller
Sitz der Ges.: Böblingen     /    Registergericht: AmtsG Stuttgart, HRB 243294