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: <6131694.LvFx2qVVIh@steina-w>
Date:   Thu, 16 Mar 2023 10:33:38 +0100
From:   Alexander Stein <alexander.stein@...tq-group.com>
To:     Bjorn Helgaas <bhelgaas@...gle.com>,
        Oliver Neukum <oneukum@...e.com>
Cc:     Korneliusz Osmenda <korneliuszo@...il.com>,
        linux-pci@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 1/1] Guard pci_create_sysfs_dev_files with atomic value

Hi Oliver,

Am Donnerstag, 16. März 2023, 10:23:54 CET schrieb Oliver Neukum:
> On 16.03.23 10:15, Alexander Stein wrote:
> > From: Korneliusz Osmenda <korneliuszo@...il.com>
> > 
> > On Gateworks Ventana there is a number of PCI devices and:
> >    - imx6_pcie_probe takes longer than start of late init
> >    - pci_sysfs_init sets up flag sysfs_initialized
> >    - pci_sysfs_init initializes already found devices
> >    - imx6_pcie_probe tries to reinitialize device
> > 
> > Link: https://bugzilla.kernel.org/show_bug.cgi?id=215515
> > 
> > Signed-off-by: Korneliusz Osmenda <korneliuszo@...il.com>
> > Signed-off-by: Alexander Stein <alexander.stein@...tq-group.com>
> > ---
> > 
> >   drivers/pci/pci-sysfs.c | 6 ++++++
> >   include/linux/pci.h     | 2 ++
> >   2 files changed, 8 insertions(+)
> > 
> > diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c
> > index dd0d9d9bc509..998e44716b6f 100644
> > --- a/drivers/pci/pci-sysfs.c
> > +++ b/drivers/pci/pci-sysfs.c
> > @@ -1497,6 +1497,9 @@ int __must_check pci_create_sysfs_dev_files(struct
> > pci_dev *pdev)> 
> >   	if (!sysfs_initialized)
> >   	
> >   		return -EACCES;
> > 
> > +	if (atomic_cmpxchg(&pdev->sysfs_init_cnt, 0, 1) == 1)
> > +		return 0;		/* already added */
> > +
> > 
> >   	return pci_create_resource_files(pdev);
> 
> This is very likely a bug. You are returning an error in the error
> case. Yet the flag stays.

Ah, you are right. This is something needed to address.

> And simply resetting it in the error case
> would be a race. There is something fishy in that design.

Admittedly 
I would like to get rid of these two pathes for creating sysfs files in the 
first place, but I do not know the pci subsystem very well.
IMHO for_each_pci_dev(pdev) in pci_sysfs_init is part of the problem as it 
unconditionally iterates over the bus, without any locks, thus creating sysfs 
files for each device added to the bus.
Any ideas?

Best regards,
Alexander
-- 
TQ-Systems GmbH | Mühlstraße 2, Gut Delling | 82229 Seefeld, Germany
Amtsgericht München, HRB 105018
Geschäftsführer: Detlef Schneider, Rüdiger Stahl, Stefan Schneider
http://www.tq-group.com/


Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ