linux-kernel - Re: [PATCH] PCI: Adjust pci_sysfs_init() to device

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: <20240807201606.GA109435@bhelgaas>
Date: Wed, 7 Aug 2024 15:16:06 -0500
From: Bjorn Helgaas <helgaas@...nel.org>
To: Ian <4dark@...look.com>
Cc: bhelgaas@...gle.com, linux-pci@...r.kernel.org,
	linux-kernel@...r.kernel.org,
	Krzysztof Wilczyński <kw@...ux.com>
Subject: Re: [PATCH] PCI: Adjust pci_sysfs_init() to device_initcall

[+cc Krzysztof]

On Wed, Aug 07, 2024 at 06:34:34PM +0800, Ian wrote:
> From: Ian Ding <4dark@...look.com>
> 
> When the controller driver uses async probe
> (.probe_type = PROBE_PREFER_ASYNCHRONOUS), pci_host_probe() is not
> guaranteed to run before pci_sysfs_init(), kernel may call
> pci_create_sysfs_dev_files() twice in pci_sysfs_init() and
> pci_host_probe() -> pci_bus_add_device(), and dump stack:
> 
>     sysfs: cannot create duplicate filename
> 
> Signed-off-by: Ian Ding <4dark@...look.com>
> ---
>  drivers/pci/pci-sysfs.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c
> index dd0d9d9bc..bef25fecb 100644
> --- a/drivers/pci/pci-sysfs.c
> +++ b/drivers/pci/pci-sysfs.c
> @@ -1534,7 +1534,7 @@ static int __init pci_sysfs_init(void)
>  
>  	return 0;
>  }
> -late_initcall(pci_sysfs_init);
> +device_initcall(pci_sysfs_init);

This is certainly a problem that needs to be solved, but I don't think
this approach is necessarily safe.

There's a long discussion about some of the issues at
https://lore.kernel.org/linux-pci/20200716110423.xtfyb3n6tn5ixedh@pali/t/#u

The goal is to remove pci_sysfs_init() completely, but we haven't
quite got there yet.

Since that thread, Krzysztof has made great progress by converting
most sysfs files to static attributes, e.g.,

  506140f9c06b ("PCI/sysfs: Convert "index", "acpi_index", "label" to static attributes")
  d93f8399053d ("PCI/sysfs: Convert "vpd" to static attribute")
  f42c35ea3b13 ("PCI/sysfs: Convert "reset" to static attribute")
  527139d738d7 ("PCI/sysfs: Convert "rom" to static attribute")
  e1d3f3268b0e ("PCI/sysfs: Convert "config" to static attribute")

but there are still a couple things left.

>  static struct attribute *pci_dev_dev_attrs[] = {
>  	&dev_attr_boot_vga.attr,
> -- 
> 2.25.1
>