linux-kernel - Re: [PATCH v4 01/10] PCI: Avoid saving error values for config space

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <aOE1JMryY_Oa663e@wunner.de>
Date: Sat, 4 Oct 2025 16:54:28 +0200
From: Lukas Wunner <lukas@...ner.de>
To: Farhan Ali <alifm@...ux.ibm.com>
Cc: Benjamin Block <bblock@...ux.ibm.com>, linux-s390@...r.kernel.org,
	kvm@...r.kernel.org, linux-kernel@...r.kernel.org,
	linux-pci@...r.kernel.org, alex.williamson@...hat.com,
	helgaas@...nel.org, clg@...hat.com, schnelle@...ux.ibm.com,
	mjrosato@...ux.ibm.com
Subject: Re: [PATCH v4 01/10] PCI: Avoid saving error values for config space

On Wed, Oct 01, 2025 at 10:12:03AM -0700, Farhan Ali wrote:
> AFAIU if the state_saved flag was set to true then any state that we have
> saved should be valid and should be okay to be restored from. We just want
> to avoid saving any invalid data.

The state_saved flag is used by the PCI core to detect whether a driver
has called pci_save_state() in one of its suspend callbacks.  If it did,
the PCI core assumes that the driver has taken on the responsibility to
put the device into a low power state.  The PCI core will thus not put
the device into a low power state itself and it won't (again) call
pci_save_state().

Hence state_saved is cleared before the driver suspend callbacks are
invoked and it is checked afterwards.

Clearing the state_saved flag in pci_restore_state() merely serves the
purpose of ensuring that the flag is cleared ahead of the next suspend
and resume cycle.

It is a fallacy to think that state_saved indicates validity of the
saved state.

Unfortunately pci_restore_state() was amended by c82f63e411f1 to
bail out if state_saved is false.  This has arguably caused more
problems than it solved, so I have prepared this development branch
which essentially reverts the commit and undoes most of the awful
workarounds that it necessitated:

https://github.com/l1k/linux/commits/aer_reset_v1

I intend to submit this after the merge window has closed.

The motivation of c82f63e411f1 was to prevent restoring state if
pci_save_state() hasn't been called before.  I am solving that by
calling pci_save_state() on device addition, hence error
recoverability is ensured at all times.

I believe this also makes patch [01/10] in your series unnecessary.

A lot of drivers call pci_save_state() in their probe hook and
that continues to be correct if they modified Config Space
vis-a-vis what was saved on device addition.

Thanks,

Lukas