On 9/23/25 7:42 PM, Brian Norris wrote:
Hi Petr,
On Tue, Sep 23, 2025 at 02:55:34PM +0200, Petr Pavlu wrote:
On 9/13/25 12:59 AM, Brian Norris wrote:
@@ -259,6 +315,12 @@ void pci_fixup_device(enum pci_fixup_pass pass, struct pci_dev *dev) return; } pci_do_fixups(dev, start, end);
- struct pci_fixup_arg arg = {
.dev = dev,.pass = pass,- };
- module_for_each_mod(pci_module_fixup, &arg);
The function module_for_each_mod() walks not only modules that are LIVE, but also those in the COMING and GOING states. This means that this code can potentially execute a PCI fixup from a module before its init function is invoked, and similarly, a fixup can be executed after the exit function has already run. Is this intentional?
Thanks for the callout. I didn't really give this part much thought previously.
Per the comments, COMING means "Full formed, running module_init". I believe that is a good thing, actually; specifically for controller drivers, module_init() might be probing the controller and enumerating child PCI devices to which we should apply these FIXUPs. That is a key case to support.
GOING is not clearly defined in the header comments, but it seems like it's a relatively narrow window between determining there are no module refcounts (and transition to GOING) and starting to really tear it down (transitioning to UNFORMED before any significant teardown). module_exit() runs in the GOING phase.
I think it does not make sense to execute FIXUPs on a GOING module; I'll make that change.
Note that when walking the modules list using module_for_each_mod(), the delete_module() operation can concurrently transition a module to MODULE_STATE_GOING. If you are thinking about simply having pci_module_fixup() check that mod->state isn't MODULE_STATE_GOING, I believe this won't quite work.
Re-quoting one piece:
This means that this code can potentially execute a PCI fixup from a module before its init function is invoked,
IIUC, this part is not true? A module is put into COMING state before its init function is invoked.
When loading a module, the load_module() function calls complete_formation(), which puts the module into the COMING state. At this point, the new code in pci_fixup_device() can see the new module and potentially attempt to invoke its PCI fixups. However, such a module has still a bit of way to go before its init function is called from do_init_module(). The module hasn't yet had its arguments parsed, is not linked in sysfs, isn't fully registered with codetag support, and hasn't invoked its constructors (needed for gcov/kasan support).
I don't know enough about PCI fixups and what is allowable in them, but I suspect it would be better to ensure that no fixup can be invoked from the module during this period.
If the above makes sense, I think using module_for_each_mod() might not be the right approach. Alternative options include registering a module notifier or having modules explicitly register their PCI fixups in their init function.