Re: [RFC PATCH 5/8] powerpc/pci: Serialize pcibios_bus_add_device()

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Potential NULL pointer dereference of `pdn` in `pseries_pcibios_bus_add_device`.
- [High] Potential NULL pointer dereference of the physical function's EEH device (`edev`) in `pseries_pcibios_bus_add_device`.
--

commit 7133056da4f8622a96d7696deea5dac9e5c2eeb4
Author: Pavol Sakac <sakacpav@amazon.de>

powerpc/pci: Serialize pcibios_bus_add_device()

This commit serializes the pcibios_bus_add_device platform hook dispatch
with an arch-local mutex to prevent race conditions during concurrent sibling
VF additions. This addresses unlocked list manipulations in the EEH PE tree.

> diff --git a/arch/powerpc/kernel/pci-common.c b/arch/powerpc/kernel/pci-common.c
> index 4fc52c21fe5d4..21cccd0e97f8b 100644
> --- a/arch/powerpc/kernel/pci-common.c
> +++ b/arch/powerpc/kernel/pci-common.c

[ ... ]

> @@ -1068,8 +1077,11 @@ void pcibios_bus_add_device(struct pci_dev *dev)
>  	if (ppc_md.pci_irq_fixup)
>  		ppc_md.pci_irq_fixup(dev);
>  
> -	if (ppc_md.pcibios_bus_add_device)
> +	if (ppc_md.pcibios_bus_add_device) {
> +		mutex_lock(&pcibios_bus_add_device_lock);
>  		ppc_md.pcibios_bus_add_device(dev);

[Severity: High]
This isn't a bug introduced by this patch, but can this hook lead to a NULL
pointer dereference if pdn allocation fails?

If add_sriov_vf_pdns() fails (for example, due to memory pressure), it
returns NULL instead of propagating an error. The PCI core proceeds to
instantiate the Virtual Functions and calls this platform hook.

In arch/powerpc/platforms/pseries/eeh_pseries.c:pseries_pcibios_bus_add_device():

    pdn = pci_get_pdn(pdev);
    pdn->device_id  =  pdev->device;

Could a privileged user trigger a kernel panic by enabling SR-IOV under
memory pressure, turning a recoverable failure into a DoS?

[Severity: High]
This is also a pre-existing issue, but could this hook dereference a NULL
pointer if the physical function lacks an initialized EEH device?

When adding a Virtual Function, pseries_pcibios_bus_add_device() assumes
the physical function's EEH device pointer is valid:

In arch/powerpc/platforms/pseries/eeh_pseries.c:pseries_pcibios_bus_add_device():

    struct eeh_pe *physfn_pe = pci_dev_to_eeh_dev(pdev->physfn)->pe;

If EEH initialization failed for the physical function (for instance, if the
device is not part of a PE or firmware didn't support EEH), its EEH device
pointer remains NULL.

Would writing to the sriov_numvfs sysfs file on such a physical function
cause an unconditional dereference of the NULL edev pointer here?

> +		mutex_unlock(&pcibios_bus_add_device_lock);
> +	}
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260911-vfopt-s1-v1-0-693271dc0226@amazon.de?part=5