Re: [PATCH] PCI: pciehp: Re-read Slot Control when cached HPIE is clear

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] pci: TOCTOU race in config space read leading to master abort
--

commit 7ceb468446bd25d57acbe78d0b28b124ca7b5902
Author: Zhu Qiyu <qiyuzhu2@amd.com>

PCI: pciehp: Re-read Slot Control when cached HPIE is clear

This commit addresses missed hot-plug events by re-reading the Slot
Control register directly from hardware when the cached Hot-Plug
Interrupt Enable (HPIE) bit is clear and the port is in the D0 state.

> diff --git a/drivers/pci/hotplug/pciehp_hpc.c b/drivers/pci/hotplug/pciehp_hpc.c
> index 4c62140a3cb44..df977d4af4056 100644
> --- a/drivers/pci/hotplug/pciehp_hpc.c
> +++ b/drivers/pci/hotplug/pciehp_hpc.c

[ ... ]

> @@ -628,13 +628,34 @@ static irqreturn_t pciehp_isr(int irq, void *dev_id)
>  	u16 status, events = 0;
>  
>  	/*
> -	 * Interrupts only occur in D3hot or shallower and only if enabled
> -	 * in the Slot Control register (PCIe r4.0, sec 6.7.3.4).
> +	 * Interrupts only occur in D3hot or shallower (PCIe r4.0, sec 6.7.3.4).
>  	 */
> -	if (pdev->current_state == PCI_D3cold ||
> -	    (!(ctrl->slot_ctrl & PCI_EXP_SLTCTL_HPIE) && !pciehp_poll_mode))
> +	if (pdev->current_state == PCI_D3cold)
>  		return IRQ_NONE;
>  
> +	/*
> +	 * Interrupts are only sent if enabled in the Slot Control register
> +	 * (PCIe r4.0, sec 6.7.3.4).  ctrl->slot_ctrl caches that register, but
> +	 * the cached Hot-Plug Interrupt Enable bit can fall transiently out of
> +	 * sync with the hardware if platform firmware clears it behind the
> +	 * driver's back (e.g. concurrently with a Slot Control read-modify-
> +	 * write).  While the port is in D0 it is accessible, so re-read the
> +	 * register from hardware before dismissing the interrupt as not ours.
> +	 * In a low-power state the port may be inaccessible and the cached
> +	 * value is authoritative.
> +	 */
> +	if (!pciehp_poll_mode && !(ctrl->slot_ctrl & PCI_EXP_SLTCTL_HPIE)) {
> +		u16 slot_ctrl;
> +
> +		if (pdev->current_state != PCI_D0)
> +			return IRQ_NONE;
> +
> +		pcie_capability_read_word(pdev, PCI_EXP_SLTCTL, &slot_ctrl);

[Severity: High]
Does this introduce a Time-of-Check to Time-of-Use (TOCTOU) race condition when
reading the config space?

A concurrent pm_runtime_suspend operation powering down the device can race
with pciehp_isr receiving an interrupt.

The lockless pdev->current_state != PCI_D0 check could pass just before the
state is updated, leading to an unprotected pcie_capability_read_word after
the device becomes inaccessible.

Reading the PCI config space of a suspended device can trigger an Unsupported
Request (UR) or Completion Timeout. On some platforms (e.g. ARM64), this does
not gracefully return 0xFFFF but instead triggers a Synchronous External Abort
(SEA), leading to a kernel panic.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260924101929.143806-1-qiyuzhu2@amd.com?part=1