[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()

[Priyank Rathod]

On Mon, Oct 12, 2026 at 10:16:29PM +0000, sashiko-bot@kernel.org wrote:
> While this allows devices without Function 0 to be successfully hot-added,
> will it lead to spurious teardowns upon system resume?
...
> Could this spurious teardown issue be avoided by modifying
> pciehp_device_replaced() to handle missing Function 0 devices correctly?

Hi Bjorn, Lukas,

Gentle ping on this fix now that v7.4-rc1 is out (the patch still
applies cleanly to v7.4-rc1 and pci/next).

Regarding the bot's question above: pciehp assumes Function 0
(PCI_DEVFN(0, 0)) represents an occupied slot across all four of its
lookup sites -- the -EEXIST check at the top of
pciehp_configure_device(), pcie_init(), pciehp_device_replaced(), and
the DSN read at the end of pciehp_configure_device() (consistent with
PCIe r6.3 sec 7.9.3 requiring Multi-Function Devices implementing DSN to
expose it on Function 0).

I intentionally kept this patch scoped strictly to the missing NULL
check in pciehp_configure_device() -- the only unchecked pci_get_slot()
call in pciehp -- rather than mixing a broader change to pciehp's
pre-existing Function 0 behavior into a minimal NULL-dereference fix.

Please let me know which direction you prefer: taking this minimal NULL
check fix standalone as-is, and whether you think handling isolated
non-zero functions in pciehp_device_replaced() (and the other
PCI_DEVFN(0, 0) sites) is worth pursuing as a separate follow-up or best
left anchored to Function 0.

Thanks,
Priyank

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

[Lukas Wunner]

On Mon, Nov 09, 2026 at 09:53:38PM +0000, Priyank Rathod wrote:
> On Mon, Oct 12, 2026 at 10:16:29PM +0000, sashiko-bot@kernel.org wrote:
> > While this allows devices without Function 0 to be successfully hot-added,
> > will it lead to spurious teardowns upon system resume?
> ...
> > Could this spurious teardown issue be avoided by modifying
> > pciehp_device_replaced() to handle missing Function 0 devices correctly?
[...]
> Regarding the bot's question above: pciehp assumes Function 0
> (PCI_DEVFN(0, 0)) represents an occupied slot across all four of its
> lookup sites -- the -EEXIST check at the top of
> pciehp_configure_device(), pcie_init(), pciehp_device_replaced(), and
> the DSN read at the end of pciehp_configure_device() (consistent with
> PCIe r6.3 sec 7.9.3 requiring Multi-Function Devices implementing DSN to
> expose it on Function 0).

I'm not even sure whether pciehp is actually used with jailhouse,
s390 or loongarch.  pciehp assumes in multiple places that function 0
is present, so guarding against a missing function 0 in a single place
doesn't really make sense.

Note that there's a patch pending which moves the cached dsn from
pciehp's struct controller to struct pci_dev, so the days are numbered
for the lines you're changing:

https://lore.kernel.org/r/20260902133933.2992457-7-terry.bowman@amd.com/

Are you actually observing a crash in practice because of a missing NULL
pointer check?

Thanks,

Lukas

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

[Niklas Schnelle]

On Tue, 2026-11-10 at 11:04 +0100, Lukas Wunner wrote:
> On Mon, Nov 09, 2026 at 09:53:38PM +0000, Priyank Rathod wrote:
> > On Mon, Oct 12, 2026 at 10:16:29PM +0000, sashiko-bot@kernel.org wrote:
> > > While this allows devices without Function 0 to be successfully hot-added,
> > > will it lead to spurious teardowns upon system resume?
> > ...
> > > Could this spurious teardown issue be avoided by modifying
> > > pciehp_device_replaced() to handle missing Function 0 devices correctly?
> [...]
> > Regarding the bot's question above: pciehp assumes Function 0
> > (PCI_DEVFN(0, 0)) represents an occupied slot across all four of its
> > lookup sites -- the -EEXIST check at the top of
> > pciehp_configure_device(), pcie_init(), pciehp_device_replaced(), and
> > the DSN read at the end of pciehp_configure_device() (consistent with
> > PCIe r6.3 sec 7.9.3 requiring Multi-Function Devices implementing DSN to
> > expose it on Function 0).
> 
> I'm not even sure whether pciehp is actually used with jailhouse,
> s390 or loongarch.  pciehp assumes in multiple places that function 0
> is present, so guarding against a missing function 0 in a single place
> doesn't really make sense.
> 
> Note that there's a patch pending which moves the cached dsn from
> pciehp's struct controller to struct pci_dev, so the days are numbered
> for the lines you're changing:
> 
> https://lore.kernel.org/r/20260902133933.2992457-7-terry.bowman@amd.com/
> 
> Are you actually observing a crash in practice because of a missing NULL
> pointer check?
> 
> Thanks,
> 
> Lukas

FWIW we (s390) do not use pciehp we use our own s390_pci_hpc for
hotplug. I'd be interested if this comes out of a real issue or just an
LLM worry too though.

Thanks,
Niklas