Re: [PATCH v8 01/25] PCI/sysfs: Use PCI resource accessor macros

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Lockless access to the global `vga_default` pointer leads to a Use-After-Free.
--

commit be1111a7de2552597e9d80551a60eee90c47b957
Author: Krzysztof Wilczyński <kwilczynski@kernel.org>

PCI/sysfs: Use PCI resource accessor macros

Replace direct pdev->resource[] accesses with pci_resource_n(),
and pdev->resource[].flags accesses with pci_resource_flags().

No functional changes intended.

Tested-by: Shivaprasad G Bhat <sbhat@linux.ibm.com>
Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
Signed-off-by: Krzysztof Wilczyński <kwilczynski@kernel.org>

> diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c
> index d37860841260c..1fbc3daf87cc5 100644
> --- a/drivers/pci/pci-sysfs.c
> +++ b/drivers/pci/pci-sysfs.c

[ ... ]

> @@ -689,7 +689,7 @@ static ssize_t boot_vga_show(struct device *dev, struct device_attribute *attr,
>  		return sysfs_emit(buf, "%u\n", (pdev == vga_dev));
>  
>  	return sysfs_emit(buf, "%u\n",
> -			  !!(pdev->resource[PCI_ROM_RESOURCE].flags &
> +			  !!(pci_resource_flags(pdev, PCI_ROM_RESOURCE) &
>  			     IORESOURCE_ROM_SHADOW));

[Severity: High]
This isn't a bug introduced by this patch, but I noticed a lockless access
pattern here involving vga_default_device() that could lead to a Use-After-Free
elsewhere in the code.

For example, in drivers/pci/vgaarb.c:vga_arb_write():

    if (!strncmp(curr_pos, "default", 7))
        pdev = pci_dev_get(vga_default_device());

Could this race with device removal? If vga_arbiter_del_pci_device() takes
vga_lock, updates vga_default to NULL, and drops its reference with
pci_dev_put():

    void vga_set_default_device(struct pci_dev *pdev)
    {
        if (vga_default == pdev)
            return;

        pci_dev_put(vga_default);
        vga_default = pci_dev_get(pdev);
    }

If the thread executing vga_arb_write() reads vga_default but is preempted
before incrementing the reference count in pci_dev_get(), couldn't the device
be entirely freed before pci_dev_get() executes, leading to a Use-After-Free?

>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260619085200.3729431-1-kwilczynski@kernel.org?part=1