linux-kernel - Re: [PATCH 1/4] PCI: Support FIXUP quirks in modules

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <8e75d6cc3847899ba8d6a0cbd0ef3ac57eabf009.camel@sipsolutions.net>
Date: Mon, 15 Sep 2025 08:33:08 +0200
From: Johannes Berg <johannes@...solutions.net>
To: Brian Norris <briannorris@...omium.org>, Bjorn Helgaas
 <bhelgaas@...gle.com>,  Luis Chamberlain	 <mcgrof@...nel.org>, Petr Pavlu
 <petr.pavlu@...e.com>, Daniel Gomez	 <da.gomez@...nel.org>
Cc: linux-pci@...r.kernel.org, David Gow <davidgow@...gle.com>, Rae Moar	
 <rmoar@...gle.com>, linux-kselftest@...r.kernel.org, 
	linux-kernel@...r.kernel.org, linux-modules@...r.kernel.org, Sami Tolvanen	
 <samitolvanen@...gle.com>, Richard Weinberger <richard@....at>, Wei Liu	
 <wei.liu@...nel.org>, Brendan Higgins <brendan.higgins@...ux.dev>, 
	kunit-dev@...glegroups.com, Anton Ivanov <anton.ivanov@...bridgegreys.com>,
 	linux-um@...ts.infradead.org
Subject: Re: [PATCH 1/4] PCI: Support FIXUP quirks in modules

On Fri, 2025-09-12 at 15:59 -0700, Brian Norris wrote:
> The PCI framework supports "quirks" for PCI devices via several
> DECLARE_PCI_FIXUP_*() macros. These macros allow arch or driver code to
> match device IDs to provide customizations or workarounds for broken
> devices.
> 
> This mechanism is generally used in code that can only be built into the
> kernel, but there are a few occasions where this mechanism is used in
> drivers that can be modules. For example, see commit 574f29036fce ("PCI:
> iproc: Apply quirk_paxc_bridge() for module as well as built-in").
> 
> The PCI fixup mechanism only works for built-in code, however, because
> pci_fixup_device() only scans the ".pci_fixup_*" linker sections found
> in the main kernel; it never touches modules.
> 
> Extend the fixup approach to modules.

This _feels_ a bit odd to me - what if you reload a module, should the
fixup be done twice? Right now this was not possible in a module, which
is a bit of a gotcha, but at least that's only one for developers, not
for users (unless someone messes up and puts it into modular code, as in
the example you gave.)

Although, come to think of it, you don't even apply the fixup when the
module is loaded, so what I just wrote isn't really true. That almost
seems like an oversight though, now the module has to be loaded before
the PCI device is enumerated, which is unlikely to happen in practice?
But then we get the next gotcha - the device is already enumerated, so
the fixups cannot be applied at the various enumeration stages, and
you're back to having to load the module before PCI enumeration, which
could be tricky, or somehow forcing re-enumeration of a given device
from userspace, but then you're firmly in "gotcha for the user"
territory again ...

I don't really have any skin in this game, but overall I'd probably
argue it's better to occasionally have to fix things such as in the
commit you point out but have a predictable system, than apply things
from modules.

Perhaps it'd be better to extend the section checking infrastructure to
catch and error out on these sections in modules instead, so we catch it
at build time, rather than finding things missing at runtime?

And yeah, now I've totally ignored the kunit angle, but ... not sure how
to combine the two requirements if they are, as I think, conflicting.

johannes