* Re: [PATCH] alloc_tag: remove empty module tag section
From: Petr Pavlu @ 2025-06-17 9:27 UTC (permalink / raw)
To: Casey Chen
Cc: akpm, linux-mm, linux-kernel, linux-modules, linux-arch, surenb,
kent.overstreet, arnd, mcgrof, pasha.tatashin, yzhong
In-Reply-To: <20250610162258.324645-1-cachen@purestorage.com>
On 6/10/25 6:22 PM, Casey Chen wrote:
> The empty MOD_CODETAG_SECTIONS() macro added an incomplete .data
> section in module linker script, which caused symbol lookup tools
> like gdb to misinterpret symbol addresses e.g., __ib_process_cq
> incorrectly mapping to unrelated functions like below.
>
> (gdb) disas __ib_process_cq
> Dump of assembler code for function trace_event_fields_cq_schedule:
>
> Removing the empty section restores proper symbol resolution and
> layout, ensuring .data placement behaves as expected.
The patch looks ok me, but I'm somewhat confused about the problem.
I think a linker should not add an empty output section if it doesn't
contain anything, or if .data actually contains something then the extra
dummy definition should be also harmless?
This also reminds me of my previous related fix "codetag: Avoid unused
alloc_tags sections/symbols" [1] which fell through the cracks. I can
rebase it on top of this patch.
[1] https://lore.kernel.org/all/20250313143002.9118-1-petr.pavlu@suse.com/
--
Thanks,
Petr
^ permalink raw reply
* Re: [PATCH 3/3] kunit: test: Drop CONFIG_MODULE ifdeffery
From: Thomas Weißschuh @ 2025-06-17 8:39 UTC (permalink / raw)
To: Petr Pavlu
Cc: Luis Chamberlain, Sami Tolvanen, Daniel Gomez, Brendan Higgins,
David Gow, Rae Moar, linux-modules, linux-kernel, linux-kselftest,
kunit-dev
In-Reply-To: <fb2a41b2-a872-4fcd-8a97-df3a946c6a81@suse.com>
On Tue, Jun 17, 2025 at 09:44:49AM +0200, Petr Pavlu wrote:
> On 6/12/25 4:53 PM, Thomas Weißschuh wrote:
> > The function stubs exposed by module.h allow the code to compile properly
> > without the ifdeffery. The generated object code stays the same, as the
> > compiler can optimize away all the dead code.
> > As the code is still typechecked developer errors can be detected faster.
> >
> > Signed-off-by: Thomas Weißschuh <thomas.weissschuh@linutronix.de>
>
> I'm worried that patches #2 and #3 make the code harder to understand
> because they hide what is compiled and when.
>
> Normally, '#ifdef CONFIG_XYZ' or IS_ENABLED(CONFIG_XYZ) directly
> surrounds functionality that should be conditional. This makes it clear
> what is used and when.
#ifdef is discouraged in C files and IS_ENABLED(CONFIG_MODULES) does not work
(here) without patch #2.
> The patches obscure whether, for instance, kunit_module_notify() in
> lib/kunit/test.c is actually used and present in the resulting binary
> behind several steps. Understanding its usage requires tracing the code
> from kunit_module_notify() to the definition of kunit_mod_nb, then to
> the register_module_notifier() call, and finally depends on an ifdef in
> another file, include/linux/module.h.
I agree that it is not completely clear what will end up in the binary.
On the other hand we can program the happy path and the compiler will take care
of all the corner cases.
We could add an "if (IS_ENABLED(CONFIG_MODULES))" which does not really change
anything but would be clearer to read.
> Is this really better? Are there places where this pattern is already
> used? Does it actually help in practice, considering that CONFIG_MODULES
> is enabled in most cases?
This came up for me when refactoring some KUnit internal code.
I used "kunit.py run" (which uses CONFIG_MODULES=n) to test my changes.
But some callers of changed functions were not updated and this wasn't reported.
The stub functions are a standard pattern and already implemented by module.h.
I have not found a header which hides structure definitions.
Documentation/process/coding-style.rst:
21) Conditional Compilation
---------------------------
Wherever possible, don't use preprocessor conditionals (#if, #ifdef) in .c
files; doing so makes code harder to read and logic harder to follow. Instead,
use such conditionals in a header file defining functions for use in those .c
files, providing no-op stub versions in the #else case, and then call those
functions unconditionally from .c files. The compiler will avoid generating
any code for the stub calls, producing identical results, but the logic will
remain easy to follow.
I should add the documentation reference to patch #2.
^ permalink raw reply
* Re: [PATCH 3/3] kunit: test: Drop CONFIG_MODULE ifdeffery
From: Petr Pavlu @ 2025-06-17 7:44 UTC (permalink / raw)
To: Thomas Weißschuh
Cc: Luis Chamberlain, Sami Tolvanen, Daniel Gomez, Brendan Higgins,
David Gow, Rae Moar, linux-modules, linux-kernel, linux-kselftest,
kunit-dev
In-Reply-To: <20250612-kunit-ifdef-modules-v1-3-fdccd42dcff8@linutronix.de>
On 6/12/25 4:53 PM, Thomas Weißschuh wrote:
> The function stubs exposed by module.h allow the code to compile properly
> without the ifdeffery. The generated object code stays the same, as the
> compiler can optimize away all the dead code.
> As the code is still typechecked developer errors can be detected faster.
>
> Signed-off-by: Thomas Weißschuh <thomas.weissschuh@linutronix.de>
I'm worried that patches #2 and #3 make the code harder to understand
because they hide what is compiled and when.
Normally, '#ifdef CONFIG_XYZ' or IS_ENABLED(CONFIG_XYZ) directly
surrounds functionality that should be conditional. This makes it clear
what is used and when.
The patches obscure whether, for instance, kunit_module_notify() in
lib/kunit/test.c is actually used and present in the resulting binary
behind several steps. Understanding its usage requires tracing the code
from kunit_module_notify() to the definition of kunit_mod_nb, then to
the register_module_notifier() call, and finally depends on an ifdef in
another file, include/linux/module.h.
Is this really better? Are there places where this pattern is already
used? Does it actually help in practice, considering that CONFIG_MODULES
is enabled in most cases?
--
Thanks,
Petr
^ permalink raw reply
* Re: [PATCH 1/3] module: move 'struct module_use' to internal.h
From: Petr Pavlu @ 2025-06-17 7:38 UTC (permalink / raw)
To: Thomas Weißschuh
Cc: Luis Chamberlain, Sami Tolvanen, Daniel Gomez, Brendan Higgins,
David Gow, Rae Moar, linux-modules, linux-kernel, linux-kselftest,
kunit-dev
In-Reply-To: <20250612-kunit-ifdef-modules-v1-1-fdccd42dcff8@linutronix.de>
On 6/12/25 4:53 PM, Thomas Weißschuh wrote:
> The struct was moved to the public header file in
> commit c8e21ced08b3 ("module: fix kdb's illicit use of struct module_use.").
> Back then the structure was used outside of the module core.
> Nowadays this is not true anymore, so the structure can be made internal.
>
> Signed-off-by: Thomas Weißschuh <thomas.weissschuh@linutronix.de>
Reviewed-by: Petr Pavlu <petr.pavlu@suse.com>
--
Thanks,
Petr
^ permalink raw reply
* Re: [PATCH v3] audit,module: restore audit logging in load failure case
From: Paul Moore @ 2025-06-16 21:00 UTC (permalink / raw)
To: Richard Guy Briggs, Linux-Audit Mailing List, LKML, linux-modules,
Linux Kernel Audit Mailing List
Cc: Eric Paris, Steve Grubb, Richard Guy Briggs
In-Reply-To: <9b7b21e5f37f5718fe9176a36ebf670bbdfdd54a.1748462545.git.rgb@redhat.com>
On Jun 13, 2025 Richard Guy Briggs <rgb@redhat.com> wrote:
>
> The move of the module sanity check to earlier skipped the audit logging
> call in the case of failure and to a place where the previously used
> context is unavailable.
>
> Add an audit logging call for the module loading failure case and get
> the module name when possible.
>
> Link: https://issues.redhat.com/browse/RHEL-52839
> Fixes: 02da2cbab452 ("module: move check_modinfo() early to early_mod_check()")
> Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
> Reviewed-by: Petr Pavlu <petr.pavlu@suse.com>
> ---
> Changelog:
> v2
> - use info->name for both audit_log_kern_module() calls and add const
> v3
> - use "?" rather than "(unavailable)" for consistency with other records
> ---
> include/linux/audit.h | 9 ++++-----
> kernel/audit.h | 2 +-
> kernel/auditsc.c | 2 +-
> kernel/module/main.c | 6 ++++--
> 4 files changed, 10 insertions(+), 9 deletions(-)
Merged into audit/dev, thanks!
--
paul-moore.com
^ permalink raw reply
* Re: [PATCH 1/2] module: Fix memory deallocation on error path in move_module()
From: Petr Pavlu @ 2025-06-16 13:58 UTC (permalink / raw)
To: Daniel Gomez
Cc: Luis Chamberlain, Sami Tolvanen, Daniel Gomez, linux-modules,
linux-kernel
In-Reply-To: <7cf40cd1-fe0d-4493-ac15-e70c418e54a5@kernel.org>
On 6/14/25 11:28 PM, Daniel Gomez wrote:
>> This seems to be off by one. For instance, if the loop reaches the last
>> valid type in mod_mem_type, MOD_INIT_RODATA, and successfully allocates
>> its memory, the variable t gets set to MOD_INIT_RODATA. Subsequently, if
>> an error occurs later in move_module() and control is transferred to
>> out_err, the deallocation starts from t-1, and therefore MOD_INIT_RODATA
>> doesn't get freed.
>>
>> If we want to always start from the last type found, the code would need
>> to be:
>>
>> [...]
>> ret = module_memory_alloc(mod, type);
>> if (ret)
>> goto out_err;
>> t = type + 1;
>> }
>>
>> I can adjust it in this way if it is preferred.
>>
>
> My earlier suggestion was incorrect. We can simply initialize the memory
> type t to MOD_MEM_NUM_TYPES since it's only used in the error path of
> module_memory_alloc().
Do you mean the following, or something else:
static int move_module(struct module *mod, struct load_info *info)
{
int i;
enum mod_mem_type t = MOD_MEM_NUM_TYPES;
int ret;
bool codetag_section_found = false;
for_each_mod_mem_type(type) {
if (!mod->mem[type].size) {
mod->mem[type].base = NULL;
continue;
}
ret = module_memory_alloc(mod, type);
if (ret) {
t = type;
goto out_err;
}
}
[...]
}
--
Thanks,
Petr
^ permalink raw reply
* Re: [PATCH v3] audit,module: restore audit logging in load failure case
From: Petr Pavlu @ 2025-06-16 13:55 UTC (permalink / raw)
To: Richard Guy Briggs
Cc: Paul Moore, Eric Paris, Steve Grubb, Linux-Audit Mailing List,
LKML, linux-modules, Linux Kernel Audit Mailing List
In-Reply-To: <9b7b21e5f37f5718fe9176a36ebf670bbdfdd54a.1748462545.git.rgb@redhat.com>
On 6/13/25 9:58 PM, Richard Guy Briggs wrote:
> The move of the module sanity check to earlier skipped the audit logging
> call in the case of failure and to a place where the previously used
> context is unavailable.
>
> Add an audit logging call for the module loading failure case and get
> the module name when possible.
>
> Link: https://issues.redhat.com/browse/RHEL-52839
> Fixes: 02da2cbab452 ("module: move check_modinfo() early to early_mod_check()")
> Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
Looks ok to me from the modules perspective.
Reviewed-by: Petr Pavlu <petr.pavlu@suse.com>
--
Thanks,
Petr
^ permalink raw reply
* How to run tests ?
From: PF @ 2025-06-15 12:20 UTC (permalink / raw)
To: linux-modules
Hello,
I'm trying to run tests on Debian 12 ou Fedora 38.
Following command runs without any error:
meson setup --native-file build-dev.ini builddir/
But following command always fails (here on Debian 12 but on Fedora 38 I
get similar error)
$ ninja -C builddir test
ninja: Entering directory `builddir'
[0/68] Generating testsuite/build-module-playground with a custom command
make: Entering directory
'/home/pierre/kmod/builddir/testsuite/module-playground'
make -C /lib/modules/$(uname -r)/build M=$PWD modules
make[1]: Entering directory '/usr/lib/modules/6.1.0-37-amd64/build'
make[1]: *** No rule to make target 'modules'. Stop.
make[1]: Leaving directory '/usr/lib/modules/6.1.0-37-amd64/build'
make: *** [Makefile:23: modules] Error 2
make: Leaving directory
'/home/pierre/kmod/builddir/testsuite/module-playground'
[0/68] Linking target testsuite/delete_module.so
FAILED: testsuite/bb-rootfs
/home/pierre/kmod/scripts/setup-modules.sh /home/pierre/kmod
testsuite/module-playground
[3/68] Linking target kmod
ninja: build stopped: subcommand failed.
(First run says that the directory "build" does not exist. But creating
the directory does not help)
What am I doing wrong ?
Thanks.
^ permalink raw reply
* Re: [PATCH 1/2] module: Fix memory deallocation on error path in move_module()
From: Daniel Gomez @ 2025-06-14 21:28 UTC (permalink / raw)
To: Petr Pavlu
Cc: Luis Chamberlain, Sami Tolvanen, Daniel Gomez, linux-modules,
linux-kernel
In-Reply-To: <c7dbb33d-98b6-45da-be77-e86b9e6787ee@suse.com>
> This seems to be off by one. For instance, if the loop reaches the last
> valid type in mod_mem_type, MOD_INIT_RODATA, and successfully allocates
> its memory, the variable t gets set to MOD_INIT_RODATA. Subsequently, if
> an error occurs later in move_module() and control is transferred to
> out_err, the deallocation starts from t-1, and therefore MOD_INIT_RODATA
> doesn't get freed.
>
> If we want to always start from the last type found, the code would need
> to be:
>
> [...]
> ret = module_memory_alloc(mod, type);
> if (ret)
> goto out_err;
> t = type + 1;
> }
>
> I can adjust it in this way if it is preferred.
>
My earlier suggestion was incorrect. We can simply initialize the memory
type t to MOD_MEM_NUM_TYPES since it's only used in the error path of
module_memory_alloc().
^ permalink raw reply
* [PATCH v2] gendwarfksyms: Fix structure type overrides
From: Sami Tolvanen @ 2025-06-14 0:55 UTC (permalink / raw)
To: Masahiro Yamada
Cc: Luis Chamberlain, Petr Pavlu, Daniel Gomez, linux-modules,
linux-kbuild, linux-kernel, Sami Tolvanen, Giuliano Procida
As we always iterate through the entire die_map when expanding
type strings, recursively processing referenced types in
type_expand_child() is not actually necessary. Furthermore,
the type_string kABI rule added in commit c9083467f7b9
("gendwarfksyms: Add a kABI rule to override type strings") can
fail to override type strings for structures due to a missing
kabi_get_type_string() check in this function.
Fix the issue by dropping the unnecessary recursion and moving
the override check to type_expand(). Note that symbol versions
are otherwise unchanged with this patch.
Fixes: c9083467f7b9 ("gendwarfksyms: Add a kABI rule to override type strings")
Reported-by: Giuliano Procida <gprocida@google.com>
Signed-off-by: Sami Tolvanen <samitolvanen@google.com>
Reviewed-by: Petr Pavlu <petr.pavlu@suse.com>
---
v2:
- Dropped the now unused __cache_*_expanded() functions per
Petr's suggestion.
v1: https://lore.kernel.org/r/20250609154926.1237033-2-samitolvanen@google.com/
scripts/gendwarfksyms/gendwarfksyms.h | 14 +-----
scripts/gendwarfksyms/types.c | 65 ++++++++-------------------
2 files changed, 21 insertions(+), 58 deletions(-)
diff --git a/scripts/gendwarfksyms/gendwarfksyms.h b/scripts/gendwarfksyms/gendwarfksyms.h
index 7dd03ffe0c5c..d9c06d2cb1df 100644
--- a/scripts/gendwarfksyms/gendwarfksyms.h
+++ b/scripts/gendwarfksyms/gendwarfksyms.h
@@ -216,24 +216,14 @@ int cache_get(struct cache *cache, unsigned long key);
void cache_init(struct cache *cache);
void cache_free(struct cache *cache);
-static inline void __cache_mark_expanded(struct cache *cache, uintptr_t addr)
-{
- cache_set(cache, addr, 1);
-}
-
-static inline bool __cache_was_expanded(struct cache *cache, uintptr_t addr)
-{
- return cache_get(cache, addr) == 1;
-}
-
static inline void cache_mark_expanded(struct cache *cache, void *addr)
{
- __cache_mark_expanded(cache, (uintptr_t)addr);
+ cache_set(cache, (unsigned long)addr, 1);
}
static inline bool cache_was_expanded(struct cache *cache, void *addr)
{
- return __cache_was_expanded(cache, (uintptr_t)addr);
+ return cache_get(cache, (unsigned long)addr) == 1;
}
/*
diff --git a/scripts/gendwarfksyms/types.c b/scripts/gendwarfksyms/types.c
index 39ce1770e463..7bd459ea6c59 100644
--- a/scripts/gendwarfksyms/types.c
+++ b/scripts/gendwarfksyms/types.c
@@ -333,37 +333,11 @@ static void calculate_version(struct version *version,
cache_free(&expansion_cache);
}
-static void __type_expand(struct die *cache, struct type_expansion *type,
- bool recursive);
-
-static void type_expand_child(struct die *cache, struct type_expansion *type,
- bool recursive)
-{
- struct type_expansion child;
- char *name;
-
- name = get_type_name(cache);
- if (!name) {
- __type_expand(cache, type, recursive);
- return;
- }
-
- if (recursive && !__cache_was_expanded(&expansion_cache, cache->addr)) {
- __cache_mark_expanded(&expansion_cache, cache->addr);
- type_expansion_init(&child);
- __type_expand(cache, &child, true);
- type_map_add(name, &child);
- type_expansion_free(&child);
- }
-
- type_expansion_append(type, name, name);
-}
-
-static void __type_expand(struct die *cache, struct type_expansion *type,
- bool recursive)
+static void __type_expand(struct die *cache, struct type_expansion *type)
{
struct die_fragment *df;
struct die *child;
+ char *name;
list_for_each_entry(df, &cache->fragments, list) {
switch (df->type) {
@@ -379,7 +353,12 @@ static void __type_expand(struct die *cache, struct type_expansion *type,
error("unknown child: %" PRIxPTR,
df->data.addr);
- type_expand_child(child, type, recursive);
+ name = get_type_name(child);
+ if (name)
+ type_expansion_append(type, name, name);
+ else
+ __type_expand(child, type);
+
break;
case FRAGMENT_LINEBREAK:
/*
@@ -397,12 +376,17 @@ static void __type_expand(struct die *cache, struct type_expansion *type,
}
}
-static void type_expand(struct die *cache, struct type_expansion *type,
- bool recursive)
+static void type_expand(const char *name, struct die *cache,
+ struct type_expansion *type)
{
+ const char *override;
+
type_expansion_init(type);
- __type_expand(cache, type, recursive);
- cache_free(&expansion_cache);
+
+ if (stable && kabi_get_type_string(name, &override))
+ type_parse(name, override, type);
+ else
+ __type_expand(cache, type);
}
static void type_parse(const char *name, const char *str,
@@ -416,8 +400,6 @@ static void type_parse(const char *name, const char *str,
if (!*str)
error("empty type string override for '%s'", name);
- type_expansion_init(type);
-
for (pos = 0; str[pos]; ++pos) {
bool empty;
char marker = ' ';
@@ -478,7 +460,6 @@ static void type_parse(const char *name, const char *str,
static void expand_type(struct die *cache, void *arg)
{
struct type_expansion type;
- const char *override;
char *name;
if (cache->mapped)
@@ -504,11 +485,7 @@ static void expand_type(struct die *cache, void *arg)
debug("%s", name);
- if (stable && kabi_get_type_string(name, &override))
- type_parse(name, override, &type);
- else
- type_expand(cache, &type, true);
-
+ type_expand(name, cache, &type);
type_map_add(name, &type);
type_expansion_free(&type);
free(name);
@@ -518,7 +495,6 @@ static void expand_symbol(struct symbol *sym, void *arg)
{
struct type_expansion type;
struct version version;
- const char *override;
struct die *cache;
/*
@@ -532,10 +508,7 @@ static void expand_symbol(struct symbol *sym, void *arg)
if (__die_map_get(sym->die_addr, DIE_SYMBOL, &cache))
return; /* We'll warn about missing CRCs later. */
- if (stable && kabi_get_type_string(sym->name, &override))
- type_parse(sym->name, override, &type);
- else
- type_expand(cache, &type, false);
+ type_expand(sym->name, cache, &type);
/* If the symbol already has a version, don't calculate it again. */
if (sym->state != SYMBOL_PROCESSED) {
base-commit: 19272b37aa4f83ca52bdf9c16d5d81bdd1354494
--
2.50.0.rc1.591.g9c95f17f64-goog
^ permalink raw reply related
* Re: [PATCH] gendwarfksyms: Fix structure type overrides
From: Sami Tolvanen @ 2025-06-14 0:53 UTC (permalink / raw)
To: Petr Pavlu
Cc: Masahiro Yamada, Luis Chamberlain, Daniel Gomez, linux-modules,
linux-kbuild, linux-kernel, Giuliano Procida
In-Reply-To: <dec42b04-b76f-4a2e-b060-9ac999fe1462@suse.com>
Hi Petr,
On Fri, Jun 13, 2025 at 12:52 PM Petr Pavlu <petr.pavlu@suse.com> wrote:
>
> Nit: This code was the only user of __cache_was_expanded() and
> __cache_mark_expanded(). It is now possible to merge
> __cache_was_expanded() into cache_was_expanded() and
> __cache_mark_expanded() into cache_mark_expanded().
Nice catch. I'll send v2 with these dropped.
> Looks ok to me otherwise, feel free to add:
>
> Reviewed-by: Petr Pavlu <petr.pavlu@suse.com>
Thanks!
Sami
^ permalink raw reply
* [PATCH v3] audit,module: restore audit logging in load failure case
From: Richard Guy Briggs @ 2025-06-13 19:58 UTC (permalink / raw)
To: Linux-Audit Mailing List, LKML, linux-modules,
Linux Kernel Audit Mailing List
Cc: Paul Moore, Eric Paris, Steve Grubb, Richard Guy Briggs
The move of the module sanity check to earlier skipped the audit logging
call in the case of failure and to a place where the previously used
context is unavailable.
Add an audit logging call for the module loading failure case and get
the module name when possible.
Link: https://issues.redhat.com/browse/RHEL-52839
Fixes: 02da2cbab452 ("module: move check_modinfo() early to early_mod_check()")
Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
---
Changelog:
v2
- use info->name for both audit_log_kern_module() calls and add const
v3
- use "?" rather than "(unavailable)" for consistency with other records
---
include/linux/audit.h | 9 ++++-----
kernel/audit.h | 2 +-
kernel/auditsc.c | 2 +-
kernel/module/main.c | 6 ++++--
4 files changed, 10 insertions(+), 9 deletions(-)
diff --git a/include/linux/audit.h b/include/linux/audit.h
index 0050ef288ab3..a394614ccd0b 100644
--- a/include/linux/audit.h
+++ b/include/linux/audit.h
@@ -417,7 +417,7 @@ extern int __audit_log_bprm_fcaps(struct linux_binprm *bprm,
extern void __audit_log_capset(const struct cred *new, const struct cred *old);
extern void __audit_mmap_fd(int fd, int flags);
extern void __audit_openat2_how(struct open_how *how);
-extern void __audit_log_kern_module(char *name);
+extern void __audit_log_kern_module(const char *name);
extern void __audit_fanotify(u32 response, struct fanotify_response_info_audit_rule *friar);
extern void __audit_tk_injoffset(struct timespec64 offset);
extern void __audit_ntp_log(const struct audit_ntp_data *ad);
@@ -519,7 +519,7 @@ static inline void audit_openat2_how(struct open_how *how)
__audit_openat2_how(how);
}
-static inline void audit_log_kern_module(char *name)
+static inline void audit_log_kern_module(const char *name)
{
if (!audit_dummy_context())
__audit_log_kern_module(name);
@@ -677,9 +677,8 @@ static inline void audit_mmap_fd(int fd, int flags)
static inline void audit_openat2_how(struct open_how *how)
{ }
-static inline void audit_log_kern_module(char *name)
-{
-}
+static inline void audit_log_kern_module(const char *name)
+{ }
static inline void audit_fanotify(u32 response, struct fanotify_response_info_audit_rule *friar)
{ }
diff --git a/kernel/audit.h b/kernel/audit.h
index 0211cb307d30..2a24d01c5fb0 100644
--- a/kernel/audit.h
+++ b/kernel/audit.h
@@ -200,7 +200,7 @@ struct audit_context {
int argc;
} execve;
struct {
- char *name;
+ const char *name;
} module;
struct {
struct audit_ntp_data ntp_data;
diff --git a/kernel/auditsc.c b/kernel/auditsc.c
index 78fd876a5473..eb98cd6fe91f 100644
--- a/kernel/auditsc.c
+++ b/kernel/auditsc.c
@@ -2864,7 +2864,7 @@ void __audit_openat2_how(struct open_how *how)
context->type = AUDIT_OPENAT2;
}
-void __audit_log_kern_module(char *name)
+void __audit_log_kern_module(const char *name)
{
struct audit_context *context = audit_context();
diff --git a/kernel/module/main.c b/kernel/module/main.c
index a2859dc3eea6..4860e534de05 100644
--- a/kernel/module/main.c
+++ b/kernel/module/main.c
@@ -3297,7 +3297,7 @@ static int load_module(struct load_info *info, const char __user *uargs,
module_allocated = true;
- audit_log_kern_module(mod->name);
+ audit_log_kern_module(info->name);
/* Reserve our place in the list. */
err = add_unformed_module(mod);
@@ -3458,8 +3458,10 @@ static int load_module(struct load_info *info, const char __user *uargs,
* failures once the proper module was allocated and
* before that.
*/
- if (!module_allocated)
+ if (!module_allocated) {
+ audit_log_kern_module(info->name ? info->name : "?");
mod_stat_bump_becoming(info, flags);
+ }
free_copy(info, flags);
return err;
}
--
2.43.5
^ permalink raw reply related
* Re: [PATCH] gendwarfksyms: Fix structure type overrides
From: Petr Pavlu @ 2025-06-13 12:52 UTC (permalink / raw)
To: Sami Tolvanen
Cc: Masahiro Yamada, Luis Chamberlain, Daniel Gomez, linux-modules,
linux-kbuild, linux-kernel, Giuliano Procida
In-Reply-To: <20250609154926.1237033-2-samitolvanen@google.com>
On 6/9/25 5:49 PM, Sami Tolvanen wrote:
> As we always iterate through the entire die_map when expanding
> type strings, recursively processing referenced types in
> type_expand_child() is not actually necessary. Furthermore,
> the type_string kABI rule added in commit c9083467f7b9
> ("gendwarfksyms: Add a kABI rule to override type strings") can
> fail to override type strings for structures due to a missing
> kabi_get_type_string() check in this function.
>
> Fix the issue by dropping the unnecessary recursion and moving
> the override check to type_expand(). Note that symbol versions
> are otherwise unchanged with this patch.
>
> Fixes: c9083467f7b9 ("gendwarfksyms: Add a kABI rule to override type strings")
> Reported-by: Giuliano Procida <gprocida@google.com>
> Signed-off-by: Sami Tolvanen <samitolvanen@google.com>
> ---
> scripts/gendwarfksyms/types.c | 65 ++++++++++-------------------------
> 1 file changed, 19 insertions(+), 46 deletions(-)
>
> diff --git a/scripts/gendwarfksyms/types.c b/scripts/gendwarfksyms/types.c
> index 39ce1770e463..7bd459ea6c59 100644
> --- a/scripts/gendwarfksyms/types.c
> +++ b/scripts/gendwarfksyms/types.c
> @@ -333,37 +333,11 @@ static void calculate_version(struct version *version,
> cache_free(&expansion_cache);
> }
>
> -static void __type_expand(struct die *cache, struct type_expansion *type,
> - bool recursive);
> -
> -static void type_expand_child(struct die *cache, struct type_expansion *type,
> - bool recursive)
> -{
> - struct type_expansion child;
> - char *name;
> -
> - name = get_type_name(cache);
> - if (!name) {
> - __type_expand(cache, type, recursive);
> - return;
> - }
> -
> - if (recursive && !__cache_was_expanded(&expansion_cache, cache->addr)) {
> - __cache_mark_expanded(&expansion_cache, cache->addr);
> - type_expansion_init(&child);
> - __type_expand(cache, &child, true);
> - type_map_add(name, &child);
> - type_expansion_free(&child);
> - }
Nit: This code was the only user of __cache_was_expanded() and
__cache_mark_expanded(). It is now possible to merge
__cache_was_expanded() into cache_was_expanded() and
__cache_mark_expanded() into cache_mark_expanded().
Looks ok to me otherwise, feel free to add:
Reviewed-by: Petr Pavlu <petr.pavlu@suse.com>
--
Thanks,
Petr
^ permalink raw reply
* [PATCH 3/3] kunit: test: Drop CONFIG_MODULE ifdeffery
From: Thomas Weißschuh @ 2025-06-12 14:53 UTC (permalink / raw)
To: Luis Chamberlain, Petr Pavlu, Sami Tolvanen, Daniel Gomez,
Brendan Higgins, David Gow, Rae Moar
Cc: linux-modules, linux-kernel, linux-kselftest, kunit-dev,
Thomas Weißschuh
In-Reply-To: <20250612-kunit-ifdef-modules-v1-0-fdccd42dcff8@linutronix.de>
The function stubs exposed by module.h allow the code to compile properly
without the ifdeffery. The generated object code stays the same, as the
compiler can optimize away all the dead code.
As the code is still typechecked developer errors can be detected faster.
Signed-off-by: Thomas Weißschuh <thomas.weissschuh@linutronix.de>
---
lib/kunit/test.c | 8 --------
1 file changed, 8 deletions(-)
diff --git a/lib/kunit/test.c b/lib/kunit/test.c
index 146d1b48a0965e8aaddb6162928f408bbb542645..019b2ac9c8469021542b610278f8842e100d57ad 100644
--- a/lib/kunit/test.c
+++ b/lib/kunit/test.c
@@ -759,7 +759,6 @@ void __kunit_test_suites_exit(struct kunit_suite **suites, int num_suites)
}
EXPORT_SYMBOL_GPL(__kunit_test_suites_exit);
-#ifdef CONFIG_MODULES
static void kunit_module_init(struct module *mod)
{
struct kunit_suite_set suite_set, filtered_set;
@@ -847,7 +846,6 @@ static struct notifier_block kunit_mod_nb = {
.notifier_call = kunit_module_notify,
.priority = 0,
};
-#endif
KUNIT_DEFINE_ACTION_WRAPPER(kfree_action_wrapper, kfree, const void *)
@@ -938,20 +936,14 @@ static int __init kunit_init(void)
kunit_debugfs_init();
kunit_bus_init();
-#ifdef CONFIG_MODULES
return register_module_notifier(&kunit_mod_nb);
-#else
- return 0;
-#endif
}
late_initcall(kunit_init);
static void __exit kunit_exit(void)
{
memset(&kunit_hooks, 0, sizeof(kunit_hooks));
-#ifdef CONFIG_MODULES
unregister_module_notifier(&kunit_mod_nb);
-#endif
kunit_bus_shutdown();
--
2.49.0
^ permalink raw reply related
* [PATCH 2/3] module: make structure definitions always visible
From: Thomas Weißschuh @ 2025-06-12 14:53 UTC (permalink / raw)
To: Luis Chamberlain, Petr Pavlu, Sami Tolvanen, Daniel Gomez,
Brendan Higgins, David Gow, Rae Moar
Cc: linux-modules, linux-kernel, linux-kselftest, kunit-dev,
Thomas Weißschuh
In-Reply-To: <20250612-kunit-ifdef-modules-v1-0-fdccd42dcff8@linutronix.de>
To write code that works with both CONFIG_MODULES=y and CONFIG_MODULES=n
it is convenient to use "if (IS_ENABLED(CONFIG_MODULES))" over raw #ifdef.
The code will still fully typechecked but the unreachable parts are
discarded by the compiler. This prevents accidental breakage when a certain
kconfig combination was not specifically tested by the developer.
This pattern is already supported to some extend by module.h defining
empty stub functions if CONFIG_MODULES=n.
However some users of module.h work on the structured defined by module.h.
Therefore these structure definitions need to be visible, too.
Many structure members are still gated by specific configuration settings.
The assumption for those is that the code using them will be gated behind
the same configuration setting anyways.
Signed-off-by: Thomas Weißschuh <thomas.weissschuh@linutronix.de>
---
include/linux/module.h | 23 ++++++++++++-----------
1 file changed, 12 insertions(+), 11 deletions(-)
diff --git a/include/linux/module.h b/include/linux/module.h
index 52f7b0487a2733c56e2531a434887e56e1bf45b2..7f783e71636542b99db3dd869a9387d14992df45 100644
--- a/include/linux/module.h
+++ b/include/linux/module.h
@@ -302,17 +302,6 @@ static typeof(name) __mod_device_table__##type##__##name \
struct notifier_block;
-#ifdef CONFIG_MODULES
-
-extern int modules_disabled; /* for sysctl */
-/* Get/put a kernel symbol (calls must be symmetric) */
-void *__symbol_get(const char *symbol);
-void *__symbol_get_gpl(const char *symbol);
-#define symbol_get(x) ({ \
- static const char __notrim[] \
- __used __section(".no_trim_symbol") = __stringify(x); \
- (typeof(&x))(__symbol_get(__stringify(x))); })
-
enum module_state {
MODULE_STATE_LIVE, /* Normal state. */
MODULE_STATE_COMING, /* Full formed, running module_init. */
@@ -598,6 +587,18 @@ struct module {
struct _ddebug_info dyndbg_info;
#endif
} ____cacheline_aligned __randomize_layout;
+
+#ifdef CONFIG_MODULES
+
+extern int modules_disabled; /* for sysctl */
+/* Get/put a kernel symbol (calls must be symmetric) */
+void *__symbol_get(const char *symbol);
+void *__symbol_get_gpl(const char *symbol);
+#define symbol_get(x) ({ \
+ static const char __notrim[] \
+ __used __section(".no_trim_symbol") = __stringify(x); \
+ (typeof(&x))(__symbol_get(__stringify(x))); })
+
#ifndef MODULE_ARCH_INIT
#define MODULE_ARCH_INIT {}
#endif
--
2.49.0
^ permalink raw reply related
* [PATCH 1/3] module: move 'struct module_use' to internal.h
From: Thomas Weißschuh @ 2025-06-12 14:53 UTC (permalink / raw)
To: Luis Chamberlain, Petr Pavlu, Sami Tolvanen, Daniel Gomez,
Brendan Higgins, David Gow, Rae Moar
Cc: linux-modules, linux-kernel, linux-kselftest, kunit-dev,
Thomas Weißschuh
In-Reply-To: <20250612-kunit-ifdef-modules-v1-0-fdccd42dcff8@linutronix.de>
The struct was moved to the public header file in
commit c8e21ced08b3 ("module: fix kdb's illicit use of struct module_use.").
Back then the structure was used outside of the module core.
Nowadays this is not true anymore, so the structure can be made internal.
Signed-off-by: Thomas Weißschuh <thomas.weissschuh@linutronix.de>
---
include/linux/module.h | 7 -------
kernel/module/internal.h | 7 +++++++
2 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/include/linux/module.h b/include/linux/module.h
index 92e1420fccdffc9de9f49da9061546cc1e0c89d1..52f7b0487a2733c56e2531a434887e56e1bf45b2 100644
--- a/include/linux/module.h
+++ b/include/linux/module.h
@@ -313,13 +313,6 @@ void *__symbol_get_gpl(const char *symbol);
__used __section(".no_trim_symbol") = __stringify(x); \
(typeof(&x))(__symbol_get(__stringify(x))); })
-/* modules using other modules: kdb wants to see this. */
-struct module_use {
- struct list_head source_list;
- struct list_head target_list;
- struct module *source, *target;
-};
-
enum module_state {
MODULE_STATE_LIVE, /* Normal state. */
MODULE_STATE_COMING, /* Full formed, running module_init. */
diff --git a/kernel/module/internal.h b/kernel/module/internal.h
index 8d74b0a21c82b5360977a29736eca78ee6b6be3e..1c2e0b0dc52e72d5ecd2f1b310ce535364b3f33b 100644
--- a/kernel/module/internal.h
+++ b/kernel/module/internal.h
@@ -109,6 +109,13 @@ struct find_symbol_arg {
enum mod_license license;
};
+/* modules using other modules */
+struct module_use {
+ struct list_head source_list;
+ struct list_head target_list;
+ struct module *source, *target;
+};
+
int mod_verify_sig(const void *mod, struct load_info *info);
int try_to_force_load(struct module *mod, const char *reason);
bool find_symbol(struct find_symbol_arg *fsa);
--
2.49.0
^ permalink raw reply related
* [PATCH 0/3] module: make structure definitions always visible
From: Thomas Weißschuh @ 2025-06-12 14:53 UTC (permalink / raw)
To: Luis Chamberlain, Petr Pavlu, Sami Tolvanen, Daniel Gomez,
Brendan Higgins, David Gow, Rae Moar
Cc: linux-modules, linux-kernel, linux-kselftest, kunit-dev,
Thomas Weißschuh
Code using IS_ENABLED(CONFIG_MODULES) as a C expression may need access
to the module structure definitions to compile.
Make sure these structure definitions are always visible.
Signed-off-by: Thomas Weißschuh <thomas.weissschuh@linutronix.de>
---
Thomas Weißschuh (3):
module: move 'struct module_use' to internal.h
module: make structure definitions always visible
kunit: test: Drop CONFIG_MODULE ifdeffery
include/linux/module.h | 30 ++++++++++++------------------
kernel/module/internal.h | 7 +++++++
lib/kunit/test.c | 8 --------
3 files changed, 19 insertions(+), 26 deletions(-)
---
base-commit: 19272b37aa4f83ca52bdf9c16d5d81bdd1354494
change-id: 20250611-kunit-ifdef-modules-0fefd13ae153
Best regards,
--
Thomas Weißschuh <thomas.weissschuh@linutronix.de>
^ permalink raw reply
* [PATCH v13 1/6] rust: str: add radix prefixed integer parsing functions
From: Andreas Hindborg @ 2025-06-12 13:40 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, 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: <20250612-module-params-v3-v13-0-bc219cd1a3f8@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.
Tested-by: Daniel Gomez <da.gomez@samsung.com>
Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
---
rust/kernel/str.rs | 2 +
rust/kernel/str/parse_int.rs | 171 +++++++++++++++++++++++++++++++++++++++++++
2 files changed, 173 insertions(+)
diff --git a/rust/kernel/str.rs b/rust/kernel/str.rs
index a927db8e079c..2b6c8b4a0ae4 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 000000000000..0754490aec4b
--- /dev/null
+++ b/rust/kernel/str/parse_int.rs
@@ -0,0 +1,171 @@
+// 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::str::BStr;
+
+ /// Trait that allows parsing a [`&BStr`] to an integer with a radix.
+ ///
+ /// # Safety
+ ///
+ /// The member functions of this trait must be implemented according to
+ /// their documentation.
+ ///
+ /// [`&BStr`]: kernel::str::BStr
+ // This is required because the `from_str_radix` function on the primitive
+ // integer types is not part of any trait.
+ pub unsafe trait FromStrRadix: Sized {
+ /// The minimum value this integer type can assume.
+ const MIN: Self;
+
+ /// Parse `src` to [`Self`] using radix `radix`.
+ fn from_str_radix(src: &BStr, radix: u32) -> Result<Self, crate::error::Error>;
+
+ /// Return the absolute value of [`Self::MIN`].
+ fn abs_min() -> u64;
+
+ /// Perform bitwise 2's complement on `self`.
+ ///
+ /// Note: This function does not make sense for unsigned integers.
+ fn complement(self) -> 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)?;
+
+ if val > Self::abs_min() {
+ return Err(EINVAL);
+ }
+
+ if val == Self::abs_min() {
+ return Ok(Self::MIN);
+ }
+
+ // SAFETY: We checked that `val` will fit in `Self` above.
+ let val: Self = unsafe { val.try_into().unwrap_unchecked() };
+
+ Ok(val.complement())
+ }
+ _ => {
+ let (radix, digits) = strip_radix(src);
+ Self::from_str_radix(digits, radix).map_err(|_| EINVAL)
+ }
+ }
+ }
+}
+
+macro_rules! impl_parse_int {
+ ($ty:ty) => {
+ // SAFETY: We implement the trait according to the documentation.
+ unsafe impl private::FromStrRadix for $ty {
+ const MIN: Self = <$ty>::MIN;
+
+ fn from_str_radix(src: &BStr, radix: u32) -> Result<Self, crate::error::Error> {
+ <$ty>::from_str_radix(core::str::from_utf8(src).map_err(|_| EINVAL)?, radix)
+ .map_err(|_| EINVAL)
+ }
+
+ fn abs_min() -> u64 {
+ #[allow(unused_comparisons)]
+ if Self::MIN < 0 {
+ 1u64 << (Self::BITS - 1)
+ } else {
+ 0
+ }
+ }
+
+ fn complement(self) -> Self {
+ (!self).wrapping_add((1 as $ty))
+ }
+ }
+
+ impl ParseInt for $ty {}
+ };
+}
+
+impl_parse_int!(i8);
+impl_parse_int!(u8);
+impl_parse_int!(i16);
+impl_parse_int!(u16);
+impl_parse_int!(i32);
+impl_parse_int!(u32);
+impl_parse_int!(i64);
+impl_parse_int!(u64);
+impl_parse_int!(isize);
+impl_parse_int!(usize);
--
2.47.2
^ permalink raw reply related
* [PATCH v13 5/6] rust: samples: add a module parameter to the rust_minimal sample
From: Andreas Hindborg @ 2025-06-12 13:40 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, 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: <20250612-module-params-v3-v13-0-bc219cd1a3f8@kernel.org>
Showcase the rust module parameter support by adding a module parameter to
the `rust_minimal` sample.
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 1fc7a1be6b6d..c04cc07b3249 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.get()
+ );
let mut numbers = KVec::new();
numbers.push(72, GFP_KERNEL)?;
--
2.47.2
^ permalink raw reply related
* [PATCH v13 2/6] rust: introduce module_param module
From: Andreas Hindborg @ 2025-06-12 13:40 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, 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: <20250612-module-params-v3-v13-0-bc219cd1a3f8@kernel.org>
Add types and traits for interfacing the C moduleparam API.
Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
---
rust/kernel/lib.rs | 1 +
rust/kernel/module_param.rs | 201 ++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 202 insertions(+)
diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
index 6b4774b2b1c3..2b439ea06185 100644
--- a/rust/kernel/lib.rs
+++ b/rust/kernel/lib.rs
@@ -87,6 +87,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 000000000000..fd167df8e53d
--- /dev/null
+++ b/rust/kernel/module_param.rs
@@ -0,0 +1,201 @@
+// 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;
+
+/// Newtype to make `bindings::kernel_param` [`Sync`].
+#[repr(transparent)]
+#[doc(hidden)]
+pub struct RacyKernelParam(pub ::kernel::bindings::kernel_param);
+
+// SAFETY: C kernel handles serializing access to this type. We never access it
+// from Rust module.
+unsafe impl Sync for RacyKernelParam {}
+
+/// Types that can be used for module parameters.
+pub trait ModuleParam: Sized + Copy {
+ /// The [`ModuleParam`] will be used by the kernel module through this type.
+ ///
+ /// This may differ from `Self` if, for example, `Self` needs to track
+ /// ownership without exposing it or allocate extra space for other possible
+ /// parameter values.
+ // This is required to support string parameters in the future.
+ type Value: ?Sized;
+
+ /// Parse a parameter argument into the parameter value.
+ ///
+ /// `Err(_)` should be returned when parsing of the argument fails.
+ ///
+ /// Parameters passed at boot time will be set before [`kmalloc`] is
+ /// available (even if the module is loaded at a later time). However, in
+ /// this case, the argument buffer will be valid for the entire lifetime of
+ /// the kernel. So implementations of this method which need to allocate
+ /// should first check that the allocator is available (with
+ /// [`crate::bindings::slab_is_available`]) and when it is not available
+ /// provide an alternative implementation which doesn't allocate. In cases
+ /// where the allocator is not available it is safe to save references to
+ /// `arg` in `Self`, but in other cases a copy should be made.
+ ///
+ /// [`kmalloc`]: srctree/include/linux/slab.h
+ fn try_from_param_arg(arg: &'static 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.
+/// - `parm` must be a pointer to a `bindings::kernel_param` that is valid for reads for the
+/// duration of the call.
+/// - `param.arg` must be a pointer to an initialized `T` that is valid for writes for the duration
+/// of the function.
+///
+/// # 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 crate::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 `param` is be valid for reads.
+ let old_value = unsafe { (*param).__bindgen_anon_1.arg as *mut T };
+
+ // SAFETY: By function safety requirements, the target of `old_value` is valid for writes
+ // and is initialized.
+ unsafe { *old_value = new_value };
+ Ok(0)
+ })
+}
+
+macro_rules! impl_int_module_param {
+ ($ty:ident) => {
+ impl ModuleParam for $ty {
+ type Value = $ty;
+
+ fn try_from_param_arg(arg: &'static 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.
+#[repr(transparent)]
+pub struct ModuleParamAccess<T> {
+ data: core::cell::UnsafeCell<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(value: T) -> Self {
+ Self {
+ data: core::cell::UnsafeCell::new(value),
+ }
+ }
+
+ /// 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 get(&self) -> &T {
+ // SAFETY: As we only support read only parameters with no sysfs
+ // exposure, the kernel will not touch the parameter data after module
+ // initialization.
+ unsafe { &*self.data.get() }
+ }
+
+ /// Get a mutable pointer to the parameter value.
+ pub const fn as_mut_ptr(&self) -> *mut T {
+ self.data.get()
+ }
+}
+
+#[doc(hidden)]
+#[macro_export]
+/// 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 v13 6/6] modules: add rust modules files to MAINTAINERS
From: Andreas Hindborg @ 2025-06-12 13:40 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, 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: <20250612-module-params-v3-v13-0-bc219cd1a3f8@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 a92290fffa16..e3f43583c9c6 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -16794,6 +16794,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 v13 0/6] rust: extend `module!` macro with integer parameter support
From: Andreas Hindborg @ 2025-06-12 13:40 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, 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 and updates `BStr` with a method to strip a string
prefix.
Based on code by Adam Bratschi-Kaye lifted from the original `rust` branch [1].
Link: https://github.com/Rust-for-Linux/linux/tree/bc22545f38d74473cfef3e9fd65432733435b79f [1]
Signed-off-by: Andreas Hindborg <a.hindborg@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 (6):
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 | 201 +++++++++++++++++++++++++++++++++++++++++++
rust/kernel/str.rs | 2 +
rust/kernel/str/parse_int.rs | 171 ++++++++++++++++++++++++++++++++++++
rust/macros/helpers.rs | 25 ++++++
rust/macros/lib.rs | 31 +++++++
rust/macros/module.rs | 195 ++++++++++++++++++++++++++++++++++++-----
samples/rust/rust_minimal.rs | 10 +++
9 files changed, 618 insertions(+), 20 deletions(-)
---
base-commit: 19272b37aa4f83ca52bdf9c16d5d81bdd1354494
change-id: 20241211-module-params-v3-ae7e5c8d8b5a
Best regards,
--
Andreas Hindborg <a.hindborg@kernel.org>
^ permalink raw reply
* [PATCH v13 3/6] rust: module: use a reference in macros::module::module
From: Andreas Hindborg @ 2025-06-12 13:40 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, 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: <20250612-module-params-v3-v13-0-bc219cd1a3f8@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.
Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
---
rust/macros/module.rs | 20 ++++++++++----------
1 file changed, 10 insertions(+), 10 deletions(-)
diff --git a/rust/macros/module.rs b/rust/macros/module.rs
index 2ddd2eeb2852..1a867a1e787e 100644
--- a/rust/macros/module.rs
+++ b/rust/macros/module.rs
@@ -179,26 +179,26 @@ 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(author) = info.author {
- modinfo.emit("author", &author);
+ if let Some(author) = &info.author {
+ modinfo.emit("author", author);
}
- 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 v13 4/6] rust: module: update the module macro with module parameter support
From: Andreas Hindborg @ 2025-06-12 13:40 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, 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: <20250612-module-params-v3-v13-0-bc219cd1a3f8@kernel.org>
Allow module parameters to be declared in the rust `module!` macro.
Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
---
rust/macros/helpers.rs | 25 +++++++
rust/macros/lib.rs | 31 +++++++++
rust/macros/module.rs | 175 ++++++++++++++++++++++++++++++++++++++++++++++---
3 files changed, 221 insertions(+), 10 deletions(-)
diff --git a/rust/macros/helpers.rs b/rust/macros/helpers.rs
index e2602be402c1..365d7eb499c0 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 fa847cf3a9b5..2fb520dc930a 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 1a867a1e787e..b2605482822a 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,116 @@ 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});
+
+ #[link_section = \"__param\"]
+ #[used]
+ static __{module_name}_{param_name}_struct:
+ ::kernel::module_param::RacyKernelParam =
+ ::kernel::module_param::RacyKernelParam(::kernel::bindings::kernel_param {{
+ name: if 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 {{
+ (&::kernel::bindings::__this_module
+ as *const ::kernel::bindings::module)
+ .cast_mut()
+ }},
+ #[cfg(not(MODULE))]
+ mod_: ::core::ptr::null_mut(),
+ ops: &{ops} as *const ::kernel::bindings::kernel_param_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_mut_ptr().cast()
+ }},
+ }});
+ ",
+ 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,
@@ -99,6 +203,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 {
@@ -114,6 +262,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();
@@ -140,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:?}."),
}
@@ -205,7 +355,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!(
"
@@ -369,15 +521,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
* Re: [PATCH v3] module: Make sure relocations are applied to the per-CPU section
From: Petr Pavlu @ 2025-06-12 11:47 UTC (permalink / raw)
To: Sebastian Andrzej Siewior
Cc: Luis Chamberlain, Sami Tolvanen, Daniel Gomez, Peter Zijlstra,
Thomas Gleixner, linux-modules
In-Reply-To: <20250610163328.URcsSUC1@linutronix.de>
On 6/10/25 6:33 PM, Sebastian Andrzej Siewior wrote:
> The per-CPU data section is handled differently than the other sections.
> The memory allocations requires a special __percpu pointer and then the
> section is copied into the view of each CPU. Therefore the SHF_ALLOC
> flag is removed to ensure move_module() skips it.
>
> Later, relocations are applied and apply_relocations() skips sections
> without SHF_ALLOC because they have not been copied. This also skips the
> per-CPU data section.
> The missing relocations result in a NULL pointer on x86-64 and very
> small values on x86-32. This results in a crash because it is not
> skipped like NULL pointer would and can't be dereferenced.
>
> Such an assignment happens during static per-CPU lock initialisation
> with lockdep enabled.
>
> Allow relocation processing for the per-CPU section even if SHF_ALLOC is
> missing.
>
> Reported-by: kernel test robot <oliver.sang@intel.com>
> Closes: https://lore.kernel.org/oe-lkp/202506041623.e45e4f7d-lkp@intel.com
> Fixes: 1a6100caae425 ("Don't relocate non-allocated regions in modules.") #v2.6.1-rc3
> Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Reviewed-by: Petr Pavlu <petr.pavlu@suse.com>
--
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