Re: [PATCH 2/2] PCI: Fix NULL pointer access in pci_store_saved_state()

[Lukas Wunner <lukas@wunner.de>]

On Sat, Apr 04, 2026 at 02:23:00PM +0530, Krishna Chaitanya Chundru wrote:
> If the PCIe link goes down while pci_save_state() is in progress, reads
> from the device configuration space may return invalid values(all 0xF's).

That should be harmless.  If the link goes down, the device should
subsequently be de-enumerated by the hotplug driver.  If we save
some bogus data before de-enumerating it, so be it.

If the port above is not hotplug-capable, manual intervention is
required for remove/rescan, but that's orthogonal to this problem.

> One example is, while saving VC extended capability save path
> (pci_save_vc_state() / pci_vc_do_save_buffer()) then interprets all-1s
> capability fields as valid and ends up writing far beyond the allocated
> pci_cap_saved_state buffer, corrupting the pci_dev->saved_cap_space list.

I'm not familiar with drivers/pci/vc.c, but it seems it takes a size
read from config space and uses it to write to a particular memory
area?  That feels totally wrong, at the very least there should be
a check for PCI_POSSIBLE_ERROR().

> The link state check here is racy since the link may transition at any
> time. This is a best-effort attempt to avoid saving PCI state when the
> link is already down.

No, please validate values read from config space with
PCI_POSSIBLE_ERROR() before using them to access memory at
a location that may be out-of-bounds.  Or cache the size on
enumeration and avoid re-reading it upon pci_save_state().

> +	/*
> +	 * The link state check here is racy since the link may transition at
> +	 * any time. This is a best-effort attempt to avoid saving PCI state
> +	 * when the link is already down.
> +	 */
> +	if (!pcie_link_is_active(dev)) {
> +		dev->state_saved = false;
> +		return NULL;
> +	}

The state_saved flag is only used by PM code to determine whether
the driver called pci_save_state().  If it didn't, the PCI core
will make up for it by calling pci_save_state().

The state_saved flag is *not* an indication whether the saved state
is usable and code outside power management has no business changing
the flag's value.

Thanks,

Lukas