| 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
| ||
|
Message-ID: <c84d6952-7977-47cd-8f09-6ea223217337@suse.com>
Date: Tue, 23 Sep 2025 14:55:34 +0200
From: Petr Pavlu <petr.pavlu@...e.com>
To: Brian Norris <briannorris@...omium.org>
Cc: Bjorn Helgaas <bhelgaas@...gle.com>, Luis Chamberlain
<mcgrof@...nel.org>, Daniel Gomez <da.gomez@...nel.org>,
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,
Johannes Berg <johannes@...solutions.net>,
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 9/13/25 12:59 AM, 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.
>
> I don't attempt to be clever here; the algorithm here scales with the
> number of modules in the system.
>
> Signed-off-by: Brian Norris <briannorris@...omium.org>
> ---
>
> drivers/pci/quirks.c | 62 ++++++++++++++++++++++++++++++++++++++++++
> include/linux/module.h | 18 ++++++++++++
> kernel/module/main.c | 26 ++++++++++++++++++
> 3 files changed, 106 insertions(+)
>
> diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
> index d97335a40193..db5e0ac82ed7 100644
> --- a/drivers/pci/quirks.c
> +++ b/drivers/pci/quirks.c
> @@ -207,6 +207,62 @@ extern struct pci_fixup __end_pci_fixups_suspend_late[];
>
> static bool pci_apply_fixup_final_quirks;
>
> +struct pci_fixup_arg {
> + struct pci_dev *dev;
> + enum pci_fixup_pass pass;
> +};
> +
> +static int pci_module_fixup(struct module *mod, void *parm)
> +{
> + struct pci_fixup_arg *arg = parm;
> + void *start;
> + unsigned int size;
> +
> + switch (arg->pass) {
> + case pci_fixup_early:
> + start = mod->pci_fixup_early;
> + size = mod->pci_fixup_early_size;
> + break;
> + case pci_fixup_header:
> + start = mod->pci_fixup_header;
> + size = mod->pci_fixup_header_size;
> + break;
> + case pci_fixup_final:
> + start = mod->pci_fixup_final;
> + size = mod->pci_fixup_final_size;
> + break;
> + case pci_fixup_enable:
> + start = mod->pci_fixup_enable;
> + size = mod->pci_fixup_enable_size;
> + break;
> + case pci_fixup_resume:
> + start = mod->pci_fixup_resume;
> + size = mod->pci_fixup_resume_size;
> + break;
> + case pci_fixup_suspend:
> + start = mod->pci_fixup_suspend;
> + size = mod->pci_fixup_suspend_size;
> + break;
> + case pci_fixup_resume_early:
> + start = mod->pci_fixup_resume_early;
> + size = mod->pci_fixup_resume_early_size;
> + break;
> + case pci_fixup_suspend_late:
> + start = mod->pci_fixup_suspend_late;
> + size = mod->pci_fixup_suspend_late_size;
> + break;
> + default:
> + return 0;
> + }
> +
> + if (!size)
> + return 0;
> +
> + pci_do_fixups(arg->dev, start, start + size);
> +
> + return 0;
> +}
> +
> void pci_fixup_device(enum pci_fixup_pass pass, struct pci_dev *dev)
> {
> struct pci_fixup *start, *end;
> @@ -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?
> }
> EXPORT_SYMBOL(pci_fixup_device);
>
> diff --git a/include/linux/module.h b/include/linux/module.h
> index 3319a5269d28..7faa8987b9eb 100644
> --- a/include/linux/module.h
> +++ b/include/linux/module.h
> @@ -539,6 +539,24 @@ struct module {
> int num_kunit_suites;
> struct kunit_suite **kunit_suites;
> #endif
> +#ifdef CONFIG_PCI_QUIRKS
> + void *pci_fixup_early;
> + unsigned int pci_fixup_early_size;
> + void *pci_fixup_header;
> + unsigned int pci_fixup_header_size;
> + void *pci_fixup_final;
> + unsigned int pci_fixup_final_size;
> + void *pci_fixup_enable;
> + unsigned int pci_fixup_enable_size;
> + void *pci_fixup_resume;
> + unsigned int pci_fixup_resume_size;
> + void *pci_fixup_suspend;
> + unsigned int pci_fixup_suspend_size;
> + void *pci_fixup_resume_early;
> + unsigned int pci_fixup_resume_early_size;
> + void *pci_fixup_suspend_late;
> + unsigned int pci_fixup_suspend_late_size;
> +#endif
>
>
> #ifdef CONFIG_LIVEPATCH
> diff --git a/kernel/module/main.c b/kernel/module/main.c
> index c66b26184936..50a80c875adc 100644
> --- a/kernel/module/main.c
> +++ b/kernel/module/main.c
> @@ -2702,6 +2702,32 @@ static int find_module_sections(struct module *mod, struct load_info *info)
> sizeof(*mod->kunit_init_suites),
> &mod->num_kunit_init_suites);
> #endif
> +#ifdef CONFIG_PCI_QUIRKS
> + mod->pci_fixup_early = section_objs(info, ".pci_fixup_early",
> + sizeof(*mod->pci_fixup_early),
> + &mod->pci_fixup_early_size);
> + mod->pci_fixup_header = section_objs(info, ".pci_fixup_header",
> + sizeof(*mod->pci_fixup_header),
> + &mod->pci_fixup_header_size);
> + mod->pci_fixup_final = section_objs(info, ".pci_fixup_final",
> + sizeof(*mod->pci_fixup_final),
> + &mod->pci_fixup_final_size);
> + mod->pci_fixup_enable = section_objs(info, ".pci_fixup_enable",
> + sizeof(*mod->pci_fixup_enable),
> + &mod->pci_fixup_enable_size);
> + mod->pci_fixup_resume = section_objs(info, ".pci_fixup_resume",
> + sizeof(*mod->pci_fixup_resume),
> + &mod->pci_fixup_resume_size);
> + mod->pci_fixup_suspend = section_objs(info, ".pci_fixup_suspend",
> + sizeof(*mod->pci_fixup_suspend),
> + &mod->pci_fixup_suspend_size);
> + mod->pci_fixup_resume_early = section_objs(info, ".pci_fixup_resume_early",
> + sizeof(*mod->pci_fixup_resume_early),
> + &mod->pci_fixup_resume_early_size);
> + mod->pci_fixup_suspend_late = section_objs(info, ".pci_fixup_suspend_late",
> + sizeof(*mod->pci_fixup_suspend_late),
> + &mod->pci_fixup_suspend_late_size);
> +#endif
>
> mod->extable = section_objs(info, "__ex_table",
> sizeof(*mod->extable), &mod->num_exentries);
Nit: I suggest writing the object_size argument passed to section_objs()
here directly as "1" instead of using sizeof(*mod->pci_fixup_...) =
sizeof(void). This makes the style consistent with the other code in
find_module_sections().
--
Thanks,
Petr
Powered by blists - more mailing lists