[PATCH] PCI: pciehp: Fix NULL pointer dereference in pciehp_configure_device()

[PATCH] PCI: pciehp: Fix NULL pointer dereference in pciehp_configure_device()

[Priyank Rathod]

pciehp_configure_device() calls pci_get_dsn() on the result of
pci_get_slot(parent, PCI_DEVFN(0, 0)) without checking for NULL, unlike
the sibling call sites in pcie_init() and pciehp_device_replaced() added
by the same commit.  When hypervisor_isolated_pci_functions() is true
(e.g., Jailhouse or LoongArch) and a non-zero function is hot-added
without Function 0, pci_scan_slot() returns > 0 while pci_get_slot() for
00.0 returns NULL, causing pci_get_dsn() to dereference a NULL pointer
in pci_find_next_ext_capability().

Guard the pci_get_dsn() call against NULL and reset ctrl->dsn to 0 when
Function 0 is absent so ctrl->dsn does not retain a stale value from a
previously removed device.

Fixes: 9d573d19547b ("PCI: pciehp: Detect device replacement during system sleep")
Cc: stable@vger.kernel.org # v6.11+
Signed-off-by: Priyank Rathod <rathodpriyank@google.com>
---
Verification notes:
- Commit 9d573d19547b added 3 pci_get_slot(..., PCI_DEVFN(0, 0)) +
  pci_get_dsn() sites in pciehp: pcie_init() and
  pciehp_device_replaced() both guard against NULL, while
  pciehp_configure_device() omitted the check (the only unchecked
  pci_get_slot() call out of 21 across drivers/pci/).
- On bare metal (!hypervisor_isolated_pci_functions()), missing fn 0
  breaks out of pci_scan_slot() immediately with num == 0 (-ENODEV).
  When hypervisor_isolated_pci_functions() is true, pci_scan_slot()
  scans fn 1..7 and returns num > 0 even when 00.0 is absent, so
  pci_get_slot(parent, PCI_DEVFN(0, 0)) at line 75 returns NULL.
- Using "ctrl->dsn = dev ? pci_get_dsn(dev) : 0;" also avoids keeping a
  stale ctrl->dsn from a prior hotplug cycle since
  pciehp_unconfigure_device() does not clear ctrl->dsn.
---
 drivers/pci/hotplug/pciehp_pci.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/pci/hotplug/pciehp_pci.c b/drivers/pci/hotplug/pciehp_pci.c
index 65e50bee1a8c..5f9e8044ea5b 100644
--- a/drivers/pci/hotplug/pciehp_pci.c
+++ b/drivers/pci/hotplug/pciehp_pci.c
@@ -73,7 +73,7 @@ int pciehp_configure_device(struct controller *ctrl)
 	down_read_nested(&ctrl->reset_lock, ctrl->depth);
 
 	dev = pci_get_slot(parent, PCI_DEVFN(0, 0));
-	ctrl->dsn = pci_get_dsn(dev);
+	ctrl->dsn = dev ? pci_get_dsn(dev) : 0;
 	pci_dev_put(dev);
 
  out:

---
base-commit: cee9395acd8043be0644b25c34bfa86623f2b935
change-id: 20261012-pcie-hotplug-nullref-8f4a2c91b3e7

Best regards,
-- 
Priyank Rathod <rathodpriyank@google.com>

Re: [PATCH] PCI: pciehp: Fix NULL pointer dereference in pciehp_configure_device()

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] pciehp: spurious device teardown on resume for devices lacking Function 0
--

commit 26f6592be4bd1ee27e178fdf85798fa2b65a389a
Author: Priyank Rathod <rathodpriyank@google.com>

PCI: pciehp: Fix NULL pointer dereference in pciehp_configure_device()

This commit fixes a NULL pointer dereference in pciehp_configure_device()
when attempting to read the DSN of a newly hot-added device. By guarding the
pci_get_dsn() call against a missing Function 0, it allows devices lacking
Function 0 (such as those under hypervisor isolation) to be successfully
hot-added.

> diff --git a/drivers/pci/hotplug/pciehp_pci.c b/drivers/pci/hotplug/pciehp_pci.c
> index 65e50bee1a8c0..5f9e8044ea5be 100644
> --- a/drivers/pci/hotplug/pciehp_pci.c
> +++ b/drivers/pci/hotplug/pciehp_pci.c
> @@ -73,7 +73,7 @@ int pciehp_configure_device(struct controller *ctrl)
>  	down_read_nested(&ctrl->reset_lock, ctrl->depth);
>  
>  	dev = pci_get_slot(parent, PCI_DEVFN(0, 0));
> -	ctrl->dsn = pci_get_dsn(dev);
> +	ctrl->dsn = dev ? pci_get_dsn(dev) : 0;

[Severity: Medium]
While this allows devices without Function 0 to be successfully hot-added,
will it lead to spurious teardowns upon system resume?

When the system resumes, pciehp_resume_noirq() calls pciehp_device_replaced().

If a device legitimately lacks Function 0, pci_get_slot() for PCI_DEVFN(0, 0)
will return NULL:

drivers/pci/hotplug/pciehp_hpc.c:pciehp_device_replaced() {
    ...
	pdev = pci_get_slot(ctrl->pcie->port->subordinate, PCI_DEVFN(0, 0));
	if (!pdev)
		return true;
    ...
}

Because it unconditionally returns true when Function 0 is absent, this false
replacement signal will trigger pci_walk_bus() to set the disconnected state
and queue a teardown request.

Could this spurious teardown issue be avoided by modifying
pciehp_device_replaced() to handle missing Function 0 devices correctly?

>  	pci_dev_put(dev);
>  
>   out:

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261012-pcie-hotplug-nullref-v1-1-6e21c2e89ea4@google.com?part=1