linux-kernel - Re: [PATCH v3] PCI/DOE: Expose the DOE protocols via sysfs

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]

Date:   Thu, 10 Aug 2023 11:34:11 -0400
From:   Alistair Francis <alistair23@...il.com>
To:     Lukas Wunner <lukas@...ner.de>
Cc:     bhelgaas@...gle.com, linux-pci@...r.kernel.org,
        Jonathan.Cameron@...wei.com, alex.williamson@...hat.com,
        christian.koenig@....com, kch@...dia.com,
        gregkh@...uxfoundation.org, logang@...tatee.com,
        linux-kernel@...r.kernel.org,
        Alistair Francis <alistair.francis@....com>
Subject: Re: [PATCH v3] PCI/DOE: Expose the DOE protocols via sysfs

On Thu, Aug 10, 2023 at 3:34 AM Lukas Wunner <lukas@...ner.de> wrote:
>
> On Wed, Aug 09, 2023 at 07:28:51PM -0400, Alistair Francis wrote:
> > --- a/drivers/pci/pci-sysfs.c
> > +++ b/drivers/pci/pci-sysfs.c
> > @@ -1226,6 +1227,12 @@ static int pci_create_resource_files(struct pci_dev *pdev)
> >       int i;
> >       int retval;
> >
> > +#ifdef CONFIG_PCI_DOE
> > +     retval = doe_sysfs_init(pdev);
> > +     if (retval)
> > +             return retval;
> > +#endif
> > +
>
> The preferred way to expose PCI sysfs attributes nowadays is to add them
> to pci_dev_attr_groups[] and use the ->is_visible callback to check
> whether they're applicable to a particular pci_dev.  The alternative
> via pci_create_resource_files() has race conditions which I think
> still haven't been fixed. Bjorn recommended the ->is_visible approach
> in response to the most recent attempt to fix the race:
>
> https://lore.kernel.org/linux-pci/20230427161458.GA249886@bhelgaas/

The is_visible doen't seem to work in this case.

AFAIK is_visible only applies to the attributes under the group. Which
means that every PCIe device will see a `doe_protos` directory, no
matter if DOE is supported.

On top of that is_visible() is only called
(fs/sysfs/group.c:create_files()) if there are sub attrs, which we
don't have. We will possibly end up with some attributes, but all of
them are dynamic. So we still end up needing to call doe_sysfs_init()
anyway.

Alistair

>
> To avoid (most of) the #ifdefs, you may want to consider adding a
> doe-sysfs.c file like I've done for cma in this commit:
>
> https://github.com/l1k/linux/commit/b057e2fb0ee0
>
> Thanks,
>
> Lukas