* Re: [PATCH 1/2] kbuild: rust: allow `clippy::uninlined_format_args`
From: Miguel Ojeda @ 2026-04-03 10:06 UTC (permalink / raw)
To: Miguel Ojeda
Cc: Luis Chamberlain, Petr Pavlu, Daniel Gomez, Sami Tolvanen,
Nathan Chancellor, Nicolas Schier, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
Trevor Gross, Danilo Krummrich, rust-for-linux, Aaron Tomlin,
linux-modules, linux-kernel, linux-kbuild, stable
In-Reply-To: <20260331205849.498295-1-ojeda@kernel.org>
On Tue, Mar 31, 2026 at 10:59 PM Miguel Ojeda <ojeda@kernel.org> wrote:
>
> Clippy in Rust 1.88.0 (only) reports [1]:
>
> warning: variables can be used directly in the `format!` string
> --> rust/macros/module.rs:112:23
> |
> 112 | let content = format!("{param}:{content}", param = param, content = content);
> | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> |
> = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#uninlined_format_args
> = note: `-W clippy::uninlined-format-args` implied by `-W clippy::all`
> = help: to override `-W clippy::all` add `#[allow(clippy::uninlined_format_args)]`
> help: change this to
> |
> 112 - let content = format!("{param}:{content}", param = param, content = content);
> 112 + let content = format!("{param}:{content}");
>
> warning: variables can be used directly in the `format!` string
> --> rust/macros/module.rs:198:14
> |
> 198 | t => panic!("Unsupported parameter type {}", t),
> | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> |
> = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#uninlined_format_args
> = note: `-W clippy::uninlined-format-args` implied by `-W clippy::all`
> = help: to override `-W clippy::all` add `#[allow(clippy::uninlined_format_args)]`
> help: change this to
> |
> 198 - t => panic!("Unsupported parameter type {}", t),
> 198 + t => panic!("Unsupported parameter type {t}"),
> |
>
> The reason it only triggers in that version is that the lint was moved
> from `pedantic` to `style` in Rust 1.88.0 and then back to `pedantic`
> in Rust 1.89.0 [2][3].
>
> In the first case, the suggestion is fair and a pure simplification, thus
> we will clean it up separately.
>
> To keep the behavior the same across all versions, and since the lint
> does not work for all macros (e.g. custom ones like `pr_info!`), disable
> it globally.
>
> Cc: stable@vger.kernel.org # Needed in 6.12.y and later (Rust is pinned in older LTSs).
> Link: https://lore.kernel.org/rust-for-linux/CANiq72=drAtf3y_DZ-2o4jb6Az9J3Yj4QYwWnbRui4sm4AJD3Q@mail.gmail.com/ [1]
> Link: https://github.com/rust-lang/rust-clippy/pull/15287 [2]
> Link: https://github.com/rust-lang/rust-clippy/issues/15151 [3]
> Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
Applied series to `rust-next` -- thanks everyone!
(If wanted by modules, I can drop the top commit.)
Cheers,
Miguel
^ permalink raw reply
* Re: [PATCH 2/2] rust: macros: simplify `format!` arguments
From: Miguel Ojeda @ 2026-04-03 4:53 UTC (permalink / raw)
To: Miguel Ojeda
Cc: Luis Chamberlain, Petr Pavlu, Daniel Gomez, Sami Tolvanen,
Nathan Chancellor, Nicolas Schier, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
Trevor Gross, Danilo Krummrich, rust-for-linux, Aaron Tomlin,
linux-modules, linux-kernel, linux-kbuild
In-Reply-To: <20260331205849.498295-2-ojeda@kernel.org>
On Tue, Mar 31, 2026 at 10:59 PM Miguel Ojeda <ojeda@kernel.org> wrote:
>
> Clippy in Rust 1.88.0 (only) reported [1] up to the previous commit:
>
> warning: variables can be used directly in the `format!` string
> --> rust/macros/module.rs:112:23
> |
> 112 | let content = format!("{param}:{content}", param = param, content = content);
> | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> |
> = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#uninlined_format_args
> = note: `-W clippy::uninlined-format-args` implied by `-W clippy::all`
> = help: to override `-W clippy::all` add `#[allow(clippy::uninlined_format_args)]`
> help: change this to
> |
> 112 - let content = format!("{param}:{content}", param = param, content = content);
> 112 + let content = format!("{param}:{content}");
>
> The reason it only triggers in that version is that the lint was moved
> from `pedantic` to `style` in Rust 1.88.0 and then back to `pedantic`
> in Rust 1.89.0 [2][3].
>
> In this case, the suggestion is fair and a pure simplification, thus
> just apply it.
>
> In addition, do the same for another place in the file that Clippy does
> not report because it is multi-line.
>
> Link: https://lore.kernel.org/rust-for-linux/CANiq72=drAtf3y_DZ-2o4jb6Az9J3Yj4QYwWnbRui4sm4AJD3Q@mail.gmail.com/ [1]
> Link: https://github.com/rust-lang/rust-clippy/pull/15287 [2]
> Link: https://github.com/rust-lang/rust-clippy/issues/15151 [3]
> Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
I will pick this one up together with the other one, but if someone
prefers that I don't, please shout (e.g. if modules wants to pick it
themselves).
An Acked-by is also appreciated, thanks!
Cheers,
Miguel
^ permalink raw reply
* [PATCH] kbuild/btf: Avoid relinking modules when only vmlinux changes
From: Petr Pavlu @ 2026-04-02 14:17 UTC (permalink / raw)
To: Nathan Chancellor, Nicolas Schier, Alexei Starovoitov,
Daniel Borkmann, Andrii Nakryiko
Cc: Martin KaFai Lau, Eduard Zingerman, Song Liu, Yonghong Song,
John Fastabend, KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa,
Luis Chamberlain, Daniel Gomez, Sami Tolvanen, Aaron Tomlin,
Ihor Solodrai, Masahiro Yamada, linux-kbuild, bpf, linux-modules,
linux-kernel, Petr Pavlu
Commit 5f9ae91f7c0d ("kbuild: Build kernel module BTFs if BTF is enabled
and pahole supports it") in 2020 introduced CONFIG_DEBUG_INFO_BTF_MODULES
to enable generation of split BTF for kernel modules. This change required
the %.ko Makefile rule to additionally depend on vmlinux, which is used as
a base for deduplication. The regular ld_ko_o command executed by the rule
was then modified to be skipped if only vmlinux changes. This was done by
introducing a new if_changed_except command and updating the original call
to '+$(call if_changed_except,ld_ko_o,vmlinux)'.
Later, commit 214c0eea43b2 ("kbuild: add $(objtree)/ prefix to some
in-kernel build artifacts") in 2024 updated the rule's reference to vmlinux
from 'vmlinux' to '$(objtree)/vmlinux'. This accidentally broke the
previous logic to skip relinking modules if only vmlinux changes. The issue
is that '$(objtree)' is typically '.' and GNU Make normalizes the resulting
prerequisite './vmlinux' to just 'vmlinux', while the exclusion logic
retains the raw './vmlinux'. As a result, if_changed_except doesn't
correctly filter out vmlinux. Consequently, with
CONFIG_DEBUG_INFO_BTF_MODULES=y, modules are relinked even if only vmlinux
changes.
Additionally, commit 522397d05e7d ("resolve_btfids: Change in-place update
with raw binary output") in 2025 reworked the method for patching BTF data
into the resulting modules by using 'objcopy --add-section'. This command
fails if a section already exists.
Fix the unnecessary relinking issue by also excluding the normalized form
'vmlinux' when invoking ld_ko_o. Adjust embed_btf_data() to first use the
--remove-section option to remove the patched BTF section if it is already
present.
Fixes: 214c0eea43b2 ("kbuild: add $(objtree)/ prefix to some in-kernel build artifacts")
Signed-off-by: Petr Pavlu <petr.pavlu@suse.com>
---
scripts/Makefile.modfinal | 14 ++++++++++++--
scripts/gen-btf.sh | 6 ++++--
2 files changed, 16 insertions(+), 4 deletions(-)
diff --git a/scripts/Makefile.modfinal b/scripts/Makefile.modfinal
index adcbcde16a07..2981745a172f 100644
--- a/scripts/Makefile.modfinal
+++ b/scripts/Makefile.modfinal
@@ -54,9 +54,19 @@ if_changed_except = $(if $(call newer_prereqs_except,$(2))$(cmd-check), \
$(cmd); \
printf '%s\n' 'savedcmd_$@ := $(make-cmd)' > $(dot-target).cmd, @:)
-# Re-generate module BTFs if either module's .ko or vmlinux changed
+# Build final module objects.
+#
+# The *.ko files are usually independent of vmlinux. However, special handling
+# is required for module BTFs, which need to be regenerated if either the
+# module's .ko file or vmlinux has changed, as vmlinux is used as a base for
+# deduplication. Consequently, vmlinux is included in the rule prerequisites.
+#
+# The regular ld_ko_o call is conditional and is skipped if only vmlinux has
+# changed. The exclusion pattern used in the if_changed_except call contains
+# '$(objtree)/vmlinux' to match the exact prerequisite and plain 'vmlinux' to
+# cover the case when make normalizes './vmlinux' to 'vmlinux'.
%.ko: %.o %.mod.o .module-common.o $(objtree)/scripts/module.lds $(and $(CONFIG_DEBUG_INFO_BTF_MODULES),$(KBUILD_BUILTIN),$(objtree)/vmlinux) FORCE
- +$(call if_changed_except,ld_ko_o,$(objtree)/vmlinux)
+ +$(call if_changed_except,ld_ko_o,$(objtree)/vmlinux vmlinux)
ifdef CONFIG_DEBUG_INFO_BTF_MODULES
+$(if $(newer-prereqs),$(call cmd,btf_ko))
endif
diff --git a/scripts/gen-btf.sh b/scripts/gen-btf.sh
index 8ca96eb10a69..6d4e629c79ca 100755
--- a/scripts/gen-btf.sh
+++ b/scripts/gen-btf.sh
@@ -104,12 +104,14 @@ gen_btf_o()
embed_btf_data()
{
- ${OBJCOPY} --add-section .BTF=${ELF_FILE}.BTF ${ELF_FILE}
+ ${OBJCOPY} --remove-section .BTF \
+ --add-section .BTF=${ELF_FILE}.BTF ${ELF_FILE}
# a module might not have a .BTF_ids or .BTF.base section
btf_base="${ELF_FILE}.BTF.base"
if [ -f "${btf_base}" ]; then
- ${OBJCOPY} --add-section .BTF.base=${btf_base} ${ELF_FILE}
+ ${OBJCOPY} --remove-section .BTF.base \
+ --add-section .BTF.base=${btf_base} ${ELF_FILE}
fi
btf_ids="${ELF_FILE}.BTF_ids"
if [ -f "${btf_ids}" ]; then
base-commit: 7aaa8047eafd0bd628065b15757d9b48c5f9c07d
--
2.53.0
^ permalink raw reply related
* Re: [PATCH 1/2] kbuild: rust: allow `clippy::uninlined_format_args`
From: Gary Guo @ 2026-04-01 15:36 UTC (permalink / raw)
To: Miguel Ojeda, Gary Guo
Cc: Miguel Ojeda, Luis Chamberlain, Petr Pavlu, Daniel Gomez,
Sami Tolvanen, Nathan Chancellor, Nicolas Schier, Boqun Feng,
Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
Trevor Gross, Danilo Krummrich, rust-for-linux, Aaron Tomlin,
linux-modules, linux-kernel, linux-kbuild, stable
In-Reply-To: <CANiq72=8d03wo3_28Q91DpHs=LF8D5N3pmuZtAsr8gLuco5hKQ@mail.gmail.com>
On Tue Mar 31, 2026 at 10:53 PM BST, Miguel Ojeda wrote:
> On Tue, Mar 31, 2026 at 11:43 PM Gary Guo <gary@garyguo.net> wrote:
>>
>> I mean the lint is kinda useful, and I don't want to disable it just because
>> it doesn't exist in older versions of rustc.
>
> Yeah, personally I like the inlined way, i.e. I use it myself, so I
> don't mind enabling it everywhere if people is happy that it only
> applies to some cases.
>
> Another consideration is that the issue linked mentions that they
> don't want to mix inline and not (for field access cases), so that
> could be annoying for some, which is why they moved it back.
For mixed cases I suppose it's really up to personal taste. In that case we can
say it does have "false positive" and allow the lint.
Best,
Gary
>
> Either way sounds fine for me.
>
> Anyway, if we enable it, I should apply the other suggestion too, put
> the Cc: stable@ on them, and switch to `-W` here and remove the Cc:
> stable@.
>
> Thanks for taking a look!
>
> Cheers,
> Miguel
^ permalink raw reply
* Re: [PATCH v5 0/7] scalable symbol flags with __kflagstab
From: Sami Tolvanen @ 2026-04-01 1:30 UTC (permalink / raw)
To: Luis Chamberlain, Petr Pavlu, Daniel Gomez, Aaron Tomlin,
Arnd Bergmann, Nathan Chancellor, Nicolas Schier, Jonathan Corbet,
Shuah Khan, Siddharth Nayyar
Cc: linux-modules, linux-kernel, linux-arch, linux-kbuild, linux-doc,
maennich, gprocida
In-Reply-To: <20260326-kflagstab-v5-0-fa0796fe88d9@google.com>
On Thu, 26 Mar 2026 21:25:01 +0000, Siddharth Nayyar wrote:
> This patch series implements a mechanism for scalable exported symbol
> flags using a separate section called __kflagstab. The series introduces
> __kflagstab support, removes *_gpl sections in favor of a GPL flag,
> simplifies symbol resolution during module loading.
>
> The __kflagstab contains an 8-bit bitset which can represent up to 8
> boolean flags per symbol exported in the __ksymtab. The patch series
> also uses this bitset to store GPL-only flag values for kernel symbols,
> thereby eliminating the need for *_gpl sections for representing GPL
> only symbols.
>
> [...]
Applied to modules-next, thanks!
[1/7] module: define ksym_flags enumeration to represent kernel symbol flags
commit: 10a4eb5882ba16164ece86d99486084f02f148bb
[2/7] module: add kflagstab section to vmlinux and modules
commit: 9743311b4535dc76ce81f46303da0f69bfaa5fd6
[3/7] module: populate kflagstab in modpost
commit: 16d0e04f546ffba78c74bbfeb57d93147bcaf2c5
[4/7] module: use kflagstab instead of *_gpl sections
commit: 55fcb926b6d8b5cfb40873e4840a69961db1bb69
[5/7] module: deprecate usage of *_gpl sections in module loader
commit: b4760ff2a5e4351c59d185967735f59c0b0bd7f6
[6/7] module: remove *_gpl sections from vmlinux and modules
commit: f18540256b70c9e1f0e26e2c38f3d43a131926d9
[7/7] documentation: remove references to *_gpl sections
commit: 3b1299f25b07ef83e020d049dfc62ced9c09450d
Best regards,
Sami
^ permalink raw reply
* Re: [PATCH 1/2] kbuild: rust: allow `clippy::uninlined_format_args`
From: Miguel Ojeda @ 2026-03-31 21:53 UTC (permalink / raw)
To: Gary Guo
Cc: Miguel Ojeda, Luis Chamberlain, Petr Pavlu, Daniel Gomez,
Sami Tolvanen, Nathan Chancellor, Nicolas Schier, Boqun Feng,
Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
Trevor Gross, Danilo Krummrich, rust-for-linux, Aaron Tomlin,
linux-modules, linux-kernel, linux-kbuild, stable
In-Reply-To: <DHHANEJI7LQ0.3PGBQH34QK0DJ@garyguo.net>
On Tue, Mar 31, 2026 at 11:43 PM Gary Guo <gary@garyguo.net> wrote:
>
> I mean the lint is kinda useful, and I don't want to disable it just because
> it doesn't exist in older versions of rustc.
Yeah, personally I like the inlined way, i.e. I use it myself, so I
don't mind enabling it everywhere if people is happy that it only
applies to some cases.
Another consideration is that the issue linked mentions that they
don't want to mix inline and not (for field access cases), so that
could be annoying for some, which is why they moved it back.
Either way sounds fine for me.
Anyway, if we enable it, I should apply the other suggestion too, put
the Cc: stable@ on them, and switch to `-W` here and remove the Cc:
stable@.
Thanks for taking a look!
Cheers,
Miguel
^ permalink raw reply
* Re: [PATCH 1/2] kbuild: rust: allow `clippy::uninlined_format_args`
From: Gary Guo @ 2026-03-31 21:43 UTC (permalink / raw)
To: Miguel Ojeda, Gary Guo
Cc: Miguel Ojeda, Luis Chamberlain, Petr Pavlu, Daniel Gomez,
Sami Tolvanen, Nathan Chancellor, Nicolas Schier, Boqun Feng,
Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
Trevor Gross, Danilo Krummrich, rust-for-linux, Aaron Tomlin,
linux-modules, linux-kernel, linux-kbuild, stable
In-Reply-To: <CANiq72=wsdJf1_qwAADhmKA2i7y9U+3WOm+9utE2rv52_eqnpQ@mail.gmail.com>
On Tue Mar 31, 2026 at 10:14 PM BST, Miguel Ojeda wrote:
> On Tue, Mar 31, 2026 at 11:07 PM Gary Guo <gary@garyguo.net> wrote:
>>
>> Does it produce a false positive, or it's a false negative? If it's the latter,
>> I think we don't need to disable the lint.
>
> If you mean for custom macros, then I think it is just that it doesn't
> take those into account at all. So I guess you could say false
> negative if you consider that it should.
>
> In any case, originally I just cleaned it, because it wasn't a big
> deal either way, but then I thought it would be best to keep the
> behavior the same. So either we disable or enable everywhere.
>
> Cheers,
> Miguel
I mean the lint is kinda useful, and I don't want to disable it just because
it doesn't exist in older versions of rustc.
Best,
Gary
^ permalink raw reply
* Re: [PATCH 1/2] kbuild: rust: allow `clippy::uninlined_format_args`
From: Miguel Ojeda @ 2026-03-31 21:14 UTC (permalink / raw)
To: Gary Guo
Cc: Miguel Ojeda, Luis Chamberlain, Petr Pavlu, Daniel Gomez,
Sami Tolvanen, Nathan Chancellor, Nicolas Schier, Boqun Feng,
Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
Trevor Gross, Danilo Krummrich, rust-for-linux, Aaron Tomlin,
linux-modules, linux-kernel, linux-kbuild, stable
In-Reply-To: <DHH9VRFULJST.383BKVSWUTZ3E@garyguo.net>
On Tue, Mar 31, 2026 at 11:07 PM Gary Guo <gary@garyguo.net> wrote:
>
> Does it produce a false positive, or it's a false negative? If it's the latter,
> I think we don't need to disable the lint.
If you mean for custom macros, then I think it is just that it doesn't
take those into account at all. So I guess you could say false
negative if you consider that it should.
In any case, originally I just cleaned it, because it wasn't a big
deal either way, but then I thought it would be best to keep the
behavior the same. So either we disable or enable everywhere.
Cheers,
Miguel
^ permalink raw reply
* Re: [PATCH 2/2] rust: macros: simplify `format!` arguments
From: Gary Guo @ 2026-03-31 21:07 UTC (permalink / raw)
To: Miguel Ojeda, Luis Chamberlain, Petr Pavlu, Daniel Gomez,
Sami Tolvanen, Nathan Chancellor, Nicolas Schier
Cc: Boqun Feng, Gary Guo, Björn Roy Baron, Benno Lossin,
Andreas Hindborg, Alice Ryhl, Trevor Gross, Danilo Krummrich,
rust-for-linux, Aaron Tomlin, linux-modules, linux-kernel,
linux-kbuild
In-Reply-To: <20260331205849.498295-2-ojeda@kernel.org>
On Tue Mar 31, 2026 at 9:58 PM BST, Miguel Ojeda wrote:
> Clippy in Rust 1.88.0 (only) reported [1] up to the previous commit:
>
> warning: variables can be used directly in the `format!` string
> --> rust/macros/module.rs:112:23
> |
> 112 | let content = format!("{param}:{content}", param = param, content = content);
> | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> |
> = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#uninlined_format_args
> = note: `-W clippy::uninlined-format-args` implied by `-W clippy::all`
> = help: to override `-W clippy::all` add `#[allow(clippy::uninlined_format_args)]`
> help: change this to
> |
> 112 - let content = format!("{param}:{content}", param = param, content = content);
> 112 + let content = format!("{param}:{content}");
>
> The reason it only triggers in that version is that the lint was moved
> from `pedantic` to `style` in Rust 1.88.0 and then back to `pedantic`
> in Rust 1.89.0 [2][3].
>
> In this case, the suggestion is fair and a pure simplification, thus
> just apply it.
>
> In addition, do the same for another place in the file that Clippy does
> not report because it is multi-line.
>
> Link: https://lore.kernel.org/rust-for-linux/CANiq72=drAtf3y_DZ-2o4jb6Az9J3Yj4QYwWnbRui4sm4AJD3Q@mail.gmail.com/ [1]
> Link: https://github.com/rust-lang/rust-clippy/pull/15287 [2]
> Link: https://github.com/rust-lang/rust-clippy/issues/15151 [3]
> Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
Reviewed-by: Gary Guo <gary@garyguo.net>
> ---
> rust/macros/module.rs | 9 ++-------
> 1 file changed, 2 insertions(+), 7 deletions(-)
^ permalink raw reply
* Re: [PATCH 1/2] kbuild: rust: allow `clippy::uninlined_format_args`
From: Gary Guo @ 2026-03-31 21:07 UTC (permalink / raw)
To: Miguel Ojeda, Luis Chamberlain, Petr Pavlu, Daniel Gomez,
Sami Tolvanen, Nathan Chancellor, Nicolas Schier
Cc: Boqun Feng, Gary Guo, Björn Roy Baron, Benno Lossin,
Andreas Hindborg, Alice Ryhl, Trevor Gross, Danilo Krummrich,
rust-for-linux, Aaron Tomlin, linux-modules, linux-kernel,
linux-kbuild, stable
In-Reply-To: <20260331205849.498295-1-ojeda@kernel.org>
On Tue Mar 31, 2026 at 9:58 PM BST, Miguel Ojeda wrote:
> Clippy in Rust 1.88.0 (only) reports [1]:
>
> warning: variables can be used directly in the `format!` string
> --> rust/macros/module.rs:112:23
> |
> 112 | let content = format!("{param}:{content}", param = param, content = content);
> | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> |
> = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#uninlined_format_args
> = note: `-W clippy::uninlined-format-args` implied by `-W clippy::all`
> = help: to override `-W clippy::all` add `#[allow(clippy::uninlined_format_args)]`
> help: change this to
> |
> 112 - let content = format!("{param}:{content}", param = param, content = content);
> 112 + let content = format!("{param}:{content}");
>
> warning: variables can be used directly in the `format!` string
> --> rust/macros/module.rs:198:14
> |
> 198 | t => panic!("Unsupported parameter type {}", t),
> | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> |
> = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#uninlined_format_args
> = note: `-W clippy::uninlined-format-args` implied by `-W clippy::all`
> = help: to override `-W clippy::all` add `#[allow(clippy::uninlined_format_args)]`
> help: change this to
> |
> 198 - t => panic!("Unsupported parameter type {}", t),
> 198 + t => panic!("Unsupported parameter type {t}"),
> |
>
> The reason it only triggers in that version is that the lint was moved
> from `pedantic` to `style` in Rust 1.88.0 and then back to `pedantic`
> in Rust 1.89.0 [2][3].
>
> In the first case, the suggestion is fair and a pure simplification, thus
> we will clean it up separately.
>
> To keep the behavior the same across all versions, and since the lint
> does not work for all macros (e.g. custom ones like `pr_info!`), disable
> it globally.
Does it produce a false positive, or it's a false negative? If it's the latter,
I think we don't need to disable the lint.
Best,
Gary
>
> Cc: stable@vger.kernel.org # Needed in 6.12.y and later (Rust is pinned in older LTSs).
> Link: https://lore.kernel.org/rust-for-linux/CANiq72=drAtf3y_DZ-2o4jb6Az9J3Yj4QYwWnbRui4sm4AJD3Q@mail.gmail.com/ [1]
> Link: https://github.com/rust-lang/rust-clippy/pull/15287 [2]
> Link: https://github.com/rust-lang/rust-clippy/issues/15151 [3]
> Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
> ---
> Makefile | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/Makefile b/Makefile
> index 1a219bf1c771..a63684c36d60 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -494,6 +494,7 @@ export rust_common_flags := --edition=2021 \
> -Wclippy::ptr_cast_constness \
> -Wclippy::ref_as_ptr \
> -Wclippy::undocumented_unsafe_blocks \
> + -Aclippy::uninlined_format_args \
> -Wclippy::unnecessary_safety_comment \
> -Wclippy::unnecessary_safety_doc \
> -Wrustdoc::missing_crate_level_docs \
^ permalink raw reply
* [PATCH 2/2] rust: macros: simplify `format!` arguments
From: Miguel Ojeda @ 2026-03-31 20:58 UTC (permalink / raw)
To: Miguel Ojeda, Luis Chamberlain, Petr Pavlu, Daniel Gomez,
Sami Tolvanen, Nathan Chancellor, Nicolas Schier
Cc: Boqun Feng, Gary Guo, Björn Roy Baron, Benno Lossin,
Andreas Hindborg, Alice Ryhl, Trevor Gross, Danilo Krummrich,
rust-for-linux, Aaron Tomlin, linux-modules, linux-kernel,
linux-kbuild
In-Reply-To: <20260331205849.498295-1-ojeda@kernel.org>
Clippy in Rust 1.88.0 (only) reported [1] up to the previous commit:
warning: variables can be used directly in the `format!` string
--> rust/macros/module.rs:112:23
|
112 | let content = format!("{param}:{content}", param = param, content = content);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#uninlined_format_args
= note: `-W clippy::uninlined-format-args` implied by `-W clippy::all`
= help: to override `-W clippy::all` add `#[allow(clippy::uninlined_format_args)]`
help: change this to
|
112 - let content = format!("{param}:{content}", param = param, content = content);
112 + let content = format!("{param}:{content}");
The reason it only triggers in that version is that the lint was moved
from `pedantic` to `style` in Rust 1.88.0 and then back to `pedantic`
in Rust 1.89.0 [2][3].
In this case, the suggestion is fair and a pure simplification, thus
just apply it.
In addition, do the same for another place in the file that Clippy does
not report because it is multi-line.
Link: https://lore.kernel.org/rust-for-linux/CANiq72=drAtf3y_DZ-2o4jb6Az9J3Yj4QYwWnbRui4sm4AJD3Q@mail.gmail.com/ [1]
Link: https://github.com/rust-lang/rust-clippy/pull/15287 [2]
Link: https://github.com/rust-lang/rust-clippy/issues/15151 [3]
Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
---
rust/macros/module.rs | 9 ++-------
1 file changed, 2 insertions(+), 7 deletions(-)
diff --git a/rust/macros/module.rs b/rust/macros/module.rs
index e16298e520c7..06c18e207508 100644
--- a/rust/macros/module.rs
+++ b/rust/macros/module.rs
@@ -52,12 +52,7 @@ fn new(module: &'a str) -> Self {
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!(
- "{module}.{field}={content}\0",
- module = self.module,
- field = field,
- content = content
- )
+ format!("{module}.{field}={content}\0", module = self.module)
} else {
// Loadable modules' modinfo strings go as-is.
format!("{field}={content}\0")
@@ -109,7 +104,7 @@ fn emit_internal(&mut self, field: &str, content: &str, param: bool) {
}
fn emit_param(&mut self, field: &str, param: &str, content: &str) {
- let content = format!("{param}:{content}", param = param, content = content);
+ let content = format!("{param}:{content}");
self.emit_internal(field, &content, true);
}
--
2.53.0
^ permalink raw reply related
* [PATCH 1/2] kbuild: rust: allow `clippy::uninlined_format_args`
From: Miguel Ojeda @ 2026-03-31 20:58 UTC (permalink / raw)
To: Miguel Ojeda, Luis Chamberlain, Petr Pavlu, Daniel Gomez,
Sami Tolvanen, Nathan Chancellor, Nicolas Schier
Cc: Boqun Feng, Gary Guo, Björn Roy Baron, Benno Lossin,
Andreas Hindborg, Alice Ryhl, Trevor Gross, Danilo Krummrich,
rust-for-linux, Aaron Tomlin, linux-modules, linux-kernel,
linux-kbuild, stable
Clippy in Rust 1.88.0 (only) reports [1]:
warning: variables can be used directly in the `format!` string
--> rust/macros/module.rs:112:23
|
112 | let content = format!("{param}:{content}", param = param, content = content);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#uninlined_format_args
= note: `-W clippy::uninlined-format-args` implied by `-W clippy::all`
= help: to override `-W clippy::all` add `#[allow(clippy::uninlined_format_args)]`
help: change this to
|
112 - let content = format!("{param}:{content}", param = param, content = content);
112 + let content = format!("{param}:{content}");
warning: variables can be used directly in the `format!` string
--> rust/macros/module.rs:198:14
|
198 | t => panic!("Unsupported parameter type {}", t),
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#uninlined_format_args
= note: `-W clippy::uninlined-format-args` implied by `-W clippy::all`
= help: to override `-W clippy::all` add `#[allow(clippy::uninlined_format_args)]`
help: change this to
|
198 - t => panic!("Unsupported parameter type {}", t),
198 + t => panic!("Unsupported parameter type {t}"),
|
The reason it only triggers in that version is that the lint was moved
from `pedantic` to `style` in Rust 1.88.0 and then back to `pedantic`
in Rust 1.89.0 [2][3].
In the first case, the suggestion is fair and a pure simplification, thus
we will clean it up separately.
To keep the behavior the same across all versions, and since the lint
does not work for all macros (e.g. custom ones like `pr_info!`), disable
it globally.
Cc: stable@vger.kernel.org # Needed in 6.12.y and later (Rust is pinned in older LTSs).
Link: https://lore.kernel.org/rust-for-linux/CANiq72=drAtf3y_DZ-2o4jb6Az9J3Yj4QYwWnbRui4sm4AJD3Q@mail.gmail.com/ [1]
Link: https://github.com/rust-lang/rust-clippy/pull/15287 [2]
Link: https://github.com/rust-lang/rust-clippy/issues/15151 [3]
Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
---
Makefile | 1 +
1 file changed, 1 insertion(+)
diff --git a/Makefile b/Makefile
index 1a219bf1c771..a63684c36d60 100644
--- a/Makefile
+++ b/Makefile
@@ -494,6 +494,7 @@ export rust_common_flags := --edition=2021 \
-Wclippy::ptr_cast_constness \
-Wclippy::ref_as_ptr \
-Wclippy::undocumented_unsafe_blocks \
+ -Aclippy::uninlined_format_args \
-Wclippy::unnecessary_safety_comment \
-Wclippy::unnecessary_safety_doc \
-Wrustdoc::missing_crate_level_docs \
--
2.53.0
^ permalink raw reply related
* Re: [PATCH v3 2/4] kallsyms: extend lineinfo to loadable modules
From: Petr Pavlu @ 2026-03-31 16:23 UTC (permalink / raw)
To: Sasha Levin
Cc: Andrew Morton, Masahiro Yamada, Luis Chamberlain, Linus Torvalds,
Richard Weinberger, Juergen Gross, Geert Uytterhoeven,
James Bottomley, Jonathan Corbet, Nathan Chancellor,
Nicolas Schier, Daniel Gomez, Greg KH, Petr Mladek,
Steven Rostedt, Kees Cook, Peter Zijlstra, Thorsten Leemhuis,
Vlastimil Babka, Helge Deller, Randy Dunlap, Laurent Pinchart,
Vivian Wang, linux-kernel, linux-kbuild, linux-modules, linux-doc
In-Reply-To: <abwnvdbu4kjx7esJ@laps>
On 3/19/26 5:43 PM, Sasha Levin wrote:
>>> --- /dev/null
>>> +++ b/include/linux/mod_lineinfo.h
>>> @@ -0,0 +1,68 @@
>>> +/* SPDX-License-Identifier: GPL-2.0 */
>>> +/*
>>> + * mod_lineinfo.h - Binary format for per-module source line information
>>> + *
>>> + * This header defines the layout of the .mod_lineinfo section embedded
>>> + * in loadable kernel modules. It is dual-use: included from both the
>>> + * kernel and the userspace gen_lineinfo tool.
>>> + *
>>> + * Section layout (all values in target-native endianness):
>>> + *
>>> + * struct mod_lineinfo_header (16 bytes)
>>> + * u32 addrs[num_entries] -- offsets from .text base, sorted
>>
>> Modules are relocatable objects. The typical way to express a reference
>> from one section to data in another section is to use relocations.
>> Choosing to use an implicit base and resolved offsets means that the
>> code has trouble correctly referencing the .text section and can't
>> express line information data for other sections, such as .exit.text.
>
> I agree, which is why I scoped this just to .text :)
>
> My thinking was that using ELF relocations would add significant complexity to
> both the build tool and the runtime lookup path, which must remain NMI-safe and
> allocation-free.
I agree that using ELF relocations would be somewhat more complex. I had
a look at adding a new u32/u64 "target" member to the
mod_lineinfo_header struct which would point to the .text section in the
resulting module via an absolute relocation. The idea was to keep the
current offset and compression schema and only have a relocation that
identifies the associated text section. I hoped to replace the objcopy
steps in scripts/gen-mod-lineinfo.sh with an additional partial link to
combine ${KO} with ${KO}.lineinfo.o.
However, the issue with this approach is that the relocation in
${KO}.lineinfo.o cannot cross-reference the .text symbol in ${KO}. It
could be worked around by defining a start .text symbol in
scripts/module.lds.S but this doesn't scale well if more sections need
to be covered in the future. A separate utility would likely be required
to join the two objects properly together.
For the runtime part, I don't immediately see any issues. Relocations
are resolved by the module loader when a module is loaded, so this would
have no effect on the functionality being NMI-safe or allocation-free.
>>> +
>>> +#endif /* _LINUX_MOD_LINEINFO_H */
>>> diff --git a/include/linux/module.h b/include/linux/module.h
>>> index 14f391b186c6d..d23e0cd9c7210 100644
>>> --- a/include/linux/module.h
>>> +++ b/include/linux/module.h
>>> @@ -508,6 +508,8 @@ struct module {
>>> void *btf_data;
>>> void *btf_base_data;
>>> #endif
>>> + void *lineinfo_data; /* .mod_lineinfo section in MOD_RODATA */
>>> + unsigned int lineinfo_data_size;
>>
>> The lineinfo-specific members should be enclosed within the `#ifdef
>> CONFIG_KALLSYMS_LINEINFO_MODULES`.
>>
>> This will require module_lookup_lineinfo() to be conditionally compiled
>> based on CONFIG_KALLSYMS_LINEINFO_MODULES, with a dummy version provided
>> otherwise. Alternatively, accessors to module::lineinfo_data and
>> module::lineinfo_data_size that handle CONFIG_KALLSYMS_LINEINFO_MODULES
>> could be introduced in include/linux/module.h. For example, see
>> module_buildid() or is_livepatch_module.
>
> The struct members were deliberately left without #ifdef guards following Helge
> Deller's suggestion in the v1 review[1]. I don't really mind either way, but
> I'd prefer to have a consensus before flipping it back and forth.
The alternative suggestion I mentioned proposes using accessors to
module::lineinfo_data and module::lineinfo_data_size that would handle
CONFIG_KALLSYMS_LINEINFO_MODULES. This approach avoids having unused
data in the module struct when CONFIG_KALLSYMS_LINEINFO_MODULES=n, while
also allowing module_lookup_lineinfo() to use the
IS_ENABLED(CONFIG_KALLSYMS_LINEINFO_MODULES) pattern and be always
syntax-checked. Only the accessors won't be always syntax-checked but
their code should be trivial.
The file include/linux/module.h could contain something like this:
struct {
[...]
#ifdef CONFIG_KALLSYMS_LINEINFO_MODULES
void *lineinfo_data; /* .mod_lineinfo section in MOD_RODATA */
unsigned int lineinfo_data_size;
#endif
[...]
}
[...]
static inline void *module_lineinfo_data(struct module *mod, unsigned int *size)
{
#ifdef CONFIG_KALLSYMS_LINEINFO_MODULES
*size = mod->lineinfo_data_size;
return mod->lineinfo_data;
#else
*size = 0;
return NULL;
#endif
}
>>> @@ -59,6 +62,9 @@ if_changed_except = $(if $(call newer_prereqs_except,$(2))$(cmd-check), \
>>> +$(call if_changed_except,ld_ko_o,$(objtree)/vmlinux)
>>> ifdef CONFIG_DEBUG_INFO_BTF_MODULES
>>> +$(if $(newer-prereqs),$(call cmd,btf_ko))
>>> +endif
>>> +ifdef CONFIG_KALLSYMS_LINEINFO_MODULES
>>> + +$(if $(newer-prereqs),$(call cmd,lineinfo_ko))
>>
>> Should this be 'if_changed_except.. vmlinux'?
>
> Lineinfo generation doesn't depend on vmlinux - it reads DWARF directly from
> the .ko file itself. Unlike BTF (which uses vmlinux as a base for
> deduplication), there's no vmlinux prerequisite to exclude.
The Makefile rule is:
%.ko: %.o %.mod.o .module-common.o $(objtree)/scripts/module.lds $(and $(CONFIG_DEBUG_INFO_BTF_MODULES),$(KBUILD_BUILTIN),$(objtree)/vmlinux) FORCE
If $(and $(CONFIG_DEBUG_INFO_BTF_MODULES),$(KBUILD_BUILTIN)) then
$(objtree)/vmlinux is added as a prerequisite for the target. Since the
new lineinfo_ko call uses $(newer-prereqs), it is invoked even when
vmlinux changes.
However, the mentioned change to 'if_changed_except.. vmlinux' won't
quite work either because if_changed_except appears to have a similar
limitation as if_changed. It can be used only once per target due to
storing the executed command in a corresponding .cmd file.
Additionally, even the already present if_changed_except call for
ld_ko_o doesn't seem to work as intended because it excludes "./vmlinux"
but the same rule prerequisite is canonicalized to only "vmlinux". This
means modules are always linked even when only vmlinux changes. I'll
send a fix for this separately.
>
>>> @@ -194,9 +200,45 @@ static const char *make_relative(const char *path, const char *comp_dir)
>>> return p ? p + 1 : path;
>>> }
>>>
>>> - /* Fall back to basename */
>>> - p = strrchr(path, '/');
>>> - return p ? p + 1 : path;
>>> + /*
>>> + * Relative path — check for duplicated-path quirk from libdw
>>> + * on ET_REL files (e.g., "a/b.c/a/b.c" → "a/b.c").
>>> + */
>>
>> When does this quirk occur? Is it a bug in libdw?
>
> This occurs with elfutils libdw when processing ET_REL .ko files. libdw
> constructs source paths by concatenating DW_AT_comp_dir with DW_AT_name from
> the compilation unit. For modules where both are relative paths with the same
> prefix, this can produce doubled results like "net/foo/bar.c/net/foo/bar.c". It
> appears to be a libdw quirk with ET_REL DWARF handling.
Thanks for the explanation.
-- Petr
^ permalink raw reply
* Re: [PATCH] init.h: discard exitcall symbols early
From: Petr Mladek @ 2026-03-31 15:35 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Arnd Bergmann, Andrew Morton, Nathan Chancellor, Nicolas Schier,
linux-kbuild, Andy Shevchenko, Dmitry Torokhov, Kees Cook,
Marco Elver, Peter Zijlstra, Josh Poimboeuf, Luis Chamberlain,
Petr Pavlu, Daniel Gomez, Sami Tolvanen, Aaron Tomlin,
linux-modules, linux-kernel
In-Reply-To: <82bc9a6f-2184-41fa-af8a-5cf50ac04f44@app.fastmail.com>
On Tue 2026-03-31 17:01:20, Arnd Bergmann wrote:
> On Tue, Mar 31, 2026, at 16:56, Petr Mladek wrote:
> > Adding module loader maintainers into Cc to make them aware of this
> > change.
> >
> > On Tue 2026-03-31 16:28:38, Arnd Bergmann wrote:
> >> From: Arnd Bergmann <arnd@arndb.de>
> >>
> >> Any __exitcall() and built-in module_exit() handler is marked as __used,
> >> which leads to the code being included in the object file and later
> >> discarded at link time.
> >
> > Is this safe for dynamically loaded modules?
> >
> > Honestly, I am not sure what is the exact efect of this change.
> > The dynamically loadded modules just came to my mind...
>
> In a loadable module, using __exitcall() directly already discards
> the function at link time, so there is no difference from built-in
> code. Actually using __exitcall() here is a mistake regardless
> of my patch.
>
> Using module_exit() in a loadable module still behaves as before,
> this uses a different macro, which already has __maybe_unused:
>
> #define module_exit(exitfn) \
> static inline exitcall_t __maybe_unused __exittest(void) \
> { return exitfn; } \
> void cleanup_module(void) __copy(exitfn) \
> __attribute__((alias(#exitfn))); \
> ___ADDRESSABLE(cleanup_module, __exitdata);
>
> so this is also unchanged.
I see, I was confused because cscope pointed me to:
/**
* module_exit() - driver exit entry point
* @x: function to be run when driver is removed
*
* module_exit() will wrap the driver clean-up code
* with cleanup_module() when used with rmmod when
* the driver is a module. If the driver is statically
* compiled into the kernel, module_exit() has no effect.
* There can only be one per module.
*/
#define module_exit(x) __exitcall(x);
and I missed that it was the variant for the built-in modules.
Best Regards,
Petr
^ permalink raw reply
* Re: [PATCH] init.h: discard exitcall symbols early
From: Arnd Bergmann @ 2026-03-31 15:01 UTC (permalink / raw)
To: Petr Mladek, Arnd Bergmann
Cc: Andrew Morton, Nathan Chancellor, Nicolas Schier, linux-kbuild,
Andy Shevchenko, Dmitry Torokhov, Kees Cook, Marco Elver,
Peter Zijlstra, Josh Poimboeuf, Luis Chamberlain, Petr Pavlu,
Daniel Gomez, Sami Tolvanen, Aaron Tomlin, linux-modules,
linux-kernel
In-Reply-To: <acvghO4glmZamFSZ@pathway.suse.cz>
On Tue, Mar 31, 2026, at 16:56, Petr Mladek wrote:
> Adding module loader maintainers into Cc to make them aware of this
> change.
>
> On Tue 2026-03-31 16:28:38, Arnd Bergmann wrote:
>> From: Arnd Bergmann <arnd@arndb.de>
>>
>> Any __exitcall() and built-in module_exit() handler is marked as __used,
>> which leads to the code being included in the object file and later
>> discarded at link time.
>
> Is this safe for dynamically loaded modules?
>
> Honestly, I am not sure what is the exact efect of this change.
> The dynamically loadded modules just came to my mind...
In a loadable module, using __exitcall() directly already discards
the function at link time, so there is no difference from built-in
code. Actually using __exitcall() here is a mistake regardless
of my patch.
Using module_exit() in a loadable module still behaves as before,
this uses a different macro, which already has __maybe_unused:
#define module_exit(exitfn) \
static inline exitcall_t __maybe_unused __exittest(void) \
{ return exitfn; } \
void cleanup_module(void) __copy(exitfn) \
__attribute__((alias(#exitfn))); \
___ADDRESSABLE(cleanup_module, __exitdata);
so this is also unchanged.
Arnd
^ permalink raw reply
* Re: [PATCH] init.h: discard exitcall symbols early
From: Petr Mladek @ 2026-03-31 14:56 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Andrew Morton, Nathan Chancellor, Nicolas Schier, linux-kbuild,
Andy Shevchenko, Dmitry Torokhov, Arnd Bergmann, Kees Cook,
Marco Elver, Peter Zijlstra (Intel), Josh Poimboeuf,
Luis Chamberlain, Petr Pavlu, Daniel Gomez, Sami Tolvanen,
Aaron Tomlin, linux-modules, linux-kernel
In-Reply-To: <20260331142846.3187706-1-arnd@kernel.org>
Adding module loader maintainers into Cc to make them aware of this
change.
On Tue 2026-03-31 16:28:38, Arnd Bergmann wrote:
> From: Arnd Bergmann <arnd@arndb.de>
>
> Any __exitcall() and built-in module_exit() handler is marked as __used,
> which leads to the code being included in the object file and later
> discarded at link time.
Is this safe for dynamically loaded modules?
Honestly, I am not sure what is the exact efect of this change.
The dynamically loadded modules just came to my mind...
Best Regards,
Petr
> As far as I can tell, this was originally added at the same time
> as initcalls were marked the same way, to prevent them from getting
> dropped with gcc-3.4, but it was never actaully necessary to keep exit
> functions around.
>
> Mark them as __maybe_unused instead, which lets the compiler treat
> the exitcalls as entirely unused, and make better decisions about
> dropping specializing static functions called from these.
>
> Link: https://lore.kernel.org/all/acruxMNdnUlyRHiy@google.com/
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
> include/linux/init.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/include/linux/init.h b/include/linux/init.h
> index 5db55c660124..ad5c19763034 100644
> --- a/include/linux/init.h
> +++ b/include/linux/init.h
> @@ -47,7 +47,7 @@
> #define __initdata __section(".init.data")
> #define __initconst __section(".init.rodata")
> #define __exitdata __section(".exit.data")
> -#define __exit_call __used __section(".exitcall.exit")
> +#define __exit_call __maybe_unused __section(".exitcall.exit")
>
> /*
> * modpost check for section mismatches during the kernel build.
> --
> 2.39.5
^ permalink raw reply
* Re: [RFC PATCH] modules: extend {kstrdup,kfree}_const() to handle per-module .rodata
From: kernel test robot @ 2026-03-31 13:37 UTC (permalink / raw)
To: Dmitry Antipov
Cc: oe-lkp, lkp, linux-modules, linux-kernel, linux-mm,
Luis Chamberlain, Petr Pavlu, Daniel Gomez, Sami Tolvanen,
Dmitry Antipov, oliver.sang
In-Reply-To: <20260327132247.861984-1-dmantipov@yandex.ru>
Hello,
kernel test robot noticed "kernel_BUG_at_arch/x86/mm/physaddr.c" on:
commit: 0803ad97505a632fc3d0cddd6659617deb0ff34b ("[RFC PATCH] modules: extend {kstrdup,kfree}_const() to handle per-module .rodata")
url: https://github.com/intel-lab-lkp/linux/commits/Dmitry-Antipov/modules-extend-kstrdup-kfree-_const-to-handle-per-module-rodata/20260329-123400
base: https://git.kernel.org/cgit/linux/kernel/git/akpm/mm.git mm-everything
patch link: https://lore.kernel.org/all/20260327132247.861984-1-dmantipov@yandex.ru/
patch subject: [RFC PATCH] modules: extend {kstrdup,kfree}_const() to handle per-module .rodata
in testcase: boot
config: i386-randconfig-002-20260331
compiler: gcc-14
test machine: qemu-system-i386 -enable-kvm -cpu SandyBridge -smp 2 -m 4G
(please refer to attached dmesg/kmsg for entire log/backtrace)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <oliver.sang@intel.com>
| Closes: https://lore.kernel.org/oe-lkp/202603312130.f9ed8d89-lkp@intel.com
[ 17.209008][ C0] ------------[ cut here ]------------
[ 17.209572][ C0] kernel BUG at arch/x86/mm/physaddr.c:66!
[ 17.210058][ C0] Oops: invalid opcode: 0000 [#1]
[ 17.210470][ C0] CPU: 0 UID: 0 PID: 13 Comm: ksoftirqd/0 Not tainted 7.0.0-rc5-00667-g0803ad97505a #1 PREEMPT(full) 4fef6f10f7892f9689059524fd4d050b8163a9be
[ 17.211614][ C0] EIP: __phys_addr (arch/x86/mm/physaddr.c:66 (discriminator 3))
[ 17.211995][ C0] Code: e5 56 53 85 c0 78 09 0f 0b b8 ec a1 e6 81 eb 21 80 3d f8 e9 4d 82 00 89 c3 8d b0 00 00 00 80 74 15 e8 cf c5 19 00 84 c0 74 0c <0f> 0b b8 e0 a1 e6 81 e8 77 3e fb ff a1 30 14 d3 82 85 c0 74 2a 89
All code
========
0: e5 56 in $0x56,%eax
2: 53 push %rbx
3: 85 c0 test %eax,%eax
5: 78 09 js 0x10
7: 0f 0b ud2
9: b8 ec a1 e6 81 mov $0x81e6a1ec,%eax
e: eb 21 jmp 0x31
10: 80 3d f8 e9 4d 82 00 cmpb $0x0,-0x7db21608(%rip) # 0xffffffff824dea0f
17: 89 c3 mov %eax,%ebx
19: 8d b0 00 00 00 80 lea -0x80000000(%rax),%esi
1f: 74 15 je 0x36
21: e8 cf c5 19 00 call 0x19c5f5
26: 84 c0 test %al,%al
28: 74 0c je 0x36
2a:* 0f 0b ud2 <-- trapping instruction
2c: b8 e0 a1 e6 81 mov $0x81e6a1e0,%eax
31: e8 77 3e fb ff call 0xfffffffffffb3ead
36: a1 30 14 d3 82 85 c0 movabs 0x2a74c08582d31430,%eax
3d: 74 2a
3f: 89 .byte 0x89
Code starting with the faulting instruction
===========================================
0: 0f 0b ud2
2: b8 e0 a1 e6 81 mov $0x81e6a1e0,%eax
7: e8 77 3e fb ff call 0xfffffffffffb3e83
c: a1 30 14 d3 82 85 c0 movabs 0x2a74c08582d31430,%eax
13: 74 2a
15: 89 .byte 0x89
[ 17.214426][ C0] EAX: f7e9f401 EBX: f7e9f4ac ECX: fffff000 EDX: ff5fe000
[ 17.215476][ C0] ESI: 77e9f4ac EDI: 00000000 EBP: 80b0be68 ESP: 80b0be60
[ 17.216539][ C0] DS: 007b ES: 007b FS: 0000 GS: 0000 SS: 0068 EFLAGS: 00010202
[ 17.217741][ C0] CR0: 80050033 CR2: 7fbf9dd8 CR3: 032cd000 CR4: 000406b0
[ 17.218849][ C0] Call Trace:
[ 17.219371][ C0] kfree (mm/slub.c:6474 (discriminator 1))
[ 17.220011][ C0] ? rcu_read_unlock (include/linux/rcupdate.h:884)
[ 17.220786][ C0] kfree_const (mm/util.c:49)
[ 17.221549][ C0] kernfs_free_rcu (fs/kernfs/dir.c:550)
[ 17.222201][ C0] ? lock_is_held (arch/x86/events/intel/core.c:4034)
[ 17.222920][ C0] rcu_do_batch+0x329/0x6a8
[ 17.223812][ C0] rcu_core (kernel/rcu/tree.c:2871)
[ 17.224493][ C0] rcu_core_si (kernel/rcu/tree.c:2887)
[ 17.225255][ C0] handle_softirqs (arch/x86/include/asm/atomic.h:23 include/linux/atomic/atomic-arch-fallback.h:457 include/linux/jump_label.h:262 include/trace/events/irq.h:142 kernel/softirq.c:623)
[ 17.226002][ C0] run_ksoftirqd (kernel/softirq.c:479 kernel/softirq.c:1064 kernel/softirq.c:1055)
[ 17.226737][ C0] smpboot_thread_fn (kernel/smpboot.c:160)
[ 17.227510][ C0] ? sort_range (kernel/smpboot.c:103)
[ 17.228198][ C0] kthread (kernel/kthread.c:438)
[ 17.228839][ C0] ? sort_range (kernel/smpboot.c:103)
[ 17.229601][ C0] ? kthread_affine_node (kernel/kthread.c:381)
[ 17.230400][ C0] ret_from_fork (arch/x86/kernel/process.c:164)
[ 17.231112][ C0] ? kthread_affine_node (kernel/kthread.c:381)
[ 17.231971][ C0] ret_from_fork_asm (arch/x86/entry/entry_32.S:737)
[ 17.232755][ C0] entry_INT80_32 (arch/x86/entry/entry_32.S:942)
[ 17.233621][ C0] Modules linked in: usbtest(+) serio_raw evdev parport_pc parport qemu_fw_cfg button
[ 17.235190][ C0] ---[ end trace 0000000000000000 ]---
[ 17.236068][ C0] EIP: __phys_addr (arch/x86/mm/physaddr.c:66 (discriminator 3))
[ 17.236831][ C0] Code: e5 56 53 85 c0 78 09 0f 0b b8 ec a1 e6 81 eb 21 80 3d f8 e9 4d 82 00 89 c3 8d b0 00 00 00 80 74 15 e8 cf c5 19 00 84 c0 74 0c <0f> 0b b8 e0 a1 e6 81 e8 77 3e fb ff a1 30 14 d3 82 85 c0 74 2a 89
All code
========
0: e5 56 in $0x56,%eax
2: 53 push %rbx
3: 85 c0 test %eax,%eax
5: 78 09 js 0x10
7: 0f 0b ud2
9: b8 ec a1 e6 81 mov $0x81e6a1ec,%eax
e: eb 21 jmp 0x31
10: 80 3d f8 e9 4d 82 00 cmpb $0x0,-0x7db21608(%rip) # 0xffffffff824dea0f
17: 89 c3 mov %eax,%ebx
19: 8d b0 00 00 00 80 lea -0x80000000(%rax),%esi
1f: 74 15 je 0x36
21: e8 cf c5 19 00 call 0x19c5f5
26: 84 c0 test %al,%al
28: 74 0c je 0x36
2a:* 0f 0b ud2 <-- trapping instruction
2c: b8 e0 a1 e6 81 mov $0x81e6a1e0,%eax
31: e8 77 3e fb ff call 0xfffffffffffb3ead
36: a1 30 14 d3 82 85 c0 movabs 0x2a74c08582d31430,%eax
3d: 74 2a
3f: 89 .byte 0x89
Code starting with the faulting instruction
===========================================
0: 0f 0b ud2
2: b8 e0 a1 e6 81 mov $0x81e6a1e0,%eax
7: e8 77 3e fb ff call 0xfffffffffffb3e83
c: a1 30 14 d3 82 85 c0 movabs 0x2a74c08582d31430,%eax
13: 74 2a
15: 89 .byte 0x89
The kernel config and materials to reproduce are available at:
https://download.01.org/0day-ci/archive/20260331/202603312130.f9ed8d89-lkp@intel.com
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply
* [PATCH v2 2/2] module: Simplify warning on positive returns from module_init()
From: Lucas De Marchi @ 2026-03-30 13:13 UTC (permalink / raw)
To: Luis Chamberlain, Daniel Gomez, Sami Tolvanen
Cc: Lucas De Marchi, Greg Kroah-Hartman, Aaron Tomlin, Petr Pavlu,
Daniel Gomez, Phil Sutter, linux-modules, linux-kernel,
Christophe Leroy
In-Reply-To: <-v2-0-a3542e15111c@kernel.org>
It should now be rare to trigger this warning - it doesn't need to be so
verbose. Make it follow the usual style in the module loading code.
For the same reason, drop the dump_stack().
Suggested-by: Petr Pavlu <petr.pavlu@suse.com>
Signed-off-by: Lucas De Marchi <demarchi@kernel.org>
Reviewed-by: Aaron Tomlin <atomlin@atomlin.com>
Reviewed-by: Petr Pavlu <petr.pavlu@suse.com>
Reviewed-by: Daniel Gomez <da.gomez@samsung.com>
---
kernel/module/main.c | 10 +++-------
1 file changed, 3 insertions(+), 7 deletions(-)
diff --git a/kernel/module/main.c b/kernel/module/main.c
index 3b60b7cda329..5aeb5af584b4 100644
--- a/kernel/module/main.c
+++ b/kernel/module/main.c
@@ -3055,13 +3055,9 @@ static noinline int do_init_module(struct module *mod)
goto fail_free_freeinit;
}
- if (ret > 0) {
- pr_warn("%s: '%s'->init suspiciously returned %d, it should "
- "follow 0/-E convention\n"
- "%s: loading module anyway...\n",
- __func__, mod->name, ret, __func__);
- dump_stack();
- }
+ if (ret > 0)
+ pr_warn("%s: init suspiciously returned %d, it should follow 0/-E convention\n",
+ mod->name, ret);
/* Now it's a first class citizen! */
mod->state = MODULE_STATE_LIVE;
--
2.53.0
^ permalink raw reply related
* [PATCH v2 1/2] module: Override -EEXISTS module return
From: Lucas De Marchi @ 2026-03-30 13:13 UTC (permalink / raw)
To: Luis Chamberlain, Daniel Gomez, Sami Tolvanen
Cc: Lucas De Marchi, Greg Kroah-Hartman, Aaron Tomlin, Petr Pavlu,
Daniel Gomez, Phil Sutter, linux-modules, linux-kernel,
Christophe Leroy, Christophe Leroy
In-Reply-To: <-v2-0-a3542e15111c@kernel.org>
The -EEXIST errno is reserved by the module loading functionality. When
userspace calls [f]init_module(), it expects a -EEXIST to mean that the
module is already loaded in the kernel. If module_init() returns it,
that is not true anymore.
Override the error when returning to userspace: it doesn't make sense to
change potentially long error propagation call chains just because it's
will end up as the return of module_init().
Closes: https://lore.kernel.org/all/aKLzsAX14ybEjHfJ@orbyte.nwl.cc/
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Aaron Tomlin <atomlin@atomlin.com>
Cc: Petr Pavlu <petr.pavlu@suse.com>
Cc: Daniel Gomez <da.gomez@samsung.com>
Cc: Phil Sutter <phil@nwl.cc>
Cc: Christophe Leroy <christophe.leroy@csgroup.eu>
Signed-off-by: Lucas De Marchi <demarchi@kernel.org>
---
kernel/module/main.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/kernel/module/main.c b/kernel/module/main.c
index c3ce106c70af..3b60b7cda329 100644
--- a/kernel/module/main.c
+++ b/kernel/module/main.c
@@ -3045,6 +3045,14 @@ static noinline int do_init_module(struct module *mod)
if (mod->init != NULL)
ret = do_one_initcall(mod->init);
if (ret < 0) {
+ /*
+ * -EEXIST is reserved by [f]init_module() to signal to usersapace that
+ * a module with this name is already loaded. Use something else if the
+ * module itself returning that.
+ */
+ if (ret == -EEXIST)
+ ret = -EBUSY;
+
goto fail_free_freeinit;
}
if (ret > 0) {
--
2.53.0
^ permalink raw reply related
* [PATCH v2 0/2] module: Tweak return and warning
From: Lucas De Marchi @ 2026-03-30 13:13 UTC (permalink / raw)
To: Luis Chamberlain, Daniel Gomez, Sami Tolvanen
Cc: Lucas De Marchi, Greg Kroah-Hartman, Aaron Tomlin, Petr Pavlu,
Daniel Gomez, Phil Sutter, linux-modules, linux-kernel,
Christophe Leroy, Christophe Leroy
Do not let userspace tools and end users confused on the return code and
log messages.
To: Luis Chamberlain <mcgrof@kernel.org>
To: Daniel Gomez <da.gomez@kernel.org>
To: Sami Tolvanen <samitolvanen@google.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Aaron Tomlin <atomlin@atomlin.com>
Cc: Petr Pavlu <petr.pavlu@suse.com>
Cc: Daniel Gomez <da.gomez@samsung.com>
Cc: Phil Sutter <phil@nwl.cc>
Cc: Christophe Leroy <christophe.leroy@csgroup.eu>
Cc: linux-modules@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
XXX "from-thread": "20251013-module-warn-ret-v1-0-ab65b41af01f@intel.com"
XXX "change-id": "20260324-module-warn-ret-7bde7d4851be",
---
Lucas De Marchi (2):
module: Override -EEXISTS module return
module: Simplify warning on positive returns from module_init()
kernel/module/main.c | 18 +++++++++++-------
1 file changed, 11 insertions(+), 7 deletions(-)
---
base-commit: c369299895a591d96745d6492d4888259b004a9e
change-id:
Best regards,
--
Lucas De Marchi <demarchi@kernel.org>
^ permalink raw reply
* [RFC PATCH] modules: extend {kstrdup,kfree}_const() to handle per-module .rodata
From: Dmitry Antipov @ 2026-03-27 13:22 UTC (permalink / raw)
To: Luis Chamberlain, Petr Pavlu, Daniel Gomez, Sami Tolvanen
Cc: linux-modules, linux-mm, Dmitry Antipov
Since kernel modules has their own .rodata sections, functions like
'kstrdup_const()' called from the module context are not required to
copy string constants from these sections. Likewise, 'kfree_const()'
in such a context becomes a no-op also because the whole module's
.rodata is freed at module unloading. OTOH this proof-of-concept
implementation introduces substantial overhead due to calls to
'__module_address()' from 'is_module_rodata()', which is much slower
than 'is_kernel_rodata()'. Anyway, comments are highly appreciated.
Signed-off-by: Dmitry Antipov <dmantipov@yandex.ru>
---
include/linux/module.h | 6 ++++++
kernel/module/main.c | 9 +++++++++
mm/util.c | 7 +++++--
3 files changed, 20 insertions(+), 2 deletions(-)
diff --git a/include/linux/module.h b/include/linux/module.h
index 14f391b186c6..7edaf2b730ce 100644
--- a/include/linux/module.h
+++ b/include/linux/module.h
@@ -623,6 +623,7 @@ bool is_module_address(unsigned long addr);
bool __is_module_percpu_address(unsigned long addr, unsigned long *can_addr);
bool is_module_percpu_address(unsigned long addr);
bool is_module_text_address(unsigned long addr);
+bool is_module_rodata(unsigned long addr);
static inline bool within_module_mem_type(unsigned long addr,
const struct module *mod,
@@ -807,6 +808,11 @@ static inline bool is_module_text_address(unsigned long addr)
return false;
}
+static inline bool is_module_rodata(unsigned long addr)
+{
+ return false;
+}
+
static inline bool within_module_core(unsigned long addr,
const struct module *mod)
{
diff --git a/kernel/module/main.c b/kernel/module/main.c
index c3ce106c70af..5003cacd0786 100644
--- a/kernel/module/main.c
+++ b/kernel/module/main.c
@@ -3858,6 +3858,15 @@ bool is_module_text_address(unsigned long addr)
return __module_text_address(addr) != NULL;
}
+bool is_module_rodata(unsigned long addr)
+{
+ struct module *mod;
+
+ guard(rcu)();
+ mod = __module_address(addr);
+ return mod && within_module_mem_type(addr, mod, MOD_RODATA);
+}
+
void module_for_each_mod(int(*func)(struct module *mod, void *data), void *data)
{
struct module *mod;
diff --git a/mm/util.c b/mm/util.c
index b05ab6f97e11..8dd1f1e95554 100644
--- a/mm/util.c
+++ b/mm/util.c
@@ -26,6 +26,7 @@
#include <linux/compat.h>
#include <linux/fsnotify.h>
#include <linux/page_idle.h>
+#include <linux/module.h>
#include <linux/uaccess.h>
@@ -42,7 +43,8 @@
*/
void kfree_const(const void *x)
{
- if (!is_kernel_rodata((unsigned long)x))
+ if (!is_kernel_rodata((unsigned long)x) &&
+ !is_module_rodata((unsigned long)x))
kfree(x);
}
EXPORT_SYMBOL(kfree_const);
@@ -98,7 +100,8 @@ EXPORT_SYMBOL(kstrdup);
*/
const char *kstrdup_const(const char *s, gfp_t gfp)
{
- if (is_kernel_rodata((unsigned long)s))
+ if (is_kernel_rodata((unsigned long)s) ||
+ is_module_rodata((unsigned long)s))
return s;
return kstrdup(s, gfp);
--
2.53.0
^ permalink raw reply related
* Re: [PATCH] module: remove MODULE_VERSION()
From: Greg Kroah-Hartman @ 2026-03-27 12:44 UTC (permalink / raw)
To: Nam Cao
Cc: Petr Pavlu, linux-modules, linux-kernel, Luis Chamberlain,
Daniel Gomez, Sami Tolvanen, Aaron Tomlin, Shyam Saini, Kees Cook,
Thorsten Blum, Christoph Hellwig
In-Reply-To: <87cy0p5ufl.fsf@yellow.woof>
On Fri, Mar 27, 2026 at 01:27:26PM +0100, Nam Cao wrote:
> Petr Pavlu <petr.pavlu@suse.com> writes:
> > On 3/13/26 3:20 PM, Greg Kroah-Hartman wrote:
> >> Module "versions" do not make sense as the kernel is built all at once,
> >> the "version" is the overall kernel version number, so modules can not
> >> really be described as having a unique version given that they rely on
> >> the infrastructure of the whole kernel.
> >>
> >> For now, just make this an "empty" define, to keep existing code
> >> building properly as the tree is slowly purged of the use of this over
> >> time.
> >>
> >> This macro will be removed entirely in the future when there are no
> >> in-tree users.
> ...
> > The original patch "Add a MODULE_VERSION macro" [1] from 2004 doesn't
> > say much about the motivation for adding module versions, but it does
> > mention that they should be accessible via sysfs. That was implemented
> > a year later in commit c988d2b28454 ("[PATCH] modules: add version and
> > srcversion to sysfs") [2], which primarily discusses use cases related
> > to DKMS, and to administrators + tech support needing to know what is
> > actually loaded on the system. For the latter, I believe srcversion (or
> > something similar) should be sufficient.
>
> I develop an external module. And our userspace program does rely on
> this to get the module's version on the user's system. This patch would
> break our program.
Why not get the module merged upstream so you don't have this problem
anymore?
> I can change to use a different mechanism. But surely I am not the only
> one who write something that rely on this.
I recommend changing it, as this is not needed in the tree :)
I've been at a conference all week, will respin this patch series next
week and resend, sorry for the delay.
thanks,
greg k-h
^ permalink raw reply
* Re: [PATCH] module: remove MODULE_VERSION()
From: Nam Cao @ 2026-03-27 12:27 UTC (permalink / raw)
To: Petr Pavlu, Greg Kroah-Hartman
Cc: linux-modules, linux-kernel, Luis Chamberlain, Daniel Gomez,
Sami Tolvanen, Aaron Tomlin, Shyam Saini, Kees Cook,
Thorsten Blum, Christoph Hellwig
In-Reply-To: <d622c70a-f79a-4215-84fb-c5de0a8f6ce5@suse.com>
Petr Pavlu <petr.pavlu@suse.com> writes:
> On 3/13/26 3:20 PM, Greg Kroah-Hartman wrote:
>> Module "versions" do not make sense as the kernel is built all at once,
>> the "version" is the overall kernel version number, so modules can not
>> really be described as having a unique version given that they rely on
>> the infrastructure of the whole kernel.
>>
>> For now, just make this an "empty" define, to keep existing code
>> building properly as the tree is slowly purged of the use of this over
>> time.
>>
>> This macro will be removed entirely in the future when there are no
>> in-tree users.
...
> The original patch "Add a MODULE_VERSION macro" [1] from 2004 doesn't
> say much about the motivation for adding module versions, but it does
> mention that they should be accessible via sysfs. That was implemented
> a year later in commit c988d2b28454 ("[PATCH] modules: add version and
> srcversion to sysfs") [2], which primarily discusses use cases related
> to DKMS, and to administrators + tech support needing to know what is
> actually loaded on the system. For the latter, I believe srcversion (or
> something similar) should be sufficient.
I develop an external module. And our userspace program does rely on
this to get the module's version on the user's system. This patch would
break our program.
I can change to use a different mechanism. But surely I am not the only
one who write something that rely on this.
Best regards,
Nam
^ permalink raw reply
* [PATCH v2 2/2] module/kallsyms: sort function symbols and use binary search
From: Stanislaw Gruszka @ 2026-03-27 11:00 UTC (permalink / raw)
To: linux-modules, Sami Tolvanen, Luis Chamberlain, Petr Pavlu
Cc: linux-kernel, linux-trace-kernel, live-patching, Daniel Gomez,
Aaron Tomlin, Steven Rostedt, Masami Hiramatsu, Jordan Rome,
Viktor Malik
In-Reply-To: <20260327110005.16499-1-stf_xl@wp.pl>
Module symbol lookup via find_kallsyms_symbol() performs a linear scan
over the entire symtab when resolving an address. The number of symbols
in module symtabs has grown over the years, largely due to additional
metadata in non-standard sections, making this lookup very slow.
Improve this by separating function symbols during module load, placing
them at the beginning of the symtab, sorting them by address, and using
binary search when resolving addresses in module text.
This also should improve times for linear symbol name lookups, as valid
function symbols are now located at the beginning of the symtab.
The cost of sorting is small relative to module load time. In repeated
module load tests [1], depending on .config options, this change
increases load time between 2% and 4%. With cold caches, the difference
is not measurable, as memory access latency dominates.
The sorting theoretically could be done in compile time, but much more
complicated as we would have to simulate kernel addresses resolution
for symbols, and then correct relocation entries. That would be risky
if get out of sync.
The improvement can be observed when listing ftrace filter functions.
Before:
root@nano:~# time cat /sys/kernel/tracing/available_filter_functions | wc -l
74908
real 0m1.315s
user 0m0.000s
sys 0m1.312s
After:
root@nano:~# time cat /sys/kernel/tracing/available_filter_functions | wc -l
74911
real 0m0.167s
user 0m0.004s
sys 0m0.175s
(there are three more symbols introduced by the patch)
For livepatch modules, the symtab layout is preserved and the existing
linear search is used. For this case, it should be possible to keep
the original ELF symtab instead of copying it 1:1, but that is outside
the scope of this patch.
Link: https://gist.github.com/sgruszka/09f3fb1dad53a97b1aad96e1927ab117 [1]
Signed-off-by: Stanislaw Gruszka <stf_xl@wp.pl>
---
v1 -> v2:
- fix searching data symbols for CONFIG_KALLSYMS_ALL
- use kallsyms_symbol_value() in elf_sym_cmp()
include/linux/module.h | 1 +
kernel/module/internal.h | 1 +
kernel/module/kallsyms.c | 171 +++++++++++++++++++++++++++++----------
3 files changed, 130 insertions(+), 43 deletions(-)
diff --git a/include/linux/module.h b/include/linux/module.h
index ac254525014c..67c053afa882 100644
--- a/include/linux/module.h
+++ b/include/linux/module.h
@@ -379,6 +379,7 @@ struct module_memory {
struct mod_kallsyms {
Elf_Sym *symtab;
unsigned int num_symtab;
+ unsigned int num_func_syms;
char *strtab;
char *typetab;
};
diff --git a/kernel/module/internal.h b/kernel/module/internal.h
index 618202578b42..6a4d498619b1 100644
--- a/kernel/module/internal.h
+++ b/kernel/module/internal.h
@@ -73,6 +73,7 @@ struct load_info {
bool sig_ok;
#ifdef CONFIG_KALLSYMS
unsigned long mod_kallsyms_init_off;
+ unsigned long num_func_syms;
#endif
#ifdef CONFIG_MODULE_DECOMPRESS
#ifdef CONFIG_MODULE_STATS
diff --git a/kernel/module/kallsyms.c b/kernel/module/kallsyms.c
index f23126d804b2..d69e99e67707 100644
--- a/kernel/module/kallsyms.c
+++ b/kernel/module/kallsyms.c
@@ -10,6 +10,7 @@
#include <linux/kallsyms.h>
#include <linux/buildid.h>
#include <linux/bsearch.h>
+#include <linux/sort.h>
#include "internal.h"
/* Lookup exported symbol in given range of kernel_symbols */
@@ -103,6 +104,95 @@ static bool is_core_symbol(const Elf_Sym *src, const Elf_Shdr *sechdrs,
return true;
}
+static inline bool is_func_symbol(const Elf_Sym *sym)
+{
+ return sym->st_shndx != SHN_UNDEF && sym->st_size != 0 &&
+ ELF_ST_TYPE(sym->st_info) == STT_FUNC;
+}
+
+static unsigned int bsearch_func_symbol(struct mod_kallsyms *kallsyms,
+ unsigned long addr,
+ unsigned long *bestval,
+ unsigned long *nextval)
+
+{
+ unsigned int mid, low = 1, high = kallsyms->num_func_syms + 1;
+ unsigned int best = 0;
+ unsigned long thisval;
+
+ while (low < high) {
+ mid = low + (high - low) / 2;
+ thisval = kallsyms_symbol_value(&kallsyms->symtab[mid]);
+
+ if (thisval <= addr) {
+ *bestval = thisval;
+ best = mid;
+ low = mid + 1;
+ } else {
+ *nextval = thisval;
+ high = mid;
+ }
+ }
+
+ return best;
+}
+
+static const char *kallsyms_symbol_name(struct mod_kallsyms *kallsyms,
+ unsigned int symnum)
+{
+ return kallsyms->strtab + kallsyms->symtab[symnum].st_name;
+}
+
+static unsigned int search_kallsyms_symbol(struct mod_kallsyms *kallsyms,
+ unsigned long addr,
+ unsigned long *bestval,
+ unsigned long *nextval)
+{
+ unsigned int i, best = 0;
+
+ /*
+ * Scan for closest preceding symbol and next symbol. (ELF starts
+ * real symbols at 1). Skip the initial function symbols range
+ * if num_func_syms is non-zero, those are handled separately for
+ * the core TEXT segment lookup.
+ */
+ for (i = 1 + kallsyms->num_func_syms; i < kallsyms->num_symtab; i++) {
+ const Elf_Sym *sym = &kallsyms->symtab[i];
+ unsigned long thisval = kallsyms_symbol_value(sym);
+
+ if (sym->st_shndx == SHN_UNDEF)
+ continue;
+
+ /*
+ * We ignore unnamed symbols: they're uninformative
+ * and inserted at a whim.
+ */
+ if (*kallsyms_symbol_name(kallsyms, i) == '\0' ||
+ is_mapping_symbol(kallsyms_symbol_name(kallsyms, i)))
+ continue;
+
+ if (thisval <= addr && thisval > *bestval) {
+ best = i;
+ *bestval = thisval;
+ }
+ if (thisval > addr && thisval < *nextval)
+ *nextval = thisval;
+ }
+
+ return best;
+}
+
+static int elf_sym_cmp(const void *a, const void *b)
+{
+ unsigned long val_a = kallsyms_symbol_value((const Elf_Sym *)a);
+ unsigned long val_b = kallsyms_symbol_value((const Elf_Sym *)b);
+
+ if (val_a < val_b)
+ return -1;
+
+ return val_a > val_b;
+}
+
/*
* We only allocate and copy the strings needed by the parts of symtab
* we keep. This is simple, but has the effect of making multiple
@@ -115,9 +205,10 @@ void layout_symtab(struct module *mod, struct load_info *info)
Elf_Shdr *symsect = info->sechdrs + info->index.sym;
Elf_Shdr *strsect = info->sechdrs + info->index.str;
const Elf_Sym *src;
- unsigned int i, nsrc, ndst, strtab_size = 0;
+ unsigned int i, nsrc, ndst, nfunc, strtab_size = 0;
struct module_memory *mod_mem_data = &mod->mem[MOD_DATA];
struct module_memory *mod_mem_init_data = &mod->mem[MOD_INIT_DATA];
+ bool is_lp_mod = is_livepatch_module(mod);
/* Put symbol section at end of init part of module. */
symsect->sh_flags |= SHF_ALLOC;
@@ -129,12 +220,14 @@ void layout_symtab(struct module *mod, struct load_info *info)
nsrc = symsect->sh_size / sizeof(*src);
/* Compute total space required for the core symbols' strtab. */
- for (ndst = i = 0; i < nsrc; i++) {
- if (i == 0 || is_livepatch_module(mod) ||
+ for (ndst = nfunc = i = 0; i < nsrc; i++) {
+ if (i == 0 || is_lp_mod ||
is_core_symbol(src + i, info->sechdrs, info->hdr->e_shnum,
info->index.pcpu)) {
strtab_size += strlen(&info->strtab[src[i].st_name]) + 1;
ndst++;
+ if (!is_lp_mod && is_func_symbol(src + i))
+ nfunc++;
}
}
@@ -156,6 +249,7 @@ void layout_symtab(struct module *mod, struct load_info *info)
mod_mem_init_data->size = ALIGN(mod_mem_init_data->size,
__alignof__(struct mod_kallsyms));
info->mod_kallsyms_init_off = mod_mem_init_data->size;
+ info->num_func_syms = nfunc;
mod_mem_init_data->size += sizeof(struct mod_kallsyms);
info->init_typeoffs = mod_mem_init_data->size;
@@ -169,7 +263,7 @@ void layout_symtab(struct module *mod, struct load_info *info)
*/
void add_kallsyms(struct module *mod, const struct load_info *info)
{
- unsigned int i, ndst;
+ unsigned int i, di, nfunc, ndst;
const Elf_Sym *src;
Elf_Sym *dst;
char *s;
@@ -178,6 +272,7 @@ void add_kallsyms(struct module *mod, const struct load_info *info)
void *data_base = mod->mem[MOD_DATA].base;
void *init_data_base = mod->mem[MOD_INIT_DATA].base;
struct mod_kallsyms *kallsyms;
+ bool is_lp_mod = is_livepatch_module(mod);
kallsyms = init_data_base + info->mod_kallsyms_init_off;
@@ -194,19 +289,28 @@ void add_kallsyms(struct module *mod, const struct load_info *info)
mod->core_kallsyms.symtab = dst = data_base + info->symoffs;
mod->core_kallsyms.strtab = s = data_base + info->stroffs;
mod->core_kallsyms.typetab = data_base + info->core_typeoffs;
+
strtab_size = info->core_typeoffs - info->stroffs;
src = kallsyms->symtab;
- for (ndst = i = 0; i < kallsyms->num_symtab; i++) {
+ ndst = info->num_func_syms + 1;
+
+ for (nfunc = i = 0; i < kallsyms->num_symtab; i++) {
kallsyms->typetab[i] = elf_type(src + i, info);
- if (i == 0 || is_livepatch_module(mod) ||
+ if (i == 0 || is_lp_mod ||
is_core_symbol(src + i, info->sechdrs, info->hdr->e_shnum,
info->index.pcpu)) {
ssize_t ret;
- mod->core_kallsyms.typetab[ndst] =
- kallsyms->typetab[i];
- dst[ndst] = src[i];
- dst[ndst++].st_name = s - mod->core_kallsyms.strtab;
+ if (i == 0)
+ di = 0;
+ else if (!is_lp_mod && is_func_symbol(src + i))
+ di = 1 + nfunc++;
+ else
+ di = ndst++;
+
+ mod->core_kallsyms.typetab[di] = kallsyms->typetab[i];
+ dst[di] = src[i];
+ dst[di].st_name = s - mod->core_kallsyms.strtab;
ret = strscpy(s, &kallsyms->strtab[src[i].st_name],
strtab_size);
if (ret < 0)
@@ -216,9 +320,13 @@ void add_kallsyms(struct module *mod, const struct load_info *info)
}
}
+ WARN_ON_ONCE(nfunc != info->num_func_syms);
+ sort(dst + 1, nfunc, sizeof(Elf_Sym), elf_sym_cmp, NULL);
+
/* Set up to point into init section. */
rcu_assign_pointer(mod->kallsyms, kallsyms);
mod->core_kallsyms.num_symtab = ndst;
+ mod->core_kallsyms.num_func_syms = nfunc;
}
#if IS_ENABLED(CONFIG_STACKTRACE_BUILD_ID)
@@ -241,11 +349,6 @@ void init_build_id(struct module *mod, const struct load_info *info)
}
#endif
-static const char *kallsyms_symbol_name(struct mod_kallsyms *kallsyms, unsigned int symnum)
-{
- return kallsyms->strtab + kallsyms->symtab[symnum].st_name;
-}
-
/*
* Given a module and address, find the corresponding symbol and return its name
* while providing its size and offset if needed.
@@ -255,7 +358,10 @@ static const char *find_kallsyms_symbol(struct module *mod,
unsigned long *size,
unsigned long *offset)
{
- unsigned int i, best = 0;
+ unsigned int (*search)(struct mod_kallsyms *kallsyms,
+ unsigned long addr, unsigned long *bestval,
+ unsigned long *nextval);
+ unsigned int best;
unsigned long nextval, bestval;
struct mod_kallsyms *kallsyms = rcu_dereference(mod->kallsyms);
struct module_memory *mod_mem = NULL;
@@ -266,6 +372,11 @@ static const char *find_kallsyms_symbol(struct module *mod,
continue;
#endif
if (within_module_mem_type(addr, mod, type)) {
+ if (type == MOD_TEXT && kallsyms->num_func_syms > 0)
+ search = bsearch_func_symbol;
+ else
+ search = search_kallsyms_symbol;
+
mod_mem = &mod->mem[type];
break;
}
@@ -278,33 +389,7 @@ static const char *find_kallsyms_symbol(struct module *mod,
nextval = (unsigned long)mod_mem->base + mod_mem->size;
bestval = (unsigned long)mod_mem->base - 1;
- /*
- * Scan for closest preceding symbol, and next symbol. (ELF
- * starts real symbols at 1).
- */
- for (i = 1; i < kallsyms->num_symtab; i++) {
- const Elf_Sym *sym = &kallsyms->symtab[i];
- unsigned long thisval = kallsyms_symbol_value(sym);
-
- if (sym->st_shndx == SHN_UNDEF)
- continue;
-
- /*
- * We ignore unnamed symbols: they're uninformative
- * and inserted at a whim.
- */
- if (*kallsyms_symbol_name(kallsyms, i) == '\0' ||
- is_mapping_symbol(kallsyms_symbol_name(kallsyms, i)))
- continue;
-
- if (thisval <= addr && thisval > bestval) {
- best = i;
- bestval = thisval;
- }
- if (thisval > addr && thisval < nextval)
- nextval = thisval;
- }
-
+ best = search(kallsyms, addr, &bestval, &nextval);
if (!best)
return NULL;
--
2.50.1
^ permalink raw reply related
* [PATCH v2 1/2] module/kallsyms: fix nextval for data symbol lookup
From: Stanislaw Gruszka @ 2026-03-27 11:00 UTC (permalink / raw)
To: linux-modules, Sami Tolvanen, Luis Chamberlain, Petr Pavlu
Cc: linux-kernel, linux-trace-kernel, live-patching, Daniel Gomez,
Aaron Tomlin, Steven Rostedt, Masami Hiramatsu, Jordan Rome,
Viktor Malik
The symbol lookup code assumes the queried address resides in either
MOD_TEXT or MOD_INIT_TEXT. This breaks for addresses in other module
memory regions (e.g. rodata or data), resulting in incorrect upper
bounds and wrong symbol size.
Select the module memory region the address belongs to instead of
hardcoding text sections. Also initialize the lower bound to the start
of that region, as searching from address 0 is unnecessary.
Signed-off-by: Stanislaw Gruszka <stf_xl@wp.pl>
---
v1 -> v2: new patch.
kernel/module/kallsyms.c | 24 ++++++++++++++++--------
1 file changed, 16 insertions(+), 8 deletions(-)
diff --git a/kernel/module/kallsyms.c b/kernel/module/kallsyms.c
index 0fc11e45df9b..f23126d804b2 100644
--- a/kernel/module/kallsyms.c
+++ b/kernel/module/kallsyms.c
@@ -258,17 +258,25 @@ static const char *find_kallsyms_symbol(struct module *mod,
unsigned int i, best = 0;
unsigned long nextval, bestval;
struct mod_kallsyms *kallsyms = rcu_dereference(mod->kallsyms);
- struct module_memory *mod_mem;
+ struct module_memory *mod_mem = NULL;
- /* At worse, next value is at end of module */
- if (within_module_init(addr, mod))
- mod_mem = &mod->mem[MOD_INIT_TEXT];
- else
- mod_mem = &mod->mem[MOD_TEXT];
+ for_each_mod_mem_type(type) {
+#ifndef CONFIG_KALLSYMS_ALL
+ if (!mod_mem_type_is_text(type))
+ continue;
+#endif
+ if (within_module_mem_type(addr, mod, type)) {
+ mod_mem = &mod->mem[type];
+ break;
+ }
+ }
- nextval = (unsigned long)mod_mem->base + mod_mem->size;
+ if (!mod_mem)
+ return NULL;
- bestval = kallsyms_symbol_value(&kallsyms->symtab[best]);
+ /* Initialize bounds within memory region the address belongs to. */
+ nextval = (unsigned long)mod_mem->base + mod_mem->size;
+ bestval = (unsigned long)mod_mem->base - 1;
/*
* Scan for closest preceding symbol, and next symbol. (ELF
--
2.50.1
^ permalink raw reply related
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