* Re: [PATCH RFC 019/104] module: add load_module_mem() helper
From: Petr Pavlu @ 2025-09-29 9:47 UTC (permalink / raw)
To: Vegard Nossum
Cc: Herbert Xu, David S. Miller, linux-crypto, Luis Chamberlain,
Daniel Gomez, Ard Biesheuvel, Eric Biggers, Jason A . Donenfeld,
Greg Kroah-Hartman, Wang, Jay, Nicolai Stange, Vladis Dronov,
Stephan Mueller, Sami Tolvanen, linux-modules,
Saeed Mirzamohammadi
In-Reply-To: <20250904155216.460962-20-vegard.nossum@oracle.com>
On 9/4/25 5:50 PM, Vegard Nossum wrote:
> Add a new helper function, load_module_mem(), which can load a kernel
> module from a byte array in memory.
>
> Also add a new module loader flag, MODULE_INIT_MEM, signalling that a
> module was loaded in this way.
>
> When a module is loaded with load_module_mem(), we do a few things
> differently:
>
> - don't do signature verification
> - ignore vermagic
Why is checking the vermagic skipped?
> - don't taint the kernel
Why is tainting the kernel skipped?
> - keep the initial reference to the module until the caller wants to
> drop it
>
> These changes are necessary for having a bundled (but separately
> compiled) FIPS module.
>
> We may want to let distros carry patches to disable tainting separately
> so this information is not lost in case somebody builds a non-distro
> kernel using a FIPS module compiled for an incompatible version.
>
> Co-developed-by: Saeed Mirzamohammadi <saeed.mirzamohammadi@oracle.com>
> Signed-off-by: Vegard Nossum <vegard.nossum@oracle.com>
I realize this is posted as an RFC so I'm not sure if you're looking for
more detailed comments on the implementation at this point. Nonetheless,
some notes are provided below.
> ---
> include/linux/module.h | 2 +
> include/uapi/linux/module.h | 5 ++
> kernel/module/main.c | 99 ++++++++++++++++++++++++++-----------
> 3 files changed, 77 insertions(+), 29 deletions(-)
>
> diff --git a/include/linux/module.h b/include/linux/module.h
> index 3319a5269d28..00d85602fb6a 100644
> --- a/include/linux/module.h
> +++ b/include/linux/module.h
> @@ -586,6 +586,8 @@ struct module {
>
> #ifdef CONFIG_MODULES
>
> +extern int load_module_mem(const char *mem, size_t size);
> +
Nit: The extern keyword is unnecessary here. See
Documentation/process/coding-style.rst, 6.1) Function prototypes.
> /* Get/put a kernel symbol (calls must be symmetric) */
> void *__symbol_get(const char *symbol);
> void *__symbol_get_gpl(const char *symbol);
> diff --git a/include/uapi/linux/module.h b/include/uapi/linux/module.h
> index 03a33ffffcba..5dcd24018be7 100644
> --- a/include/uapi/linux/module.h
> +++ b/include/uapi/linux/module.h
> @@ -7,4 +7,9 @@
> #define MODULE_INIT_IGNORE_VERMAGIC 2
> #define MODULE_INIT_COMPRESSED_FILE 4
>
> +#ifdef __KERNEL__
> +/* Internal flags */
> +#define MODULE_INIT_MEM 30
> +#endif
> +
This looks to be incorrect, 30 is 0b11110. The value should be a flag
with only one bit set.
Additionally, I think referring to this special-type module as MEM is
misleading as all modules are eventually loaded from the kernel memory.
Perhaps call it MODULE_INIT_EMBEDDED_FILE, which also aligns with
MODULE_INIT_COMPRESSED_FILE?
> #endif /* _UAPI_LINUX_MODULE_H */
> diff --git a/kernel/module/main.c b/kernel/module/main.c
> index c66b26184936..12ce4bad29ca 100644
> --- a/kernel/module/main.c
> +++ b/kernel/module/main.c
> @@ -2572,11 +2572,14 @@ static void module_augment_kernel_taints(struct module *mod, struct load_info *i
>
> static int check_modinfo(struct module *mod, struct load_info *info, int flags)
> {
> - const char *modmagic = get_modinfo(info, "vermagic");
> + const char *modmagic = NULL;
> int err;
>
> - if (flags & MODULE_INIT_IGNORE_VERMAGIC)
> - modmagic = NULL;
> + if (flags & MODULE_INIT_MEM)
> + return 0;
> +
> + if (!(flags & MODULE_INIT_IGNORE_VERMAGIC))
> + modmagic = get_modinfo(info, "vermagic");
>
> /* This is allowed: modprobe --force will invalidate it. */
> if (!modmagic) {
> @@ -3007,7 +3010,7 @@ module_param(async_probe, bool, 0644);
> * Keep it uninlined to provide a reliable breakpoint target, e.g. for the gdb
> * helper command 'lx-symbols'.
> */
> -static noinline int do_init_module(struct module *mod)
> +static noinline int do_init_module(struct module *mod, int flags)
> {
> int ret = 0;
> struct mod_initfree *freeinit;
> @@ -3071,7 +3074,8 @@ static noinline int do_init_module(struct module *mod)
> mod->mem[MOD_INIT_TEXT].base + mod->mem[MOD_INIT_TEXT].size);
> mutex_lock(&module_mutex);
> /* Drop initial reference. */
> - module_put(mod);
> + if (!(flags & MODULE_INIT_MEM))
> + module_put(mod);
> trim_init_extable(mod);
> #ifdef CONFIG_KALLSYMS
> /* Switch to core kallsyms now init is done: kallsyms may be walking! */
> @@ -3347,31 +3351,17 @@ static int early_mod_check(struct load_info *info, int flags)
> /*
> * Allocate and load the module: note that size of section 0 is always
> * zero, and we rely on this for optional sections.
> + *
> + * NOTE: module signature verification must have been done already.
> */
> -static int load_module(struct load_info *info, const char __user *uargs,
> - int flags)
> +static int _load_module(struct load_info *info, const char __user *uargs,
> + int flags)
> {
> struct module *mod;
> bool module_allocated = false;
> long err = 0;
> char *after_dashes;
>
> - /*
> - * Do the signature check (if any) first. All that
> - * the signature check needs is info->len, it does
> - * not need any of the section info. That can be
> - * set up later. This will minimize the chances
> - * of a corrupt module causing problems before
> - * we even get to the signature check.
> - *
> - * The check will also adjust info->len by stripping
> - * off the sig length at the end of the module, making
> - * checks against info->len more correct.
> - */
> - err = module_sig_check(info, flags);
> - if (err)
> - goto free_copy;
> -
> /*
> * Do basic sanity checks against the ELF header and
> * sections. Cache useful sections and set the
> @@ -3405,7 +3395,8 @@ static int load_module(struct load_info *info, const char __user *uargs,
> * We are tainting your kernel if your module gets into
> * the modules linked list somehow.
> */
> - module_augment_kernel_taints(mod, info);
> + if (!(flags & MODULE_INIT_MEM))
> + module_augment_kernel_taints(mod, info);
>
> /* To avoid stressing percpu allocator, do this once we're unique. */
> err = percpu_modalloc(mod, info);
> @@ -3452,7 +3443,11 @@ static int load_module(struct load_info *info, const char __user *uargs,
> flush_module_icache(mod);
>
> /* Now copy in args */
> - mod->args = strndup_user(uargs, ~0UL >> 1);
> + if ((flags & MODULE_INIT_MEM))
> + mod->args = kstrdup("", GFP_KERNEL);
> + else
> + mod->args = strndup_user(uargs, ~0UL >> 1);
> +
> if (IS_ERR(mod->args)) {
> err = PTR_ERR(mod->args);
> goto free_arch_cleanup;
> @@ -3500,13 +3495,10 @@ static int load_module(struct load_info *info, const char __user *uargs,
> if (codetag_load_module(mod))
> goto sysfs_cleanup;
>
> - /* Get rid of temporary copy. */
> - free_copy(info, flags);
> -
> /* Done! */
> trace_module_load(mod);
>
> - return do_init_module(mod);
> + return do_init_module(mod, flags);
>
> sysfs_cleanup:
> mod_sysfs_teardown(mod);
> @@ -3562,7 +3554,52 @@ static int load_module(struct load_info *info, const char __user *uargs,
> audit_log_kern_module(info->name ? info->name : "?");
> mod_stat_bump_becoming(info, flags);
> }
> + return err;
> +}
> +
> +/*
> + * Load module from kernel memory without signature check.
> + */
> +int load_module_mem(const char *mem, size_t size)
The description and name of this function are not ideal. All module
loads via load_module() are from the kernel memory and skipping the
signature check is not the only different property.
I suggest calling the function load_embedded_module() and improving its
description. Please preferably also use a kernel-doc to describe it as
the function is external.
> +{
> + int err;
> + struct load_info info = { };
> +
> + info.sig_ok = true;
> + info.hdr = (Elf64_Ehdr *) mem;
> + info.len = size;
> +
> + err = _load_module(&info, NULL, MODULE_INIT_MEM);
> + if (0)
> + free_copy(&info, 0);
Remove the dead code.
> +
> + return err;
> +}
> +
> +static int load_module(struct load_info *info, const char __user *uargs,
> + int flags)
> +{
> + int err;
> +
> + /*
> + * Do the signature check (if any) first. All that
> + * the signature check needs is info->len, it does
> + * not need any of the section info. That can be
> + * set up later. This will minimize the chances
> + * of a corrupt module causing problems before
> + * we even get to the signature check.
> + *
> + * The check will also adjust info->len by stripping
> + * off the sig length at the end of the module, making
> + * checks against info->len more correct.
> + */
> + err = module_sig_check(info, flags);
> + if (!err)
> + err = _load_module(info, uargs, flags);
> +
> + /* Get rid of temporary copy. */
> free_copy(info, flags);
> +
> return err;
> }
In the current code, the load_module() function frees the temporary copy
prior to calling the module's init function, which should generally
result in less memory pressure. This behavior looks useful to me to
preserve.
You could keep the current load_module() as is but wrap its
module_sig_check() call with 'if (!info->sig_ok)'. Similarly, the
free_copy() call could be protected by
'if (!(flags & MODULE_INIT_MEM))'.
>
> @@ -3728,6 +3765,10 @@ SYSCALL_DEFINE3(finit_module, int, fd, const char __user *, uargs, int, flags)
>
> pr_debug("finit_module: fd=%d, uargs=%p, flags=%i\n", fd, uargs, flags);
>
> + /*
> + * Deliberately omitting MODULE_INIT_MEM as it is for internal use
> + * only.
> + */
> if (flags & ~(MODULE_INIT_IGNORE_MODVERSIONS
> |MODULE_INIT_IGNORE_VERMAGIC
> |MODULE_INIT_COMPRESSED_FILE))
Nit: I suggest the following to improve the comment flow:
/*
* Check flags validity. Deliberately omit MODULE_INIT_MEM as it is for
* internal use only.
*/
--
Thanks,
Petr
^ permalink raw reply
* Re: [PATCH 0/4] PCI: Add support and tests for FIXUP quirks in modules
From: Christoph Hellwig @ 2025-09-29 8:56 UTC (permalink / raw)
To: Brian Norris
Cc: Christoph Hellwig, Bjorn Helgaas, Luis Chamberlain, Petr Pavlu,
Daniel Gomez, linux-pci, David Gow, Rae Moar, linux-kselftest,
linux-kernel, linux-modules, Johannes Berg, Sami Tolvanen,
Richard Weinberger, Wei Liu, Brendan Higgins, kunit-dev,
Anton Ivanov, linux-um
In-Reply-To: <aNGaBiUOb6_n8w8P@google.com>
On Mon, Sep 22, 2025 at 11:48:38AM -0700, Brian Norris wrote:
> On Mon, Sep 22, 2025 at 11:13:39AM -0700, Christoph Hellwig wrote:
> > Controller drivers are a special case I guess, but I'd rather still
> > not open it up to any random driver.
>
> I don't really see why this particular thing should develop restrictions
> beyond "can it work in modules?", but if you have an idea for how to do
> that reasonably, my ears are open.
PCI Controller seem pretty special in that they provide infrastructure.
^ permalink raw reply
* Re: [RFC PATCH 00/10] scalable symbol flags with __kflagstab
From: Sid Nayyar @ 2025-09-26 0:11 UTC (permalink / raw)
To: Petr Pavlu
Cc: Nathan Chancellor, Luis Chamberlain, Sami Tolvanen,
Nicolas Schier, Arnd Bergmann, linux-kbuild, linux-arch,
linux-modules, linux-kernel, Giuliano Procida,
Matthias Männich
In-Reply-To: <2bf54830-ea9c-4962-a7ef-653fbed8f8c0@suse.com>
On Mon, Sep 22, 2025 at 12:41 PM Petr Pavlu <petr.pavlu@suse.com> wrote:
> This is useful information. However, I was specifically interested in
> the impact of having the new flags field present as part of __ksymtab
> (kernel_symbol), compared to keeping it in a separate section. Sorry for
> not being clear.
>
> I ran a small test to get a better understanding of the different sizes.
> I used v6.17-rc6 together with the openSUSE x86_64 config [1], which is
> fairly large. The resulting vmlinux.bin (no debuginfo) had an on-disk
> size of 58 MiB, and included 5937 + 6589 (GPL-only) exported symbols.
>
> The following table summarizes my measurements and calculations
> regarding the sizes of all sections related to exported symbols:
>
> | HAVE_ARCH_PREL32_RELOCATIONS | !HAVE_ARCH_PREL32_RELOCATIONS
> Section | Base [B] | Ext. [B] | Sep. [B] | Base [B] | Ext. [B] | Sep. [B]
> ----------------------------------------------------------------------------------------
> __ksymtab | 71244 | 200416 | 150312 | 142488 | 400832 | 300624
> __ksymtab_gpl | 79068 | NA | NA | 158136 | NA | NA
> __kcrctab | 23748 | 50104 | 50104 | 23748 | 50104 | 50104
> __kcrctab_gpl | 26356 | NA | NA | 26356 | NA | NA
> __ksymtab_strings | 253628 | 253628 | 253628 | 253628 | 253628 | 253628
> __kflagstab | NA | NA | 12526 | NA | NA | 12526
> ----------------------------------------------------------------------------------------
> Total | 454044 | 504148 | 466570 | 604356 | 704564 | 616882
> Increase to base [%] | NA | 11.0 | 2.8 | NA | 16.6 | 2.1
>
> The column "HAVE_ARCH_PREL32_RELOCATIONS -> Base" contains the numbers
> that I measured. The rest of the values are calculated. The "Ext."
> column represents the variant of extending __ksymtab, and the "Sep."
> column represents the variant of having a separate __kflagstab. With
> HAVE_ARCH_PREL32_RELOCATIONS, each kernel_symbol is 12 B in size and is
> extended to 16 B. With !HAVE_ARCH_PREL32_RELOCATIONS, it is 24 B,
> extended to 32 B. Note that this does not include the metadata needed to
> relocate __ksymtab*, which is freed after the initial processing.
>
> The base export data in this case totals 0.43 MiB. About 50% is used for
> storing the names of exported symbols.
>
> Adding __kflagstab as a separate section has a negligible impact, as
> expected. When extending __ksymtab (kernel_symbol) instead, the worst
> case with !HAVE_ARCH_PREL32_RELOCATIONS increases the export data size
> by 16.6%.
>
> Based on the above, I think introducing __kflagstab makes senses, as the
> added complexity is minimal, although I feel we could probably also get
> away with extending kernel_symbol.
This investigation is very informative, thank you for sharing your
findings. I am in agreement with your conclusions.
> This seems to answer why the in-tree flag is not sufficient for you.
> However, I also suggested an alternative that the symbol protection
> could be determined by whether the module is signed by a key from the
> .builtin_trusted_keys keyring, as opposed to being signed by another key
> reachable from the .secondary_trusted_keys keyring or being completely
> unsigned.
>
> Distributions can require that external modules be signed and allow
> additional keys to be added as Machine Owner Keys, which can be made
> reachable from .secondary_trusted_keys. Nonetheless, such distributions
> might be still interested in limiting the number of symbols that such
> external modules can use.
>
> I think this option is worth considering, as it could potentially make
> this symbol protection useful for other distributions as well.
This sounds like a great solution to enhance trust and security,
apologies for missing this in the previous email. I will explore this
approach, but I would like to do it in a separate series.
> I'm personally ok with adding the kflagstab support. I think it
> introduces minimal complexity and, as you point out, simplifies certain
> aspects. Additionally, if we add it, I believe that adding the proposed
> symbol protection is simple enough to be included as well, at least from
> my perspective.
Since we are in agreement, I would like to seek code review for this
series. The code is ready for review from my side, but if you prefer I
can send out a non-RFC patch series for code review.
--
Thanks,
Siddharth Nayyar
^ permalink raw reply
* Re: [PATCH 0/6] module: enable force unloading of modules that have crashed during init
From: Julian LaGattuta @ 2025-09-24 22:16 UTC (permalink / raw)
To: Petr Pavlu
Cc: Luis Chamberlain, Sami Tolvanen, Daniel Gomez, linux-modules,
linux-kernel
In-Reply-To: <000808f3-10cf-46ad-94f9-95a142c08b59@suse.com>
> Could you please explain the motivation for doing this in more detail?
>
> I think we shouldn't attempt to do anything clever with modules that
> crashed during initialization. Such a module can already leave the
> system in an unstable state and trying to recover can cause even more
> problems. For instance, I don't see how it is safe to call the module's
> exit function.
>
> --
> Thanks,
> Petr
Thank you for your response Petr. The motivation comes from when I
wanted to replace a crashed module with one which does not crash
without having to reboot. I looked around and saw some other people
complain about it on stackoverflow.
I thought that if a module crashed during init, it would be in a no
better position compared to if it were forcefully removed.
Therefore, there is no reason why this shouldn't be an option as it
couldn't make the problem worse.
I agree that calling the exit function doesn't make sense and so I
could change the behavior.
That being said, I understand why someone would be wary of this type
of change; this is just my thought process.
Sincerely,
Julian
^ permalink raw reply
* Re: [PATCH v8 0/8] Add generated modalias to modules.builtin.modinfo
From: Nathan Chancellor @ 2025-09-24 16:17 UTC (permalink / raw)
To: Nathan Chancellor, Nicolas Schier, Petr Pavlu, Luis Chamberlain,
Sami Tolvanen, Daniel Gomez, Alexey Gladkov
Cc: linux-kernel, linux-modules, linux-kbuild
In-Reply-To: <cover.1758182101.git.legion@kernel.org>
On Thu, 18 Sep 2025 10:05:44 +0200, Alexey Gladkov wrote:
> The modules.builtin.modinfo file is used by userspace (kmod to be specific) to
> get information about builtin modules. Among other information about the module,
> information about module aliases is stored. This is very important to determine
> that a particular modalias will be handled by a module that is inside the
> kernel.
>
> There are several mechanisms for creating modalias for modules:
>
> [...]
Applied, thanks!
[1/8] s390: vmlinux.lds.S: Reorder sections
https://git.kernel.org/kbuild/c/8d18ef04f940a
[2/8] kbuild: always create intermediate vmlinux.unstripped
https://git.kernel.org/kbuild/c/0ce5139fd96e9
[3/8] kbuild: keep .modinfo section in vmlinux.unstripped
https://git.kernel.org/kbuild/c/3e86e4d74c049
[4/8] kbuild: extract modules.builtin.modinfo from vmlinux.unstripped
https://git.kernel.org/kbuild/c/39cfd5b12160b
[5/8] scsi: Always define blogic_pci_tbl structure
https://git.kernel.org/kbuild/c/b88f88c26705a
[6/8] modpost: Add modname to mod_device_table alias
https://git.kernel.org/kbuild/c/83fb49389bbe0
[7/8] modpost: Create modalias for builtin modules
https://git.kernel.org/kbuild/c/5ab23c7923a1d
[8/8] kbuild: vmlinux.unstripped should always depend on .vmlinux.export.o
https://git.kernel.org/kbuild/c/3328d39a8dca2
Best regards,
--
Nathan Chancellor <nathan@kernel.org>
^ permalink raw reply
* [PATCH v18 2/7] rust: str: add radix prefixed integer parsing functions
From: Andreas Hindborg @ 2025-09-24 12:39 UTC (permalink / raw)
To: Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Alice Ryhl, Masahiro Yamada,
Nathan Chancellor, Luis Chamberlain, Danilo Krummrich,
Benno Lossin, Daniel Gomez, Benno Lossin, Nicolas Schier
Cc: Trevor Gross, Adam Bratschi-Kaye, rust-for-linux, linux-kernel,
linux-kbuild, Petr Pavlu, Sami Tolvanen, Daniel Gomez,
Simona Vetter, Greg KH, Fiona Behrens, Daniel Almeida,
linux-modules, Andreas Hindborg
In-Reply-To: <20250924-module-params-v3-v18-0-bf512c35d910@kernel.org>
Add the trait `ParseInt` for parsing string representations of integers
where the string representations are optionally prefixed by a radix
specifier. Implement the trait for the primitive integer types.
Suggested-by: Benno Lossin <benno.lossin@proton.me>
Tested-by: Daniel Gomez <da.gomez@samsung.com>
Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Reviewed-by: Benno Lossin <lossin@kernel.org>
Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
---
rust/kernel/str.rs | 2 +
rust/kernel/str/parse_int.rs | 148 +++++++++++++++++++++++++++++++++++++++++++
2 files changed, 150 insertions(+)
diff --git a/rust/kernel/str.rs b/rust/kernel/str.rs
index 6c892550c0ba9..23fe924070e7c 100644
--- a/rust/kernel/str.rs
+++ b/rust/kernel/str.rs
@@ -8,6 +8,8 @@
use crate::prelude::*;
+pub mod parse_int;
+
/// Byte string without UTF-8 validity guarantee.
#[repr(transparent)]
pub struct BStr([u8]);
diff --git a/rust/kernel/str/parse_int.rs b/rust/kernel/str/parse_int.rs
new file mode 100644
index 0000000000000..48eb4c202984c
--- /dev/null
+++ b/rust/kernel/str/parse_int.rs
@@ -0,0 +1,148 @@
+// SPDX-License-Identifier: GPL-2.0
+
+//! Integer parsing functions.
+//!
+//! Integer parsing functions for parsing signed and unsigned integers
+//! potentially prefixed with `0x`, `0o`, or `0b`.
+
+use crate::prelude::*;
+use crate::str::BStr;
+use core::ops::Deref;
+
+// Make `FromStrRadix` a public type with a private name. This seals
+// `ParseInt`, that is, prevents downstream users from implementing the
+// trait.
+mod private {
+ use crate::prelude::*;
+ use crate::str::BStr;
+
+ /// Trait that allows parsing a [`&BStr`] to an integer with a radix.
+ pub trait FromStrRadix: Sized {
+ /// Parse `src` to [`Self`] using radix `radix`.
+ fn from_str_radix(src: &BStr, radix: u32) -> Result<Self>;
+
+ /// Tries to convert `value` into [`Self`] and negates the resulting value.
+ fn from_u64_negated(value: u64) -> Result<Self>;
+ }
+}
+
+/// Extract the radix from an integer literal optionally prefixed with
+/// one of `0x`, `0X`, `0o`, `0O`, `0b`, `0B`, `0`.
+fn strip_radix(src: &BStr) -> (u32, &BStr) {
+ match src.deref() {
+ [b'0', b'x' | b'X', rest @ ..] => (16, rest.as_ref()),
+ [b'0', b'o' | b'O', rest @ ..] => (8, rest.as_ref()),
+ [b'0', b'b' | b'B', rest @ ..] => (2, rest.as_ref()),
+ // NOTE: We are including the leading zero to be able to parse
+ // literal `0` here. If we removed it as a radix prefix, we would
+ // not be able to parse `0`.
+ [b'0', ..] => (8, src),
+ _ => (10, src),
+ }
+}
+
+/// Trait for parsing string representations of integers.
+///
+/// Strings beginning with `0x`, `0o`, or `0b` are parsed as hex, octal, or
+/// binary respectively. Strings beginning with `0` otherwise are parsed as
+/// octal. Anything else is parsed as decimal. A leading `+` or `-` is also
+/// permitted. Any string parsed by [`kstrtol()`] or [`kstrtoul()`] will be
+/// successfully parsed.
+///
+/// [`kstrtol()`]: https://docs.kernel.org/core-api/kernel-api.html#c.kstrtol
+/// [`kstrtoul()`]: https://docs.kernel.org/core-api/kernel-api.html#c.kstrtoul
+///
+/// # Examples
+///
+/// ```
+/// # use kernel::str::parse_int::ParseInt;
+/// # use kernel::b_str;
+///
+/// assert_eq!(Ok(0u8), u8::from_str(b_str!("0")));
+///
+/// assert_eq!(Ok(0xa2u8), u8::from_str(b_str!("0xa2")));
+/// assert_eq!(Ok(-0xa2i32), i32::from_str(b_str!("-0xa2")));
+///
+/// assert_eq!(Ok(-0o57i8), i8::from_str(b_str!("-0o57")));
+/// assert_eq!(Ok(0o57i8), i8::from_str(b_str!("057")));
+///
+/// assert_eq!(Ok(0b1001i16), i16::from_str(b_str!("0b1001")));
+/// assert_eq!(Ok(-0b1001i16), i16::from_str(b_str!("-0b1001")));
+///
+/// assert_eq!(Ok(127i8), i8::from_str(b_str!("127")));
+/// assert!(i8::from_str(b_str!("128")).is_err());
+/// assert_eq!(Ok(-128i8), i8::from_str(b_str!("-128")));
+/// assert!(i8::from_str(b_str!("-129")).is_err());
+/// assert_eq!(Ok(255u8), u8::from_str(b_str!("255")));
+/// assert!(u8::from_str(b_str!("256")).is_err());
+/// ```
+pub trait ParseInt: private::FromStrRadix + TryFrom<u64> {
+ /// Parse a string according to the description in [`Self`].
+ fn from_str(src: &BStr) -> Result<Self> {
+ match src.deref() {
+ [b'-', rest @ ..] => {
+ let (radix, digits) = strip_radix(rest.as_ref());
+ // 2's complement values range from -2^(b-1) to 2^(b-1)-1.
+ // So if we want to parse negative numbers as positive and
+ // later multiply by -1, we have to parse into a larger
+ // integer. We choose `u64` as sufficiently large.
+ //
+ // NOTE: 128 bit integers are not available on all
+ // platforms, hence the choice of 64 bits.
+ let val =
+ u64::from_str_radix(core::str::from_utf8(digits).map_err(|_| EINVAL)?, radix)
+ .map_err(|_| EINVAL)?;
+ Self::from_u64_negated(val)
+ }
+ _ => {
+ let (radix, digits) = strip_radix(src);
+ Self::from_str_radix(digits, radix).map_err(|_| EINVAL)
+ }
+ }
+ }
+}
+
+macro_rules! impl_parse_int {
+ ($($ty:ty),*) => {
+ $(
+ impl private::FromStrRadix for $ty {
+ fn from_str_radix(src: &BStr, radix: u32) -> Result<Self> {
+ <$ty>::from_str_radix(core::str::from_utf8(src).map_err(|_| EINVAL)?, radix)
+ .map_err(|_| EINVAL)
+ }
+
+ fn from_u64_negated(value: u64) -> Result<Self> {
+ const ABS_MIN: u64 = {
+ #[allow(unused_comparisons)]
+ if <$ty>::MIN < 0 {
+ 1u64 << (<$ty>::BITS - 1)
+ } else {
+ 0
+ }
+ };
+
+ if value > ABS_MIN {
+ return Err(EINVAL);
+ }
+
+ if value == ABS_MIN {
+ return Ok(<$ty>::MIN);
+ }
+
+ // SAFETY: The above checks guarantee that `value` fits into `Self`:
+ // - if `Self` is unsigned, then `ABS_MIN == 0` and thus we have returned above
+ // (either `EINVAL` or `MIN`).
+ // - if `Self` is signed, then we have that `0 <= value < ABS_MIN`. And since
+ // `ABS_MIN - 1` fits into `Self` by construction, `value` also does.
+ let value: Self = unsafe { value.try_into().unwrap_unchecked() };
+
+ Ok((!value).wrapping_add(1))
+ }
+ }
+
+ impl ParseInt for $ty {}
+ )*
+ };
+}
+
+impl_parse_int![i8, u8, i16, u16, i32, u32, i64, u64, isize, usize];
--
2.47.2
^ permalink raw reply related
* [PATCH v18 6/7] rust: samples: add a module parameter to the rust_minimal sample
From: Andreas Hindborg @ 2025-09-24 12:39 UTC (permalink / raw)
To: Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Alice Ryhl, Masahiro Yamada,
Nathan Chancellor, Luis Chamberlain, Danilo Krummrich,
Benno Lossin, Daniel Gomez, Benno Lossin, Nicolas Schier
Cc: Trevor Gross, Adam Bratschi-Kaye, rust-for-linux, linux-kernel,
linux-kbuild, Petr Pavlu, Sami Tolvanen, Daniel Gomez,
Simona Vetter, Greg KH, Fiona Behrens, Daniel Almeida,
linux-modules, Andreas Hindborg
In-Reply-To: <20250924-module-params-v3-v18-0-bf512c35d910@kernel.org>
Showcase the rust module parameter support by adding a module parameter to
the `rust_minimal` sample.
Reviewed-by: Benno Lossin <lossin@kernel.org>
Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
---
samples/rust/rust_minimal.rs | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/samples/rust/rust_minimal.rs b/samples/rust/rust_minimal.rs
index 1fc7a1be6b6d7..8eb9583571d72 100644
--- a/samples/rust/rust_minimal.rs
+++ b/samples/rust/rust_minimal.rs
@@ -10,6 +10,12 @@
authors: ["Rust for Linux Contributors"],
description: "Rust minimal sample",
license: "GPL",
+ params: {
+ test_parameter: i64 {
+ default: 1,
+ description: "This parameter has a default of 1",
+ },
+ },
}
struct RustMinimal {
@@ -20,6 +26,10 @@ impl kernel::Module for RustMinimal {
fn init(_module: &'static ThisModule) -> Result<Self> {
pr_info!("Rust minimal sample (init)\n");
pr_info!("Am I built-in? {}\n", !cfg!(MODULE));
+ pr_info!(
+ "test_parameter: {}\n",
+ *module_parameters::test_parameter.value()
+ );
let mut numbers = KVec::new();
numbers.push(72, GFP_KERNEL)?;
--
2.47.2
^ permalink raw reply related
* [PATCH v18 4/7] rust: module: use a reference in macros::module::module
From: Andreas Hindborg @ 2025-09-24 12:39 UTC (permalink / raw)
To: Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Alice Ryhl, Masahiro Yamada,
Nathan Chancellor, Luis Chamberlain, Danilo Krummrich,
Benno Lossin, Daniel Gomez, Benno Lossin, Nicolas Schier
Cc: Trevor Gross, Adam Bratschi-Kaye, rust-for-linux, linux-kernel,
linux-kbuild, Petr Pavlu, Sami Tolvanen, Daniel Gomez,
Simona Vetter, Greg KH, Fiona Behrens, Daniel Almeida,
linux-modules, Andreas Hindborg
In-Reply-To: <20250924-module-params-v3-v18-0-bf512c35d910@kernel.org>
When we add parameter support to the module macro, we want to be able to
pass a reference to `ModuleInfo` to a helper function. That is not possible
when we move out of the local `modinfo`. So change the function to access
the local via reference rather than value.
Reviewed-by: Benno Lossin <lossin@kernel.org>
Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
---
rust/macros/module.rs | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/rust/macros/module.rs b/rust/macros/module.rs
index 5ee54a00c0b65..cbf3ac0a8f7ba 100644
--- a/rust/macros/module.rs
+++ b/rust/macros/module.rs
@@ -176,23 +176,23 @@ pub(crate) fn module(ts: TokenStream) -> TokenStream {
// Rust does not allow hyphens in identifiers, use underscore instead.
let ident = info.name.replace('-', "_");
let mut modinfo = ModInfoBuilder::new(ident.as_ref());
- if let Some(authors) = info.authors {
+ if let Some(authors) = &info.authors {
for author in authors {
- modinfo.emit("author", &author);
+ modinfo.emit("author", author);
}
}
- if let Some(description) = info.description {
- modinfo.emit("description", &description);
+ if let Some(description) = &info.description {
+ modinfo.emit("description", description);
}
modinfo.emit("license", &info.license);
- if let Some(aliases) = info.alias {
+ if let Some(aliases) = &info.alias {
for alias in aliases {
- modinfo.emit("alias", &alias);
+ modinfo.emit("alias", alias);
}
}
- if let Some(firmware) = info.firmware {
+ if let Some(firmware) = &info.firmware {
for fw in firmware {
- modinfo.emit("firmware", &fw);
+ modinfo.emit("firmware", fw);
}
}
--
2.47.2
^ permalink raw reply related
* [PATCH v18 1/7] rust: sync: add `SetOnce`
From: Andreas Hindborg @ 2025-09-24 12:39 UTC (permalink / raw)
To: Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Alice Ryhl, Masahiro Yamada,
Nathan Chancellor, Luis Chamberlain, Danilo Krummrich,
Benno Lossin, Daniel Gomez, Benno Lossin, Nicolas Schier
Cc: Trevor Gross, Adam Bratschi-Kaye, rust-for-linux, linux-kernel,
linux-kbuild, Petr Pavlu, Sami Tolvanen, Daniel Gomez,
Simona Vetter, Greg KH, Fiona Behrens, Daniel Almeida,
linux-modules, Andreas Hindborg
In-Reply-To: <20250924-module-params-v3-v18-0-bf512c35d910@kernel.org>
Introduce the `SetOnce` type, a container that can only be written once.
The container uses an internal atomic to synchronize writes to the internal
value.
Reviewed-by: Alice Ryhl <aliceryhl@google.com>
Reviewed-by: Benno Lossin <lossin@kernel.org>
Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
---
rust/kernel/sync.rs | 2 +
rust/kernel/sync/set_once.rs | 125 +++++++++++++++++++++++++++++++++++++++++++
2 files changed, 127 insertions(+)
diff --git a/rust/kernel/sync.rs b/rust/kernel/sync.rs
index cf5b638a097d9..3f957ad3a9f0a 100644
--- a/rust/kernel/sync.rs
+++ b/rust/kernel/sync.rs
@@ -20,6 +20,7 @@
pub mod poll;
pub mod rcu;
mod refcount;
+mod set_once;
pub use arc::{Arc, ArcBorrow, UniqueArc};
pub use completion::Completion;
@@ -29,6 +30,7 @@
pub use lock::spinlock::{new_spinlock, SpinLock, SpinLockGuard};
pub use locked_by::LockedBy;
pub use refcount::Refcount;
+pub use set_once::SetOnce;
/// Represents a lockdep class. It's a wrapper around C's `lock_class_key`.
#[repr(transparent)]
diff --git a/rust/kernel/sync/set_once.rs b/rust/kernel/sync/set_once.rs
new file mode 100644
index 0000000000000..bdba601807d8b
--- /dev/null
+++ b/rust/kernel/sync/set_once.rs
@@ -0,0 +1,125 @@
+// SPDX-License-Identifier: GPL-2.0
+
+//! A container that can be initialized at most once.
+
+use super::atomic::{
+ ordering::{Acquire, Relaxed, Release},
+ Atomic,
+};
+use core::{cell::UnsafeCell, mem::MaybeUninit};
+
+/// A container that can be populated at most once. Thread safe.
+///
+/// Once the a [`SetOnce`] is populated, it remains populated by the same object for the
+/// lifetime `Self`.
+///
+/// # Invariants
+///
+/// - `init` may only increase in value.
+/// - `init` may only assume values in the range `0..=2`.
+/// - `init == 0` if and only if `value` is uninitialized.
+/// - `init == 1` if and only if there is exactly one thread with exclusive
+/// access to `self.value`.
+/// - `init == 2` if and only if `value` is initialized and valid for shared
+/// access.
+///
+/// # Example
+///
+/// ```
+/// # use kernel::sync::SetOnce;
+/// let value = SetOnce::new();
+/// assert_eq!(None, value.as_ref());
+///
+/// let status = value.populate(42u8);
+/// assert_eq!(true, status);
+/// assert_eq!(Some(&42u8), value.as_ref());
+/// assert_eq!(Some(42u8), value.copy());
+///
+/// let status = value.populate(101u8);
+/// assert_eq!(false, status);
+/// assert_eq!(Some(&42u8), value.as_ref());
+/// assert_eq!(Some(42u8), value.copy());
+/// ```
+pub struct SetOnce<T> {
+ init: Atomic<u32>,
+ value: UnsafeCell<MaybeUninit<T>>,
+}
+
+impl<T> Default for SetOnce<T> {
+ fn default() -> Self {
+ Self::new()
+ }
+}
+
+impl<T> SetOnce<T> {
+ /// Create a new [`SetOnce`].
+ ///
+ /// The returned instance will be empty.
+ pub const fn new() -> Self {
+ // INVARIANT: The container is empty and we initialize `init` to `0`.
+ Self {
+ value: UnsafeCell::new(MaybeUninit::uninit()),
+ init: Atomic::new(0),
+ }
+ }
+
+ /// Get a reference to the contained object.
+ ///
+ /// Returns [`None`] if this [`SetOnce`] is empty.
+ pub fn as_ref(&self) -> Option<&T> {
+ if self.init.load(Acquire) == 2 {
+ // SAFETY: By the type invariants of `Self`, `self.init == 2` means that `self.value`
+ // is initialized and valid for shared access.
+ Some(unsafe { &*self.value.get().cast() })
+ } else {
+ None
+ }
+ }
+
+ /// Populate the [`SetOnce`].
+ ///
+ /// Returns `true` if the [`SetOnce`] was successfully populated.
+ pub fn populate(&self, value: T) -> bool {
+ // INVARIANT: If the swap succeeds:
+ // - We increase `init`.
+ // - We write the valid value `1` to `init`.
+ // - Only one thread can succeed in this write, so we have exclusive access after the
+ // write.
+ if let Ok(0) = self.init.cmpxchg(0, 1, Relaxed) {
+ // SAFETY: By the type invariants of `Self`, the fact that we succeeded in writing `1`
+ // to `self.init` means we obtained exclusive access to `self.value`.
+ unsafe { core::ptr::write(self.value.get().cast(), value) };
+ // INVARIANT:
+ // - We increase `init`.
+ // - We write the valid value `2` to `init`.
+ // - We release our exclusive access to `self.value` and it is now valid for shared
+ // access.
+ self.init.store(2, Release);
+ true
+ } else {
+ false
+ }
+ }
+
+ /// Get a copy of the contained object.
+ ///
+ /// Returns [`None`] if the [`SetOnce`] is empty.
+ pub fn copy(&self) -> Option<T>
+ where
+ T: Copy,
+ {
+ self.as_ref().copied()
+ }
+}
+
+impl<T> Drop for SetOnce<T> {
+ fn drop(&mut self) {
+ if *self.init.get_mut() == 2 {
+ let value = self.value.get_mut();
+ // SAFETY: By the type invariants of `Self`, `self.init == 2` means that `self.value`
+ // contains a valid value. We have exclusive access, as we hold a `mut` reference to
+ // `self`.
+ unsafe { value.assume_init_drop() };
+ }
+ }
+}
--
2.47.2
^ permalink raw reply related
* [PATCH v18 7/7] modules: add rust modules files to MAINTAINERS
From: Andreas Hindborg @ 2025-09-24 12:39 UTC (permalink / raw)
To: Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Alice Ryhl, Masahiro Yamada,
Nathan Chancellor, Luis Chamberlain, Danilo Krummrich,
Benno Lossin, Daniel Gomez, Benno Lossin, Nicolas Schier
Cc: Trevor Gross, Adam Bratschi-Kaye, rust-for-linux, linux-kernel,
linux-kbuild, Petr Pavlu, Sami Tolvanen, Daniel Gomez,
Simona Vetter, Greg KH, Fiona Behrens, Daniel Almeida,
linux-modules, Andreas Hindborg
In-Reply-To: <20250924-module-params-v3-v18-0-bf512c35d910@kernel.org>
The module subsystem people agreed to maintain rust support for modules
[1]. Thus, add entries for relevant files to modules entry in MAINTAINERS.
Link: https://lore.kernel.org/all/0d9e596a-5316-4e00-862b-fd77552ae4b5@suse.com/ [1]
Acked-by: Daniel Gomez <da.gomez@samsung.com>
Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
---
MAINTAINERS | 2 ++
1 file changed, 2 insertions(+)
diff --git a/MAINTAINERS b/MAINTAINERS
index d69021b88aef0..55e3bf16ea0a8 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -17136,6 +17136,8 @@ F: include/linux/module*.h
F: kernel/module/
F: lib/test_kmod.c
F: lib/tests/module/
+F: rust/kernel/module_param.rs
+F: rust/macros/module.rs
F: scripts/module*
F: tools/testing/selftests/kmod/
F: tools/testing/selftests/module/
--
2.47.2
^ permalink raw reply related
* [PATCH v18 5/7] rust: module: update the module macro with module parameter support
From: Andreas Hindborg @ 2025-09-24 12:39 UTC (permalink / raw)
To: Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Alice Ryhl, Masahiro Yamada,
Nathan Chancellor, Luis Chamberlain, Danilo Krummrich,
Benno Lossin, Daniel Gomez, Benno Lossin, Nicolas Schier
Cc: Trevor Gross, Adam Bratschi-Kaye, rust-for-linux, linux-kernel,
linux-kbuild, Petr Pavlu, Sami Tolvanen, Daniel Gomez,
Simona Vetter, Greg KH, Fiona Behrens, Daniel Almeida,
linux-modules, Andreas Hindborg
In-Reply-To: <20250924-module-params-v3-v18-0-bf512c35d910@kernel.org>
Allow module parameters to be declared in the rust `module!` macro.
Reviewed-by: Benno Lossin <lossin@kernel.org>
Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
---
rust/macros/helpers.rs | 25 +++++++
rust/macros/lib.rs | 31 +++++++++
rust/macros/module.rs | 178 ++++++++++++++++++++++++++++++++++++++++++++++---
3 files changed, 224 insertions(+), 10 deletions(-)
diff --git a/rust/macros/helpers.rs b/rust/macros/helpers.rs
index e2602be402c10..365d7eb499c08 100644
--- a/rust/macros/helpers.rs
+++ b/rust/macros/helpers.rs
@@ -10,6 +10,17 @@ pub(crate) fn try_ident(it: &mut token_stream::IntoIter) -> Option<String> {
}
}
+pub(crate) fn try_sign(it: &mut token_stream::IntoIter) -> Option<char> {
+ let peek = it.clone().next();
+ match peek {
+ Some(TokenTree::Punct(punct)) if punct.as_char() == '-' => {
+ let _ = it.next();
+ Some(punct.as_char())
+ }
+ _ => None,
+ }
+}
+
pub(crate) fn try_literal(it: &mut token_stream::IntoIter) -> Option<String> {
if let Some(TokenTree::Literal(literal)) = it.next() {
Some(literal.to_string())
@@ -103,3 +114,17 @@ pub(crate) fn file() -> String {
proc_macro::Span::call_site().file()
}
}
+
+/// Parse a token stream of the form `expected_name: "value",` and return the
+/// string in the position of "value".
+///
+/// # Panics
+///
+/// - On parse error.
+pub(crate) fn expect_string_field(it: &mut token_stream::IntoIter, expected_name: &str) -> String {
+ assert_eq!(expect_ident(it), expected_name);
+ assert_eq!(expect_punct(it), ':');
+ let string = expect_string(it);
+ assert_eq!(expect_punct(it), ',');
+ string
+}
diff --git a/rust/macros/lib.rs b/rust/macros/lib.rs
index fa847cf3a9b5f..2fb520dc930af 100644
--- a/rust/macros/lib.rs
+++ b/rust/macros/lib.rs
@@ -28,6 +28,30 @@
/// The `type` argument should be a type which implements the [`Module`]
/// trait. Also accepts various forms of kernel metadata.
///
+/// The `params` field describe module parameters. Each entry has the form
+///
+/// ```ignore
+/// parameter_name: type {
+/// default: default_value,
+/// description: "Description",
+/// }
+/// ```
+///
+/// `type` may be one of
+///
+/// - [`i8`]
+/// - [`u8`]
+/// - [`i8`]
+/// - [`u8`]
+/// - [`i16`]
+/// - [`u16`]
+/// - [`i32`]
+/// - [`u32`]
+/// - [`i64`]
+/// - [`u64`]
+/// - [`isize`]
+/// - [`usize`]
+///
/// C header: [`include/linux/moduleparam.h`](srctree/include/linux/moduleparam.h)
///
/// [`Module`]: ../kernel/trait.Module.html
@@ -44,6 +68,12 @@
/// description: "My very own kernel module!",
/// license: "GPL",
/// alias: ["alternate_module_name"],
+/// params: {
+/// my_parameter: i64 {
+/// default: 1,
+/// description: "This parameter has a default of 1",
+/// },
+/// },
/// }
///
/// struct MyModule(i32);
@@ -52,6 +82,7 @@
/// fn init(_module: &'static ThisModule) -> Result<Self> {
/// let foo: i32 = 42;
/// pr_info!("I contain: {}\n", foo);
+/// pr_info!("i32 param is: {}\n", module_parameters::my_parameter.read());
/// Ok(Self(foo))
/// }
/// }
diff --git a/rust/macros/module.rs b/rust/macros/module.rs
index cbf3ac0a8f7ba..d62e9c1e2a898 100644
--- a/rust/macros/module.rs
+++ b/rust/macros/module.rs
@@ -26,6 +26,7 @@ struct ModInfoBuilder<'a> {
module: &'a str,
counter: usize,
buffer: String,
+ param_buffer: String,
}
impl<'a> ModInfoBuilder<'a> {
@@ -34,10 +35,11 @@ fn new(module: &'a str) -> Self {
module,
counter: 0,
buffer: String::new(),
+ param_buffer: String::new(),
}
}
- fn emit_base(&mut self, field: &str, content: &str, builtin: bool) {
+ fn emit_base(&mut self, field: &str, content: &str, builtin: bool, param: bool) {
let string = if builtin {
// Built-in modules prefix their modinfo strings by `module.`.
format!(
@@ -51,8 +53,14 @@ fn emit_base(&mut self, field: &str, content: &str, builtin: bool) {
format!("{field}={content}\0")
};
+ let buffer = if param {
+ &mut self.param_buffer
+ } else {
+ &mut self.buffer
+ };
+
write!(
- &mut self.buffer,
+ buffer,
"
{cfg}
#[doc(hidden)]
@@ -75,20 +83,119 @@ fn emit_base(&mut self, field: &str, content: &str, builtin: bool) {
self.counter += 1;
}
- fn emit_only_builtin(&mut self, field: &str, content: &str) {
- self.emit_base(field, content, true)
+ fn emit_only_builtin(&mut self, field: &str, content: &str, param: bool) {
+ self.emit_base(field, content, true, param)
}
- fn emit_only_loadable(&mut self, field: &str, content: &str) {
- self.emit_base(field, content, false)
+ fn emit_only_loadable(&mut self, field: &str, content: &str, param: bool) {
+ self.emit_base(field, content, false, param)
}
fn emit(&mut self, field: &str, content: &str) {
- self.emit_only_builtin(field, content);
- self.emit_only_loadable(field, content);
+ self.emit_internal(field, content, false);
+ }
+
+ fn emit_internal(&mut self, field: &str, content: &str, param: bool) {
+ self.emit_only_builtin(field, content, param);
+ self.emit_only_loadable(field, content, param);
+ }
+
+ fn emit_param(&mut self, field: &str, param: &str, content: &str) {
+ let content = format!("{param}:{content}", param = param, content = content);
+ self.emit_internal(field, &content, true);
+ }
+
+ fn emit_params(&mut self, info: &ModuleInfo) {
+ let Some(params) = &info.params else {
+ return;
+ };
+
+ for param in params {
+ let ops = param_ops_path(¶m.ptype);
+
+ // Note: The spelling of these fields is dictated by the user space
+ // tool `modinfo`.
+ self.emit_param("parmtype", ¶m.name, ¶m.ptype);
+ self.emit_param("parm", ¶m.name, ¶m.description);
+
+ write!(
+ self.param_buffer,
+ "
+ pub(crate) static {param_name}:
+ ::kernel::module_param::ModuleParamAccess<{param_type}> =
+ ::kernel::module_param::ModuleParamAccess::new({param_default});
+
+ const _: () = {{
+ #[link_section = \"__param\"]
+ #[used]
+ static __{module_name}_{param_name}_struct:
+ ::kernel::module_param::KernelParam =
+ ::kernel::module_param::KernelParam::new(
+ ::kernel::bindings::kernel_param {{
+ name: if ::core::cfg!(MODULE) {{
+ ::kernel::c_str!(\"{param_name}\").as_bytes_with_nul()
+ }} else {{
+ ::kernel::c_str!(\"{module_name}.{param_name}\")
+ .as_bytes_with_nul()
+ }}.as_ptr(),
+ // SAFETY: `__this_module` is constructed by the kernel at load
+ // time and will not be freed until the module is unloaded.
+ #[cfg(MODULE)]
+ mod_: unsafe {{
+ core::ptr::from_ref(&::kernel::bindings::__this_module)
+ .cast_mut()
+ }},
+ #[cfg(not(MODULE))]
+ mod_: ::core::ptr::null_mut(),
+ ops: core::ptr::from_ref(&{ops}),
+ perm: 0, // Will not appear in sysfs
+ level: -1,
+ flags: 0,
+ __bindgen_anon_1: ::kernel::bindings::kernel_param__bindgen_ty_1 {{
+ arg: {param_name}.as_void_ptr()
+ }},
+ }}
+ );
+ }};
+ ",
+ module_name = info.name,
+ param_type = param.ptype,
+ param_default = param.default,
+ param_name = param.name,
+ ops = ops,
+ )
+ .unwrap();
+ }
+ }
+}
+
+fn param_ops_path(param_type: &str) -> &'static str {
+ match param_type {
+ "i8" => "::kernel::module_param::PARAM_OPS_I8",
+ "u8" => "::kernel::module_param::PARAM_OPS_U8",
+ "i16" => "::kernel::module_param::PARAM_OPS_I16",
+ "u16" => "::kernel::module_param::PARAM_OPS_U16",
+ "i32" => "::kernel::module_param::PARAM_OPS_I32",
+ "u32" => "::kernel::module_param::PARAM_OPS_U32",
+ "i64" => "::kernel::module_param::PARAM_OPS_I64",
+ "u64" => "::kernel::module_param::PARAM_OPS_U64",
+ "isize" => "::kernel::module_param::PARAM_OPS_ISIZE",
+ "usize" => "::kernel::module_param::PARAM_OPS_USIZE",
+ t => panic!("Unsupported parameter type {}", t),
}
}
+fn expect_param_default(param_it: &mut token_stream::IntoIter) -> String {
+ assert_eq!(expect_ident(param_it), "default");
+ assert_eq!(expect_punct(param_it), ':');
+ let sign = try_sign(param_it);
+ let default = try_literal(param_it).expect("Expected default param value");
+ assert_eq!(expect_punct(param_it), ',');
+ let mut value = sign.map(String::from).unwrap_or_default();
+ value.push_str(&default);
+ value
+}
+
#[derive(Debug, Default)]
struct ModuleInfo {
type_: String,
@@ -98,6 +205,50 @@ struct ModuleInfo {
description: Option<String>,
alias: Option<Vec<String>>,
firmware: Option<Vec<String>>,
+ params: Option<Vec<Parameter>>,
+}
+
+#[derive(Debug)]
+struct Parameter {
+ name: String,
+ ptype: String,
+ default: String,
+ description: String,
+}
+
+fn expect_params(it: &mut token_stream::IntoIter) -> Vec<Parameter> {
+ let params = expect_group(it);
+ assert_eq!(params.delimiter(), Delimiter::Brace);
+ let mut it = params.stream().into_iter();
+ let mut parsed = Vec::new();
+
+ loop {
+ let param_name = match it.next() {
+ Some(TokenTree::Ident(ident)) => ident.to_string(),
+ Some(_) => panic!("Expected Ident or end"),
+ None => break,
+ };
+
+ assert_eq!(expect_punct(&mut it), ':');
+ let param_type = expect_ident(&mut it);
+ let group = expect_group(&mut it);
+ assert_eq!(group.delimiter(), Delimiter::Brace);
+ assert_eq!(expect_punct(&mut it), ',');
+
+ let mut param_it = group.stream().into_iter();
+ let param_default = expect_param_default(&mut param_it);
+ let param_description = expect_string_field(&mut param_it, "description");
+ expect_end(&mut param_it);
+
+ parsed.push(Parameter {
+ name: param_name,
+ ptype: param_type,
+ default: param_default,
+ description: param_description,
+ })
+ }
+
+ parsed
}
impl ModuleInfo {
@@ -112,6 +263,7 @@ fn parse(it: &mut token_stream::IntoIter) -> Self {
"license",
"alias",
"firmware",
+ "params",
];
const REQUIRED_KEYS: &[&str] = &["type", "name", "license"];
let mut seen_keys = Vec::new();
@@ -137,6 +289,7 @@ fn parse(it: &mut token_stream::IntoIter) -> Self {
"license" => info.license = expect_string_ascii(it),
"alias" => info.alias = Some(expect_string_array(it)),
"firmware" => info.firmware = Some(expect_string_array(it)),
+ "params" => info.params = Some(expect_params(it)),
_ => panic!("Unknown key \"{key}\". Valid keys are: {EXPECTED_KEYS:?}."),
}
@@ -199,7 +352,9 @@ pub(crate) fn module(ts: TokenStream) -> TokenStream {
// Built-in modules also export the `file` modinfo string.
let file =
std::env::var("RUST_MODFILE").expect("Unable to fetch RUST_MODFILE environmental variable");
- modinfo.emit_only_builtin("file", &file);
+ modinfo.emit_only_builtin("file", &file, false);
+
+ modinfo.emit_params(&info);
format!(
"
@@ -363,15 +518,18 @@ unsafe fn __exit() {{
__MOD.assume_init_drop();
}}
}}
-
{modinfo}
}}
}}
+ mod module_parameters {{
+ {params}
+ }}
",
type_ = info.type_,
name = info.name,
ident = ident,
modinfo = modinfo.buffer,
+ params = modinfo.param_buffer,
initcall_section = ".initcall6.init"
)
.parse()
--
2.47.2
^ permalink raw reply related
* [PATCH v18 3/7] rust: introduce module_param module
From: Andreas Hindborg @ 2025-09-24 12:39 UTC (permalink / raw)
To: Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Alice Ryhl, Masahiro Yamada,
Nathan Chancellor, Luis Chamberlain, Danilo Krummrich,
Benno Lossin, Daniel Gomez, Benno Lossin, Nicolas Schier
Cc: Trevor Gross, Adam Bratschi-Kaye, rust-for-linux, linux-kernel,
linux-kbuild, Petr Pavlu, Sami Tolvanen, Daniel Gomez,
Simona Vetter, Greg KH, Fiona Behrens, Daniel Almeida,
linux-modules, Andreas Hindborg
In-Reply-To: <20250924-module-params-v3-v18-0-bf512c35d910@kernel.org>
Add types and traits for interfacing the C moduleparam API.
Reviewed-by: Benno Lossin <lossin@kernel.org>
Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
---
rust/kernel/lib.rs | 1 +
rust/kernel/module_param.rs | 181 ++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 182 insertions(+)
diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
index fef97f2a50984..e34b377d93b2e 100644
--- a/rust/kernel/lib.rs
+++ b/rust/kernel/lib.rs
@@ -98,6 +98,7 @@
pub mod list;
pub mod miscdevice;
pub mod mm;
+pub mod module_param;
#[cfg(CONFIG_NET)]
pub mod net;
pub mod of;
diff --git a/rust/kernel/module_param.rs b/rust/kernel/module_param.rs
new file mode 100644
index 0000000000000..e7d5c930a467d
--- /dev/null
+++ b/rust/kernel/module_param.rs
@@ -0,0 +1,181 @@
+// SPDX-License-Identifier: GPL-2.0
+
+//! Support for module parameters.
+//!
+//! C header: [`include/linux/moduleparam.h`](srctree/include/linux/moduleparam.h)
+
+use crate::prelude::*;
+use crate::str::BStr;
+use bindings;
+use kernel::sync::SetOnce;
+
+/// Newtype to make `bindings::kernel_param` [`Sync`].
+#[repr(transparent)]
+#[doc(hidden)]
+pub struct KernelParam(bindings::kernel_param);
+
+impl KernelParam {
+ #[doc(hidden)]
+ pub const fn new(val: bindings::kernel_param) -> Self {
+ Self(val)
+ }
+}
+
+// SAFETY: C kernel handles serializing access to this type. We never access it
+// from Rust module.
+unsafe impl Sync for KernelParam {}
+
+/// Types that can be used for module parameters.
+// NOTE: This trait is `Copy` because drop could produce unsoundness during teardown.
+pub trait ModuleParam: Sized + Copy {
+ /// Parse a parameter argument into the parameter value.
+ fn try_from_param_arg(arg: &BStr) -> Result<Self>;
+}
+
+/// Set the module parameter from a string.
+///
+/// Used to set the parameter value at kernel initialization, when loading
+/// the module or when set through `sysfs`.
+///
+/// See `struct kernel_param_ops.set`.
+///
+/// # Safety
+///
+/// - If `val` is non-null then it must point to a valid null-terminated string that must be valid
+/// for reads for the duration of the call.
+/// - `param` must be a pointer to a `bindings::kernel_param` initialized by the rust module macro.
+/// The pointee must be valid for reads for the duration of the call.
+///
+/// # Note
+///
+/// - The safety requirements are satisfied by C API contract when this function is invoked by the
+/// module subsystem C code.
+/// - Currently, we only support read-only parameters that are not readable from `sysfs`. Thus, this
+/// function is only called at kernel initialization time, or at module load time, and we have
+/// exclusive access to the parameter for the duration of the function.
+///
+/// [`module!`]: macros::module
+unsafe extern "C" fn set_param<T>(val: *const c_char, param: *const bindings::kernel_param) -> c_int
+where
+ T: ModuleParam,
+{
+ // NOTE: If we start supporting arguments without values, val _is_ allowed
+ // to be null here.
+ if val.is_null() {
+ // TODO: Use pr_warn_once available.
+ crate::pr_warn!("Null pointer passed to `module_param::set_param`");
+ return EINVAL.to_errno();
+ }
+
+ // SAFETY: By function safety requirement, val is non-null, null-terminated
+ // and valid for reads for the duration of this function.
+ let arg = unsafe { CStr::from_char_ptr(val) };
+
+ crate::error::from_result(|| {
+ let new_value = T::try_from_param_arg(arg)?;
+
+ // SAFETY: By function safety requirements, this access is safe.
+ let container = unsafe { &*((*param).__bindgen_anon_1.arg.cast::<SetOnce<T>>()) };
+
+ container
+ .populate(new_value)
+ .then_some(0)
+ .ok_or(kernel::error::code::EEXIST)
+ })
+}
+
+macro_rules! impl_int_module_param {
+ ($ty:ident) => {
+ impl ModuleParam for $ty {
+ fn try_from_param_arg(arg: &BStr) -> Result<Self> {
+ <$ty as crate::str::parse_int::ParseInt>::from_str(arg)
+ }
+ }
+ };
+}
+
+impl_int_module_param!(i8);
+impl_int_module_param!(u8);
+impl_int_module_param!(i16);
+impl_int_module_param!(u16);
+impl_int_module_param!(i32);
+impl_int_module_param!(u32);
+impl_int_module_param!(i64);
+impl_int_module_param!(u64);
+impl_int_module_param!(isize);
+impl_int_module_param!(usize);
+
+/// A wrapper for kernel parameters.
+///
+/// This type is instantiated by the [`module!`] macro when module parameters are
+/// defined. You should never need to instantiate this type directly.
+///
+/// Note: This type is `pub` because it is used by module crates to access
+/// parameter values.
+pub struct ModuleParamAccess<T> {
+ value: SetOnce<T>,
+ default: T,
+}
+
+// SAFETY: We only create shared references to the contents of this container,
+// so if `T` is `Sync`, so is `ModuleParamAccess`.
+unsafe impl<T: Sync> Sync for ModuleParamAccess<T> {}
+
+impl<T> ModuleParamAccess<T> {
+ #[doc(hidden)]
+ pub const fn new(default: T) -> Self {
+ Self {
+ value: SetOnce::new(),
+ default,
+ }
+ }
+
+ /// Get a shared reference to the parameter value.
+ // Note: When sysfs access to parameters are enabled, we have to pass in a
+ // held lock guard here.
+ pub fn value(&self) -> &T {
+ self.value.as_ref().unwrap_or(&self.default)
+ }
+
+ /// Get a mutable pointer to `self`.
+ ///
+ /// NOTE: In most cases it is not safe deref the returned pointer.
+ pub const fn as_void_ptr(&self) -> *mut c_void {
+ core::ptr::from_ref(self).cast_mut().cast()
+ }
+}
+
+#[doc(hidden)]
+/// Generate a static [`kernel_param_ops`](srctree/include/linux/moduleparam.h) struct.
+///
+/// # Examples
+///
+/// ```ignore
+/// make_param_ops!(
+/// /// Documentation for new param ops.
+/// PARAM_OPS_MYTYPE, // Name for the static.
+/// MyType // A type which implements [`ModuleParam`].
+/// );
+/// ```
+macro_rules! make_param_ops {
+ ($ops:ident, $ty:ty) => {
+ #[doc(hidden)]
+ pub static $ops: $crate::bindings::kernel_param_ops = $crate::bindings::kernel_param_ops {
+ flags: 0,
+ set: Some(set_param::<$ty>),
+ get: None,
+ free: None,
+ };
+ };
+}
+
+make_param_ops!(PARAM_OPS_I8, i8);
+make_param_ops!(PARAM_OPS_U8, u8);
+make_param_ops!(PARAM_OPS_I16, i16);
+make_param_ops!(PARAM_OPS_U16, u16);
+make_param_ops!(PARAM_OPS_I32, i32);
+make_param_ops!(PARAM_OPS_U32, u32);
+make_param_ops!(PARAM_OPS_I64, i64);
+make_param_ops!(PARAM_OPS_U64, u64);
+make_param_ops!(PARAM_OPS_ISIZE, isize);
+make_param_ops!(PARAM_OPS_USIZE, usize);
--
2.47.2
^ permalink raw reply related
* [PATCH v18 0/7] rust: extend `module!` macro with integer parameter support
From: Andreas Hindborg @ 2025-09-24 12:39 UTC (permalink / raw)
To: Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Alice Ryhl, Masahiro Yamada,
Nathan Chancellor, Luis Chamberlain, Danilo Krummrich,
Benno Lossin, Daniel Gomez, Benno Lossin, Nicolas Schier
Cc: Trevor Gross, Adam Bratschi-Kaye, rust-for-linux, linux-kernel,
linux-kbuild, Petr Pavlu, Sami Tolvanen, Daniel Gomez,
Simona Vetter, Greg KH, Fiona Behrens, Daniel Almeida,
linux-modules, Andreas Hindborg
Extend the `module!` macro with support module parameters. Also add some
string to integer parsing functions.
Based on the original module parameter support by Miguel [1],
later extended and generalized by Adam for more types [2][3].
Originally tracked at [4].
Link: https://github.com/Rust-for-Linux/linux/pull/7 [1]
Link: https://github.com/Rust-for-Linux/linux/pull/82 [2]
Link: https://github.com/Rust-for-Linux/linux/pull/87 [3]
Link: https://github.com/Rust-for-Linux/linux/issues/11 [4]
Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
---
Changes in v18:
- Rebase on Rust atomic patches (tip/master).
- Link to v17: https://lore.kernel.org/r/20250711-module-params-v3-v17-0-cf9b10d4923d@kernel.org
Changes in v17:
- Fix drop impl of `SetOnce` so that it works with `UnsafeCell<MaybeUninit<_>>`.
- Slightly reword safety framework in `SetOnce`.
- Rebase on atomic series v6.
- Link to v16: https://lore.kernel.org/r/20250709-module-params-v3-v16-0-4f926bcccb50@kernel.org
Changes in v16:
- Normalize imports in `set_once.rs`.
- Use `UnsafeCell<MaybeUninit<T>>` rather than `Opaque<T>` for `SetOnce`.
- Use regular load in drop of `SetOnce`.
- Update attribution paragraph in cover letter with details from Miguel.
- Remove stray TODO in `set_once.rs`
- Link to v15: https://lore.kernel.org/r/20250707-module-params-v3-v15-0-c1f4269a57b9@kernel.org
Changes in v15:
- Rebase on v6.16-rc5.
- Dedent code in module macro for better formatting.
- Rename `OnceLock` to `SetOnce`.
- Use "being initialized" rather than "being mutably accessed" when
describing initialization state of `OnceLock`.
- Use `Relaxed` ordering when transitioning to exclusive access in
`OnceLock`.
- Add drop implementation for `OnceLock`.
- Re-export `OnceLock` from `kernel::sync` module.
- Improve indentation of in macro code. Prefix `cfg` to `::core::cfg` in
macro code.
- Use `core::ptr::from_ref` rather than `as` casts.
- Hide `KernelParam` instances behind `const _: ()` blocks.
- Rename `ModuleParamAccess::get` to `ModuleParamAccess::value`.
- Rename `RacyKernelParam` to `KernelParam`.
- Remove `ModuleParam::Value`.
- Move `copy` implementation of `OnceLock`.
- Update safety comments and invariants of `OnceLock`.
- Link to v14: https://lore.kernel.org/r/20250702-module-params-v3-v14-0-5b1cc32311af@kernel.org
Note: This series now depends on the atomics series [1].
[1] https://lore.kernel.org/all/20250618164934.19817-1-boqun.feng@gmail.com
Changes in v14:
- Remove unnecessary `crate::` prefix from `module_param::set_param`.
- Make `FromStrRadix` safe again by moving unsafe blocks to macro implementation (thanks Benno).
- Use `core::ptr::write` in `set_param` and drop safety requirement regarding initialization.
- Add a TODO to use `SyncUnsafeCell` for `ModuleParamAccess` when available.
- Add a NOTE regarding `Copy` bound on `ModuleParam`.
- Remove `'static` lifetime qualifier from `ModuleParam::try_from_param_arg` argument.
- Fix a typo in the safety requirements for `set_param`.
- Remove unused `#[macro_export]` attribute.
- Remove obsolete documentation for `ModuleParam::try_from_param_arg`.
- Make `RacyKernelParam` tuple field private.
- Introduce `OnceLock` and use that to synchronize population of parameter values.
- Link to v13: https://lore.kernel.org/r/20250612-module-params-v3-v13-0-bc219cd1a3f8@kernel.org
Changes in v13:
- remove absolute path for `ffi` types.
- Split patch 2 into 4 separate patches.
- Overhaul safety framework for `set_param`.
- Remove generated docs for `kernel_param_ops`.
- Move `parse_int` to separate file.
- Rebase on v6.16-rc1
- Link to v12: https://lore.kernel.org/r/20250506-module-params-v3-v12-0-c04d80c8a2b1@kernel.org
Changes in v12:
- Assign through pointer rather than using `core::ptr::replace`.
- Prevent a potential use-after-free during module teardown.
- Link to v11: https://lore.kernel.org/r/20250502-module-params-v3-v11-0-6096875a2b78@kernel.org
Changes in v11:
- Apply a few nits from Miguel.
- Link to v10: https://lore.kernel.org/r/20250501-module-params-v3-v10-0-4da485d343d5@kernel.org
Changes in v10:
- Apply fixups from Miguel:
- Add integer type suffixes to `assert!` in tests.
- Fix links to docs.kernel.org.
- Applyy markdown and intra-doc links where possible.
- Change to `///` for `mod` docs.
- Slightly reword a comment.
- Pluralize "Examples" section name.
- Hide `use`s in example.
- Removed `#[expect]` for the `rusttest` target.
- Link to v9: https://lore.kernel.org/r/20250321-module-params-v3-v9-0-28b905f2e345@kernel.org
Changes in v9:
- Remove UB when parsing the minimum integer values.
- Make `FromStr` trait unsafe, since wrong implementations can cause UB.
- Drop patches that were applied to rust-next.
- Link to v8: https://lore.kernel.org/r/20250227-module-params-v3-v8-0-ceeee85d9347@kernel.org
Changes in v8:
- Change print statement in sample to better communicate parameter name.
- Use imperative mode in commit messages.
- Remove prefix path from `EINVAL`.
- Change `try_from_param_arg` to accept `&BStr` rather than `&[u8]`.
- Parse integers without 128 bit integer types.
- Seal trait `FromStrRadix`.
- Strengthen safety requirement of `set_param`.
- Remove comment about Display and `PAGE_SIZE`.
- Add note describing why `ModuleParamAccess` is pub.
- Typo and grammar fixes for documentation.
- Update MAINTAINERS with rust module files.
- Link to v7: https://lore.kernel.org/r/20250218-module-params-v3-v7-0-5e1afabcac1b@kernel.org
Changes in v7:
- Remove dependency on `pr_warn_once` patches, replace with TODO.
- Rework `ParseInt::from_str` to avoid allocating.
- Add a comment explaining how we parse "0".
- Change trait bound on `Index` impl for `BStr` to match std library approach.
- Link to v6: https://lore.kernel.org/r/20250211-module-params-v3-v6-0-24b297ddc43d@kernel.org
Changes in v6:
- Fix a bug that prevented parsing of negative default values for
parameters in the `module!` macro.
- Fix a bug that prevented parsing zero in `strip_radix`. Also add a
test case for this.
- Add `AsRef<BStr>` for `[u8]` and `BStr`.
- Use `impl AsRef<BStr>` as type of prefix in `BStr::strip_prefix`.
- Link to v5: https://lore.kernel.org/r/20250204-module-params-v3-v5-0-bf5ec2041625@kernel.org
Changes in v5:
- Fix a typo in a safety comment in `set_param`.
- Use a match statement in `parse_int::strip_radix`.
- Add an implementation of `Index` for `BStr`.
- Fix a logic inversion bug where parameters would not be parsed.
- Use `kernel::ffi::c_char` in `set_param` rather than the one in `core`.
- Use `kernel::c_str!` rather than `c"..."` literal in module macro.
- Rebase on v6.14-rc1.
- Link to v4: https://lore.kernel.org/r/20250109-module-params-v3-v4-0-c208bcfbe11f@kernel.org
Changes in v4:
- Add module maintainers to Cc list (sorry)
- Add a few missing [`doc_links`]
- Add panic section to `expect_string_field`
- Fix a typo in safety requirement of `module_params::free`
- Change `assert!` to `pr_warn_once!` in `module_params::set_param`
- Remove `module_params::get_param` and install null pointer instead
- Remove use of the unstable feature `sync_unsafe_cell`
- Link to v3: https://lore.kernel.org/r/20241213-module-params-v3-v3-0-485a015ac2cf@kernel.org
Changes in v3:
- use `SyncUnsafeCell` rather than `static mut` and simplify parameter access
- remove `Display` bound from `ModuleParam`
- automatically generate documentation for `PARAM_OPS_.*`
- remove `as *const _ as *mut_` phrasing
- inline parameter name in struct instantiation in `emit_params`
- move `RacyKernelParam` out of macro template
- use C string literals rather than byte string literals with explicit null
- template out `__{name}_{param_name}` in `emit_param`
- indent template in `emit_params`
- use let-else expression in `emit_params` to get rid of an indentation level
- document `expect_string_field`
- move invication of `impl_int_module_param` to be closer to macro def
- move attributes after docs in `make_param_ops`
- rename `impl_module_param` to impl_int_module_param`
- use `ty` instead of `ident` in `impl_parse_int`
- use `BStr` instead of `&str` for string manipulation
- move string parsing functions to seperate patch and add examples, fix bugs
- degrade comment about future support from doc comment to regular comment
- remove std lib path from `Sized` marker
- update documentation for `trait ModuleParam`
- Link to v2: https://lore.kernel.org/all/20240819133345.3438739-1-nmi@metaspace.dk/
Changes in v2:
- Remove support for params without values (`NOARG_ALLOWED`).
- Improve documentation for `try_from_param_arg`.
- Use prelude import.
- Refactor `try_from_param_arg` to return `Result`.
- Refactor `ParseInt::from_str` to return `Result`.
- Move C callable functions out of `ModuleParam` trait.
- Rename literal string field parser to `expect_string_field`.
- Move parameter parsing from generation to parsing stage.
- Use absolute type paths in macro code.
- Inline `kparam`and `read_func` values.
- Resolve TODO regarding alignment attributes.
- Remove unnecessary unsafe blocks in macro code.
- Improve error message for unrecognized parameter types.
- Do not use `self` receiver when reading parameter value.
- Add parameter documentation to `module!` macro.
- Use empty `enum` for parameter type.
- Use `addr_of_mut` to get address of parameter value variable.
- Enabled building of docs for for `module_param` module.
- Link to v1: https://lore.kernel.org/rust-for-linux/20240705111455.142790-1-nmi@metaspace.dk/
---
Andreas Hindborg (7):
rust: sync: add `SetOnce`
rust: str: add radix prefixed integer parsing functions
rust: introduce module_param module
rust: module: use a reference in macros::module::module
rust: module: update the module macro with module parameter support
rust: samples: add a module parameter to the rust_minimal sample
modules: add rust modules files to MAINTAINERS
MAINTAINERS | 2 +
rust/kernel/lib.rs | 1 +
rust/kernel/module_param.rs | 181 ++++++++++++++++++++++++++++++++++++++++
rust/kernel/str.rs | 2 +
rust/kernel/str/parse_int.rs | 148 +++++++++++++++++++++++++++++++++
rust/kernel/sync.rs | 2 +
rust/kernel/sync/set_once.rs | 125 ++++++++++++++++++++++++++++
rust/macros/helpers.rs | 25 ++++++
rust/macros/lib.rs | 31 +++++++
rust/macros/module.rs | 194 +++++++++++++++++++++++++++++++++++++++----
samples/rust/rust_minimal.rs | 10 +++
11 files changed, 703 insertions(+), 18 deletions(-)
---
base-commit: 103265a1a936cfe910c9ac0f0ab153f7dac818ba
change-id: 20241211-module-params-v3-ae7e5c8d8b5a
Best regards,
--
Andreas Hindborg <a.hindborg@kernel.org>
^ permalink raw reply
* Re: [PATCH 1/4] PCI: Support FIXUP quirks in modules
From: Petr Pavlu @ 2025-09-24 7:48 UTC (permalink / raw)
To: Brian Norris
Cc: Bjorn Helgaas, Luis Chamberlain, Daniel Gomez, linux-pci,
David Gow, Rae Moar, linux-kselftest, linux-kernel, linux-modules,
Johannes Berg, Sami Tolvanen, Richard Weinberger, Wei Liu,
Brendan Higgins, kunit-dev, Anton Ivanov, linux-um
In-Reply-To: <aNLb9g0AbBXZCJ4m@google.com>
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.
--
Cheers,
Petr
^ permalink raw reply
* Re: [PATCH v8 8/8] kbuild: vmlinux.unstripped should always depend on .vmlinux.export.o
From: Nicolas Schier @ 2025-09-24 6:39 UTC (permalink / raw)
To: Alexey Gladkov
Cc: Nathan Chancellor, Petr Pavlu, Luis Chamberlain, Sami Tolvanen,
Daniel Gomez, linux-kernel, linux-modules, linux-kbuild
In-Reply-To: <0e63a9c7741fe8217e4fd7c60afcf057ffa2ef5a.1758182101.git.legion@kernel.org>
On Thu, Sep 18, 2025 at 10:05:52AM +0200, Alexey Gladkov wrote:
> Since .vmlinux.export.c is used to add generated by modpost modaliases
> for builtin modules the .vmlinux.export.o is no longer optional and
> should always be created. The generation of this file is not dependent
> on CONFIG_MODULES.
>
> Signed-off-by: Alexey Gladkov <legion@kernel.org>
> ---
> scripts/Makefile.vmlinux | 9 ++-------
> scripts/link-vmlinux.sh | 5 +----
> 2 files changed, 3 insertions(+), 11 deletions(-)
>
Reviewed-by: Nicolas Schier <nsc@kernel.org>
--
Nicolas
^ permalink raw reply
* Re: [PATCH v8 7/8] modpost: Create modalias for builtin modules
From: Nicolas Schier @ 2025-09-24 6:38 UTC (permalink / raw)
To: Alexey Gladkov
Cc: Nathan Chancellor, Petr Pavlu, Luis Chamberlain, Sami Tolvanen,
Daniel Gomez, linux-kernel, linux-modules, linux-kbuild,
Masahiro Yamada, Stephen Rothwell
In-Reply-To: <28d4da3b0e3fc8474142746bcf469e03752c3208.1758182101.git.legion@kernel.org>
On Thu, Sep 18, 2025 at 10:05:51AM +0200, Alexey Gladkov wrote:
> For some modules, modalias is generated using the modpost utility and
> the section is added to the module file.
>
> When a module is added inside vmlinux, modpost does not generate
> modalias for such modules and the information is lost.
>
> As a result kmod (which uses modules.builtin.modinfo in userspace)
> cannot determine that modalias is handled by a builtin kernel module.
>
> $ cat /sys/devices/pci0000:00/0000:00:14.0/modalias
> pci:v00008086d0000A36Dsv00001043sd00008694bc0Csc03i30
>
> $ modinfo xhci_pci
> name: xhci_pci
> filename: (builtin)
> license: GPL
> file: drivers/usb/host/xhci-pci
> description: xHCI PCI Host Controller Driver
>
> Missing modalias "pci:v*d*sv*sd*bc0Csc03i30*" which will be generated by
> modpost if the module is built separately.
>
> To fix this it is necessary to generate the same modalias for vmlinux as
> for the individual modules. Fortunately '.vmlinux.export.o' is already
> generated from which '.modinfo' can be extracted in the same way as for
> vmlinux.o.
>
> Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
> Signed-off-by: Alexey Gladkov <legion@kernel.org>
> Tested-by: Stephen Rothwell <sfr@canb.auug.org.au>
> ---
> include/linux/module.h | 4 ----
> scripts/Makefile.vmlinux | 4 +++-
> scripts/mksysmap | 3 +++
> scripts/mod/file2alias.c | 19 ++++++++++++++++++-
> scripts/mod/modpost.c | 15 +++++++++++++++
> scripts/mod/modpost.h | 2 ++
> 6 files changed, 41 insertions(+), 6 deletions(-)
>
Reviewed-by: Nicolas Schier <nsc@kernel.org>
--
Nicolas
^ permalink raw reply
* Re: [PATCH v8 6/8] modpost: Add modname to mod_device_table alias
From: Nicolas Schier @ 2025-09-24 6:31 UTC (permalink / raw)
To: Alexey Gladkov
Cc: Nathan Chancellor, Petr Pavlu, Luis Chamberlain, Sami Tolvanen,
Daniel Gomez, linux-kernel, linux-modules, linux-kbuild,
Miguel Ojeda, Andreas Hindborg, Danilo Krummrich, Alex Gaynor,
rust-for-linux
In-Reply-To: <1a0d0bd87a4981d465b9ed21e14f4e78eaa03ded.1758182101.git.legion@kernel.org>
On Thu, Sep 18, 2025 at 10:05:50AM +0200, Alexey Gladkov wrote:
> At this point, if a symbol is compiled as part of the kernel,
> information about which module the symbol belongs to is lost.
>
> To save this it is possible to add the module name to the alias name.
> It's not very pretty, but it's possible for now.
>
> Cc: Miguel Ojeda <ojeda@kernel.org>
> Cc: Andreas Hindborg <a.hindborg@kernel.org>
> Cc: Danilo Krummrich <dakr@kernel.org>
> Cc: Alex Gaynor <alex.gaynor@gmail.com>
> Cc: rust-for-linux@vger.kernel.org
> Signed-off-by: Alexey Gladkov <legion@kernel.org>
> Acked-by: Danilo Krummrich <dakr@kernel.org>
> ---
> include/linux/module.h | 14 +++++++++++++-
> rust/kernel/device_id.rs | 8 ++++----
> scripts/mod/file2alias.c | 15 ++++++++++++---
> 3 files changed, 29 insertions(+), 8 deletions(-)
>
Acked-by: Nicolas Schier <nsc@kernel.org>
--
Nicolas
^ permalink raw reply
* Re: [PATCH v8 3/8] kbuild: keep .modinfo section in vmlinux.unstripped
From: Nicolas Schier @ 2025-09-24 6:29 UTC (permalink / raw)
To: Alexey Gladkov
Cc: Nathan Chancellor, Petr Pavlu, Luis Chamberlain, Sami Tolvanen,
Daniel Gomez, linux-kernel, linux-modules, linux-kbuild,
Masahiro Yamada
In-Reply-To: <aaf67c07447215463300fccaa758904bac42f992.1758182101.git.legion@kernel.org>
On Thu, Sep 18, 2025 at 10:05:47AM +0200, Alexey Gladkov wrote:
> From: Masahiro Yamada <masahiroy@kernel.org>
>
> Keep the .modinfo section during linking, but strip it from the final
> vmlinux.
>
> Adjust scripts/mksysmap to exclude modinfo symbols from kallsyms.
>
> This change will allow the next commit to extract the .modinfo section
> from the vmlinux.unstripped intermediate.
>
> Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
> Signed-off-by: Alexey Gladkov <legion@kernel.org>
> ---
> include/asm-generic/vmlinux.lds.h | 2 +-
> scripts/Makefile.vmlinux | 7 +++++--
> scripts/mksysmap | 3 +++
> 3 files changed, 9 insertions(+), 3 deletions(-)
>
Thanks!
Reviewed-by: Nicolas Schier <nsc@kernel.org>
--
Nicolas
^ permalink raw reply
* Re: [PATCH 1/4] PCI: Support FIXUP quirks in modules
From: Brian Norris @ 2025-09-23 17:42 UTC (permalink / raw)
To: Petr Pavlu
Cc: Bjorn Helgaas, Luis Chamberlain, Daniel Gomez, linux-pci,
David Gow, Rae Moar, linux-kselftest, linux-kernel, linux-modules,
Johannes Berg, Sami Tolvanen, Richard Weinberger, Wei Liu,
Brendan Higgins, kunit-dev, Anton Ivanov, linux-um
In-Reply-To: <c84d6952-7977-47cd-8f09-6ea223217337@suse.com>
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.
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.
> > --- 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().
Ack.
Thanks,
Brian
^ permalink raw reply
* Re: [PATCH 0/4] PCI: Add support and tests for FIXUP quirks in modules
From: Manivannan Sadhasivam @ 2025-09-23 16:20 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Brian Norris, Bjorn Helgaas, Luis Chamberlain, Petr Pavlu,
Daniel Gomez, linux-pci, David Gow, Rae Moar, linux-kselftest,
linux-kernel, linux-modules, Johannes Berg, Sami Tolvanen,
Richard Weinberger, Wei Liu, Brendan Higgins, kunit-dev,
Anton Ivanov, linux-um
In-Reply-To: <aNGR0x185VGHxSde@infradead.org>
On Mon, Sep 22, 2025 at 11:13:39AM -0700, Christoph Hellwig wrote:
> On Mon, Sep 15, 2025 at 11:41:37AM -0700, Brian Norris wrote:
> > I see fixups in controller drivers here:
> >
> > drivers/pci/controller/dwc/pci-imx6.c
> > drivers/pci/controller/dwc/pci-keystone.c
> > drivers/pci/controller/dwc/pcie-qcom.c
> > drivers/pci/controller/pci-loongson.c
> > drivers/pci/controller/pci-tegra.c
> > drivers/pci/controller/pcie-iproc-bcma.c
> > drivers/pci/controller/pcie-iproc.c
> >
> > Are any of those somehow wrong?
>
> When did we allow modular
> controller drivers anyway? That feels like a somewhat bad idea, too.
>
Why not? We currently only restrict the controller drivers implementing the
irqchip controller from being *removed* because of the IRQ disposal concern.
Other than that, I don't see why kernel should restrict building them as
modules.
- Mani
--
மணிவண்ணன் சதாசிவம்
^ permalink raw reply
* Re: [PATCH 1/4] PCI: Support FIXUP quirks in modules
From: Petr Pavlu @ 2025-09-23 12:55 UTC (permalink / raw)
To: Brian Norris
Cc: Bjorn Helgaas, Luis Chamberlain, Daniel Gomez, linux-pci,
David Gow, Rae Moar, linux-kselftest, linux-kernel, linux-modules,
Johannes Berg, Sami Tolvanen, Richard Weinberger, Wei Liu,
Brendan Higgins, kunit-dev, Anton Ivanov, linux-um
In-Reply-To: <20250912230208.967129-2-briannorris@chromium.org>
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@chromium.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
^ permalink raw reply
* Re: [PATCH 0/6] module: enable force unloading of modules that have crashed during init
From: Petr Pavlu @ 2025-09-23 7:21 UTC (permalink / raw)
To: julian-lagattuta
Cc: Luis Chamberlain, Sami Tolvanen, Daniel Gomez, linux-modules,
linux-kernel
In-Reply-To: <20250918201109.24620-2-julian.lagattuta@gmail.com>
On 9/18/25 10:11 PM, julian-lagattuta wrote:
> Running a module that encounters a fatal error during the initcall leaves
> the module in a state where it cannot be forcefully unloaded since it is
> "being used" despite there being no reason it couldn't be unloaded.
> This means that unloading the crashed module requires rebooting.
>
> This patch allows modules that have crashed during initialization to be
> forcefully unloaded with CONFIG_MODULE_FORCE_UNLOAD enabled.
Could you please explain the motivation for doing this in more detail?
I think we shouldn't attempt to do anything clever with modules that
crashed during initialization. Such a module can already leave the
system in an unstable state and trying to recover can cause even more
problems. For instance, I don't see how it is safe to call the module's
exit function.
--
Thanks,
Petr
^ permalink raw reply
* Re: [PATCH 0/4] PCI: Add support and tests for FIXUP quirks in modules
From: Brian Norris @ 2025-09-22 18:48 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Bjorn Helgaas, Luis Chamberlain, Petr Pavlu, Daniel Gomez,
linux-pci, David Gow, Rae Moar, linux-kselftest, linux-kernel,
linux-modules, Johannes Berg, Sami Tolvanen, Richard Weinberger,
Wei Liu, Brendan Higgins, kunit-dev, Anton Ivanov, linux-um
In-Reply-To: <aNGR0x185VGHxSde@infradead.org>
On Mon, Sep 22, 2025 at 11:13:39AM -0700, Christoph Hellwig wrote:
> Controller drivers are a special case I guess, but I'd rather still
> not open it up to any random driver.
I don't really see why this particular thing should develop restrictions
beyond "can it work in modules?", but if you have an idea for how to do
that reasonably, my ears are open.
> When did we allow modular
> controller drivers anyway?
An approximate count:
$ git grep tristate ./drivers/pci/controller/ | wc -l
39
There's been a steady trickle of module-related changes over the years.
And several modular controller drivers predate the
drivers/pci/controller/ creation in 2018 at commit 6e0832fa432e ("PCI:
Collect all native drivers under drivers/pci/controller/").
> That feels like a somewhat bad idea, too.
Any particular reason behind that feeling? Most other bus frameworks I'm
familiar with support modular drivers.
Brian
^ permalink raw reply
* Re: [PATCH 0/4] PCI: Add support and tests for FIXUP quirks in modules
From: Christoph Hellwig @ 2025-09-22 18:13 UTC (permalink / raw)
To: Brian Norris
Cc: Christoph Hellwig, Bjorn Helgaas, Luis Chamberlain, Petr Pavlu,
Daniel Gomez, linux-pci, David Gow, Rae Moar, linux-kselftest,
linux-kernel, linux-modules, Johannes Berg, Sami Tolvanen,
Richard Weinberger, Wei Liu, Brendan Higgins, kunit-dev,
Anton Ivanov, linux-um
In-Reply-To: <aMhd4REssOE-AlYw@google.com>
On Mon, Sep 15, 2025 at 11:41:37AM -0700, Brian Norris wrote:
> I see fixups in controller drivers here:
>
> drivers/pci/controller/dwc/pci-imx6.c
> drivers/pci/controller/dwc/pci-keystone.c
> drivers/pci/controller/dwc/pcie-qcom.c
> drivers/pci/controller/pci-loongson.c
> drivers/pci/controller/pci-tegra.c
> drivers/pci/controller/pcie-iproc-bcma.c
> drivers/pci/controller/pcie-iproc.c
>
> Are any of those somehow wrong?
Controller drivers are a special case I guess, but I'd rather still
not open it up to any random driver. When did we allow modular
controller drivers anyway? That feels like a somewhat bad idea, too.
^ permalink raw reply
* Re: [RFC PATCH 00/10] scalable symbol flags with __kflagstab
From: Petr Pavlu @ 2025-09-22 11:41 UTC (permalink / raw)
To: Sid Nayyar
Cc: Nathan Chancellor, Luis Chamberlain, Sami Tolvanen,
Nicolas Schier, Arnd Bergmann, linux-kbuild, linux-arch,
linux-modules, linux-kernel, Giuliano Procida,
Matthias Männich
In-Reply-To: <CA+OvW8bhWK7prmyQMMJ_VYBeGMbn_mNiamHhUgYuCsnht+LFtA@mail.gmail.com>
On 9/15/25 5:53 PM, Sid Nayyar wrote:
> On Mon, Sep 8, 2025 at 11:09 AM Petr Pavlu <petr.pavlu@suse.com> wrote:
>> This sounds reasonable to me. Do you have any numbers on hand that would
>> show the impact of extending __ksymtab?
>
> I did performance analysis for module loading. The kflagstab
> optimizes symbol search, which accounts for less than 2% of the
> average module load time. Therefore, this change does not translate
> into any meaningful gains (or losses) in module loading performance.
>
> On the binary size side, the on-disk size for vmlinux is somewhat
> inflated due to extra entries in .symtab and .strtab. Since these
> sections are not part of the final Image, the only increase in the
> in-memory size of the kernel is for the kflagstab itself. This new
> table occupies 1 byte for each symbol in the ksymtab.
This is useful information. However, I was specifically interested in
the impact of having the new flags field present as part of __ksymtab
(kernel_symbol), compared to keeping it in a separate section. Sorry for
not being clear.
I ran a small test to get a better understanding of the different sizes.
I used v6.17-rc6 together with the openSUSE x86_64 config [1], which is
fairly large. The resulting vmlinux.bin (no debuginfo) had an on-disk
size of 58 MiB, and included 5937 + 6589 (GPL-only) exported symbols.
The following table summarizes my measurements and calculations
regarding the sizes of all sections related to exported symbols:
| HAVE_ARCH_PREL32_RELOCATIONS | !HAVE_ARCH_PREL32_RELOCATIONS
Section | Base [B] | Ext. [B] | Sep. [B] | Base [B] | Ext. [B] | Sep. [B]
----------------------------------------------------------------------------------------
__ksymtab | 71244 | 200416 | 150312 | 142488 | 400832 | 300624
__ksymtab_gpl | 79068 | NA | NA | 158136 | NA | NA
__kcrctab | 23748 | 50104 | 50104 | 23748 | 50104 | 50104
__kcrctab_gpl | 26356 | NA | NA | 26356 | NA | NA
__ksymtab_strings | 253628 | 253628 | 253628 | 253628 | 253628 | 253628
__kflagstab | NA | NA | 12526 | NA | NA | 12526
----------------------------------------------------------------------------------------
Total | 454044 | 504148 | 466570 | 604356 | 704564 | 616882
Increase to base [%] | NA | 11.0 | 2.8 | NA | 16.6 | 2.1
The column "HAVE_ARCH_PREL32_RELOCATIONS -> Base" contains the numbers
that I measured. The rest of the values are calculated. The "Ext."
column represents the variant of extending __ksymtab, and the "Sep."
column represents the variant of having a separate __kflagstab. With
HAVE_ARCH_PREL32_RELOCATIONS, each kernel_symbol is 12 B in size and is
extended to 16 B. With !HAVE_ARCH_PREL32_RELOCATIONS, it is 24 B,
extended to 32 B. Note that this does not include the metadata needed to
relocate __ksymtab*, which is freed after the initial processing.
The base export data in this case totals 0.43 MiB. About 50% is used for
storing the names of exported symbols.
Adding __kflagstab as a separate section has a negligible impact, as
expected. When extending __ksymtab (kernel_symbol) instead, the worst
case with !HAVE_ARCH_PREL32_RELOCATIONS increases the export data size
by 16.6%.
Based on the above, I think introducing __kflagstab makes senses, as the
added complexity is minimal, although I feel we could probably also get
away with extending kernel_symbol.
>
>>> The Android Common Kernel source is compiled into what we call
>>> GKI (Generic Kernel Image), which consists of a kernel and a
>>> number of modules. We maintain a stable interface (based on CRCs and
>>> types) between the GKI components and vendor-specific modules
>>> (compiled by device manufacturers, e.g., for hardware-specific
>>> drivers) for the lifetime of a given GKI version.
>>>
>>> This interface is intentionally restricted to the minimal set of
>>> symbols required by the union of all vendor modules; our partners
>>> declare their requirements in symbol lists. Any additions to these
>>> lists are reviewed to ensure kernel internals are not overly exposed.
>>> For example, we restrict drivers from having the ability to open and
>>> read arbitrary files. This ABI boundary also allows us to evolve
>>> internal kernel types that are not exposed to vendor modules, for
>>> example, when a security fix requires a type to change.
>>>
>>> The mechanism we use for this is CONFIG_TRIM_UNUSED_KSYMS and
>>> CONFIG_UNUSED_KSYMS_WHITELIST. This results in a ksymtab
>>> containing two kinds of exported symbols: those explicitly required
>>> by vendors ("vendor-listed") and those only required by GKI modules
>>> ("GKI use only").
>>>
>>> On top of this, we have implemented symbol import protection
>>> (covered in patches 9/10 and 10/10). This feature prevents vendor
>>> modules from using symbols that are not on the vendor-listed
>>> whitelist. It is built on top of CONFIG_MODULE_SIG. GKI modules are
>>> signed with a specific key, while vendor modules are unsigned and thus
>>> treated as untrusted. This distinction allows signed GKI modules to
>>> use any symbol in the ksymtab, while unsigned vendor modules can only
>>> access the declared subset. This provides a significant layer of
>>> defense and security against potentially exploitable vendor module
>>> code.
>>
>> If I understand correctly, this is similar to the recently introduced
>> EXPORT_SYMBOL_FOR_MODULES() macro, but with a coarser boundary.
>>
>> I think that if the goal is to control the kABI scope and limit the use
>> of certain symbols only to GKI modules, then having the protection
>> depend on whether the module is signed is somewhat odd. It doesn't give
>> me much confidence if vendor modules are unsigned in the Android
>> ecosystem. I would expect that you want to improve this in the long
>> term.
>
> GKI modules are the only modules built in the same Kbuild as the
> kernel image, which Google builds and provides to partners. In
> contrast, vendor modules are built and packaged entirely by partners.
>
> Google signs GKI modules with ephemeral keys. Since partners do
> not have these keys, vendor modules are treated as unsigned by
> the kernel.
>
> To ensure the authenticity of these unsigned modules, partners
> package them into a separate image that becomes one of the boot
> partitions. This entire image is signed, and its signature is
> verified by the bootloader at boot time.
>
>> It would then make more sense to me if the protection was determined by
>> whether the module is in-tree (the "intree" flag in modinfo) or,
>> alternatively, if it is signed by a built-in trusted key. I feel this
>> way the feature could be potentially useful for other distributions that
>> care about the kABI scope and have ecosystems where vendor modules are
>> properly signed with some key. However, I'm not sure if this would still
>> work in your case.
>
> Partners can produce both in-tree and out-of-tree modules. We do not
> trust either type regarding symbol exposure, as there is no way to know
> exactly what sources were used. Furthermore, symbols exported via
> EXPORT_SYMBOL_FOR_MODULES can be accessed by any vendor module that
> mimics the GKI module name.
>
> Therefore, neither the in-tree flag nor the EXPORT_SYMBOL_FOR_MODULES
> mechanism provides a strong enough guarantee for the Android kernel to
> identify GKI modules.
>
> Only module signatures are sufficient to allow a module to access the
> full set of exported symbols. Unsigned vendor modules may only access
> the symbol subset declared ahead of time by partners.
This seems to answer why the in-tree flag is not sufficient for you.
However, I also suggested an alternative that the symbol protection
could be determined by whether the module is signed by a key from the
.builtin_trusted_keys keyring, as opposed to being signed by another key
reachable from the .secondary_trusted_keys keyring or being completely
unsigned.
Distributions can require that external modules be signed and allow
additional keys to be added as Machine Owner Keys, which can be made
reachable from .secondary_trusted_keys. Nonetheless, such distributions
might be still interested in limiting the number of symbols that such
external modules can use.
I think this option is worth considering, as it could potentially make
this symbol protection useful for other distributions as well.
>
> In case such symbol protection is not useful for the Linux community, I
> am happy to keep this as an Android-specific feature. However, I would
> urge you to at least accept the kflagstab, as it allows us (and
> potentially other Linux distributions) to easily introduce additional
> flags for symbols. It is also a simplification/clean-up of the module
> loader code.
I'm personally ok with adding the kflagstab support. I think it
introduces minimal complexity and, as you point out, simplifies certain
aspects. Additionally, if we add it, I believe that adding the proposed
symbol protection is simple enough to be included as well, at least from
my perspective.
[1] https://github.com/openSUSE/kernel-source/blob/307f149d9100a0e229eb94cbb997ae61187995c3/config/x86_64/default
--
Thanks,
Petr
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox