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

[Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>]

On 4/5/2026 1:32 PM, Lukas Wunner wrote:
> 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.
we are seeing this issue in non hotplug-capable device.
>> 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().
This issue is coming as part of the probe, we are checking if the link is up first
and then trying to do pci_save_state() and when the execution is in the middle
save state we see there is a  linkdown and pci_save_state is actually storing
some all F's.  we are hitting the issue in pci_store_saved_state() which is
due wrong data saved as part of pci_save_state(). pci_save_state() has many
config reading having check for each read is not ideal way, that is why we are
tring to keep check so that it will not crash. - Krishna Chaitanya.
>
>> +	/*
>> +	 * 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.
ack.
> Thanks,
>
> Lukas