* Re: [PATCH v2] rust: module_param: return value by copy from `value`
From: Miguel Ojeda @ 2026-07-10 17:33 UTC (permalink / raw)
To: Petr Pavlu
Cc: Alice Ryhl, Andreas Hindborg, Miguel Ojeda, Luis Chamberlain,
Daniel Gomez, Sami Tolvanen, Aaron Tomlin, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Trevor Gross,
Danilo Krummrich, linux-modules, linux-kernel, rust-for-linux
In-Reply-To: <CANiq72nxK=nCf7C-GQ8JCtaTnOaDGwGDFKoQEVb4fqNonkKn3w@mail.gmail.com>
On Fri, Jul 10, 2026 at 7:00 PM Miguel Ojeda
<miguel.ojeda.sandonis@gmail.com> wrote:
>
> It looks like `panic_null_reference_constructed`, added the other day
> -- I will send the patch.
https://lore.kernel.org/rust-for-linux/20260710173252.191781-1-ojeda@kernel.org/
Cheers,
Miguel
^ permalink raw reply
* Re: [PATCH v2] rust: module_param: return value by copy from `value`
From: Miguel Ojeda @ 2026-07-10 17:00 UTC (permalink / raw)
To: Petr Pavlu
Cc: Alice Ryhl, Andreas Hindborg, Miguel Ojeda, Luis Chamberlain,
Daniel Gomez, Sami Tolvanen, Aaron Tomlin, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Trevor Gross,
Danilo Krummrich, linux-modules, linux-kernel, rust-for-linux
In-Reply-To: <bd1f78bc-51b4-44b7-8e3c-7accc9e173f2@suse.com>
On Fri, Jul 10, 2026 at 5:58 PM Petr Pavlu <petr.pavlu@suse.com> wrote:
>
> I can't immediately reproduce this issue. I tried with linux-next
> (next-20260710), rustc 1.99.0-nightly (af3d95584 2026-07-09) and
> x86_64_defconfig with Rust enabled. What am I missing? Could you send me
> your config and the produced rust/kernel.o off-list?
I can reproduce it independently of this patch.
It looks like `panic_null_reference_constructed`, added the other day
-- I will send the patch.
Cheers,
Miguel
^ permalink raw reply
* Re: [PATCH v4] module: Extend module_blacklist parameter to built-in modules
From: Arnd Bergmann @ 2026-07-10 15:59 UTC (permalink / raw)
To: Aaron Tomlin, Luis Chamberlain, Petr Pavlu, da.gomez,
Sami Tolvanen, Peter Zijlstra
Cc: Andrew Morton, Masami Hiramatsu, neelx, da.anzani, sean, chjohnst,
steve, mproche, nick.lane, Linux-Arch, linux-modules,
linux-kernel
In-Reply-To: <20260708020007.55728-1-atomlin@atomlin.com>
On Wed, Jul 8, 2026, at 04:00, Aaron Tomlin wrote:
> Currently, the "module_blacklist=" command-line parameter only applies
> to loadable modules. If a module is built-in, the parameter is silently
> ignored. This patch extends the blacklisting functionality to built-in
> modules by intercepting their initialisation routines during early boot.
Andrew already asked you to provide more background on what you need
this part for. Do you have a specific driver you need to disable?
Can't you do the same thing using initcall_blacklist?
> To preserve the existing user-space ABI, "module_blacklist=" is kept
> as a legacy alias pointing to the same module_denylist variable.
It looks like the denylist is only introduced in the same patch?
That sounds more useful, but would better be done in a separate
change, and also needs a proper changelog text.
Arnd
^ permalink raw reply
* Re: [PATCH v2] rust: module_param: return value by copy from `value`
From: Petr Pavlu @ 2026-07-10 15:58 UTC (permalink / raw)
To: Alice Ryhl
Cc: Miguel Ojeda, Andreas Hindborg, Miguel Ojeda, Luis Chamberlain,
Daniel Gomez, Sami Tolvanen, Aaron Tomlin, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Trevor Gross,
Danilo Krummrich, linux-modules, linux-kernel, rust-for-linux
In-Reply-To: <alEBInX9gD1M5NAr@google.com>
On 7/10/26 4:26 PM, Alice Ryhl wrote:
> On Fri, Jul 10, 2026 at 10:13:53AM +0200, Petr Pavlu wrote:
>> On 7/9/26 4:13 PM, Miguel Ojeda wrote:
>>> On Thu, Jul 9, 2026 at 1:48 PM Petr Pavlu <petr.pavlu@suse.com> wrote:
>>>>
>>>> @Miguel, please let me know if I should take this patch on modules-next,
>>>> or if you'd prefer for it to go through the Rust tree.
>>>
>>> Please feel free to take it through modules-next, of course.
>>
>> Thanks for the confirmation. Queued now on modules-next for v7.3-rc1.
>
> I'm seeing this error on x86 with latest nightly rust compiler:
>
> rust/kernel.o: error: objtool: _RINvNtCs65fqAZscXCv_6kernel12module_param9set_paramaEB4_() falls through to next function _RINvNtCs65fqAZscXCv_6kernel12module_param9set_paramhEB4_()
> rust/kernel.o: error: objtool: _RINvNtCs65fqAZscXCv_6kernel12module_param9set_paramhEB4_() falls through to next function _RINvNtCs65fqAZscXCv_6kernel12module_param9set_paramiEB4_()
> rust/kernel.o: error: objtool: _RINvNtCs65fqAZscXCv_6kernel12module_param9set_paramiEB4_() falls through to next function _RINvNtCs65fqAZscXCv_6kernel12module_param9set_paramjEB4_()
> rust/kernel.o: error: objtool: _RINvNtCs65fqAZscXCv_6kernel12module_param9set_paramjEB4_() falls through to next function _RINvNtCs65fqAZscXCv_6kernel12module_param9set_paramlEB4_()
> rust/kernel.o: error: objtool: _RINvNtCs65fqAZscXCv_6kernel12module_param9set_paramlEB4_() falls through to next function _RINvNtCs65fqAZscXCv_6kernel12module_param9set_parammEB4_()
> rust/kernel.o: error: objtool: _RINvNtCs65fqAZscXCv_6kernel12module_param9set_parammEB4_() falls through to next function _RINvNtCs65fqAZscXCv_6kernel12module_param9set_paramsEB4_()
> rust/kernel.o: error: objtool: _RINvNtCs65fqAZscXCv_6kernel12module_param9set_paramsEB4_() falls through to next function _RINvNtCs65fqAZscXCv_6kernel12module_param9set_paramtEB4_()
> rust/kernel.o: error: objtool: _RINvNtCs65fqAZscXCv_6kernel12module_param9set_paramtEB4_() falls through to next function _RINvNtCs65fqAZscXCv_6kernel12module_param9set_paramyEB4_()
> rust/kernel.o: error: objtool: _RINvNtCs65fqAZscXCv_6kernel12module_param9set_paramyEB4_() falls through to next function _RINvNvNtCs65fqAZscXCv_6kernel6devres16register_foreign8callbackINtNtCsbtRzKR1rrUg_4core3pin3PinINtNtNtB6_5alloc4kbox3BoxNtNtNtB6_3pci3irq21IrqVectorRegistrationNtNtB1A_9allocator7KmallocEEEB6_()
> rust/kernel.o: error: objtool: _RNvMNtCs65fqAZscXCv_6kernel6deviceNtB2_6Device10get_device() falls through to next function _RNvMNtCs65fqAZscXCv_6kernel6devresINtB2_6DevresNtNtB4_3i2c12RegistrationE19devres_node_releaseB4_()
> rust/kernel.o: error: objtool: _RNvMNtCs65fqAZscXCv_6kernel6devresINtB2_6DevresNtNtB4_3i2c12RegistrationE19devres_node_releaseB4_() falls through to next function _RNvMNtCs65fqAZscXCv_6kernel6devresINtB2_6DevresNtNtB4_3i2c12RegistrationE21devres_node_free_nodeB4_()
>
> I understand this is probably due to a new function in Rust stdlib that
> is no_return, so probably not caused by this patch even if this is
> triggering it.
I can't immediately reproduce this issue. I tried with linux-next
(next-20260710), rustc 1.99.0-nightly (af3d95584 2026-07-09) and
x86_64_defconfig with Rust enabled. What am I missing? Could you send me
your config and the produced rust/kernel.o off-list?
--
Thanks,
Petr
^ permalink raw reply
* Re: [PATCH v4] module: Extend module_blacklist parameter to built-in modules
From: Sami Tolvanen @ 2026-07-10 15:42 UTC (permalink / raw)
To: Aaron Tomlin
Cc: arnd, mcgrof, petr.pavlu, da.gomez, peterz, akpm, mhiramat, neelx,
da.anzani, sean, chjohnst, steve, mproche, nick.lane, linux-arch,
linux-modules, linux-kernel
In-Reply-To: <20260708020007.55728-1-atomlin@atomlin.com>
Hi Aaron,
On Tue, Jul 7, 2026 at 7:00 PM Aaron Tomlin <atomlin@atomlin.com> wrote:
>
> +static const char *initcall_get_modname(initcall_t fn)
> +{
> + struct initcall_modname *p;
> + unsigned long addr = (unsigned long)dereference_function_descriptor(fn);
> +
> + if (system_state >= SYSTEM_FREEING_INITMEM)
> + return NULL;
> +
> + if (!is_kernel_text(addr) &&
> + !is_kernel_inittext(addr))
> + return NULL;
> +
> + for (p = __start_initcall_modnames; p < __stop_initcall_modnames; p++) {
> + if (dereference_function_descriptor(p->initcall_fn) ==
> + dereference_function_descriptor(fn))
> + return p->modname;
> + }
> + return NULL;
> +}
> +
> int __init_or_module do_one_initcall(initcall_t fn)
> {
> int count = preempt_count();
> char msgbuf[64];
> + const char *modname;
> int ret;
>
> + modname = initcall_get_modname(fn);
If I'm reading this correctly, this ends up scanning the
initcall_modnames list for every initcall. Have you measured whether
this has any boot time impact? Can we at least skip this scan if no
module denylist is provided?
Sami
^ permalink raw reply
* Re: [PATCH] module: validate string table section types
From: Aaron Tomlin @ 2026-07-10 15:37 UTC (permalink / raw)
To: Thiébaud Weksteen
Cc: Luis Chamberlain, Petr Pavlu, Daniel Gomez, Sami Tolvanen,
Siddharth Nayyar, linux-modules, linux-kernel
In-Reply-To: <20260708012107.1621513-1-tweek@google.com>
On Wed, Jul 08, 2026 at 11:21:07AM +1000, Thiébaud Weksteen wrote:
> In elf_validity_cache_sechdrs, section sizes and offsets are validated,
> unless the section type is SHT_NULL or SHT_NOBITS.
>
> Later, elf_validity_cache_secstrings and elf_validity_cache_index_str
> access the section name table (.shstrtab) and symbol string table
> (.strtab) headers without first ensuring that their types are
> SHT_STRTAB. If a section type is SHT_NULL or SHT_NOBITS, sh_offset has
> not been validated and may reference out-of-bounds memory when
> dereferenced in elf_validity_cache_secstrings or
> elf_validity_cache_strtab.
>
> Validate that both string section headers are of type SHT_STRTAB before
> caching them.
>
> Signed-off-by: Thiébaud Weksteen <tweek@google.com>
> ---
> kernel/module/main.c | 14 +++++++++++++-
> 1 file changed, 13 insertions(+), 1 deletion(-)
>
> diff --git a/kernel/module/main.c b/kernel/module/main.c
> index 46dd8d25a605..7cbc8f0e28c6 100644
> --- a/kernel/module/main.c
> +++ b/kernel/module/main.c
> @@ -2011,6 +2011,7 @@ static int elf_validity_cache_sechdrs(struct load_info *info)
> * Specifically checks:
> *
> * * Section name table index is inbounds of section headers
> + * * Section name table type is SHT_STRTAB
> * * Section name table is not empty
> * * Section name table is NUL terminated
> * * All section name offsets are inbounds of the section
> @@ -2038,6 +2039,11 @@ static int elf_validity_cache_secstrings(struct load_info *info)
>
> strhdr = &info->sechdrs[info->hdr->e_shstrndx];
>
> + if (strhdr->sh_type != SHT_STRTAB) {
> + pr_err("Invalid ELF section name table type: %u\n", strhdr->sh_type);
> + return -ENOEXEC;
> + }
> +
> /*
> * The section name table must be NUL-terminated, as required
> * by the spec. This makes strcmp and pr_* calls that access
> @@ -2204,7 +2210,7 @@ static int elf_validity_cache_index_sym(struct load_info *info)
> * Must have &load_info->index.sym populated.
> *
> * Looks at the symbol table's associated string table, makes sure it is
> - * in-bounds, and caches it.
> + * in-bounds and of type SHT_STRTAB, and caches it.
> *
> * Return: %0 if valid, %-ENOEXEC on failure.
> */
> @@ -2218,6 +2224,12 @@ static int elf_validity_cache_index_str(struct load_info *info)
> return -ENOEXEC;
> }
>
> + if (info->sechdrs[str_idx].sh_type != SHT_STRTAB) {
> + pr_err("Invalid ELF symbol string table type: %u\n",
> + info->sechdrs[str_idx].sh_type);
> + return -ENOEXEC;
> + }
> +
> info->index.str = str_idx;
> return 0;
> }
> --
> 2.55.0.795.g602f6c329a-goog
>
Reviewed-by: Aaron Tomlin <atomlin@atomlin.com>
--
Aaron Tomlin
^ permalink raw reply
* Re: [PATCH v2] rust: module_param: return value by copy from `value`
From: Miguel Ojeda @ 2026-07-10 14:41 UTC (permalink / raw)
To: Alice Ryhl
Cc: Petr Pavlu, Andreas Hindborg, Miguel Ojeda, Luis Chamberlain,
Daniel Gomez, Sami Tolvanen, Aaron Tomlin, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Trevor Gross,
Danilo Krummrich, linux-modules, linux-kernel, rust-for-linux
In-Reply-To: <alEBInX9gD1M5NAr@google.com>
On Fri, Jul 10, 2026 at 4:26 PM Alice Ryhl <aliceryhl@google.com> wrote:
>
> I understand this is probably due to a new function in Rust stdlib that
> is no_return, so probably not caused by this patch even if this is
> triggering it.
I can take a look and perhaps push a patch via `rust-fixes`, which
would make it fixed by the time this one lands in the merge window.
Cheers,
Miguel
^ permalink raw reply
* Re: [PATCH v2] rust: module_param: return value by copy from `value`
From: Alice Ryhl @ 2026-07-10 14:26 UTC (permalink / raw)
To: Petr Pavlu
Cc: Miguel Ojeda, Andreas Hindborg, Miguel Ojeda, Luis Chamberlain,
Daniel Gomez, Sami Tolvanen, Aaron Tomlin, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Trevor Gross,
Danilo Krummrich, linux-modules, linux-kernel, rust-for-linux
In-Reply-To: <30e500ce-e9e6-43cc-bbdf-ded7c0ae56cd@suse.com>
On Fri, Jul 10, 2026 at 10:13:53AM +0200, Petr Pavlu wrote:
> On 7/9/26 4:13 PM, Miguel Ojeda wrote:
> > On Thu, Jul 9, 2026 at 1:48 PM Petr Pavlu <petr.pavlu@suse.com> wrote:
> >>
> >> @Miguel, please let me know if I should take this patch on modules-next,
> >> or if you'd prefer for it to go through the Rust tree.
> >
> > Please feel free to take it through modules-next, of course.
>
> Thanks for the confirmation. Queued now on modules-next for v7.3-rc1.
I'm seeing this error on x86 with latest nightly rust compiler:
rust/kernel.o: error: objtool: _RINvNtCs65fqAZscXCv_6kernel12module_param9set_paramaEB4_() falls through to next function _RINvNtCs65fqAZscXCv_6kernel12module_param9set_paramhEB4_()
rust/kernel.o: error: objtool: _RINvNtCs65fqAZscXCv_6kernel12module_param9set_paramhEB4_() falls through to next function _RINvNtCs65fqAZscXCv_6kernel12module_param9set_paramiEB4_()
rust/kernel.o: error: objtool: _RINvNtCs65fqAZscXCv_6kernel12module_param9set_paramiEB4_() falls through to next function _RINvNtCs65fqAZscXCv_6kernel12module_param9set_paramjEB4_()
rust/kernel.o: error: objtool: _RINvNtCs65fqAZscXCv_6kernel12module_param9set_paramjEB4_() falls through to next function _RINvNtCs65fqAZscXCv_6kernel12module_param9set_paramlEB4_()
rust/kernel.o: error: objtool: _RINvNtCs65fqAZscXCv_6kernel12module_param9set_paramlEB4_() falls through to next function _RINvNtCs65fqAZscXCv_6kernel12module_param9set_parammEB4_()
rust/kernel.o: error: objtool: _RINvNtCs65fqAZscXCv_6kernel12module_param9set_parammEB4_() falls through to next function _RINvNtCs65fqAZscXCv_6kernel12module_param9set_paramsEB4_()
rust/kernel.o: error: objtool: _RINvNtCs65fqAZscXCv_6kernel12module_param9set_paramsEB4_() falls through to next function _RINvNtCs65fqAZscXCv_6kernel12module_param9set_paramtEB4_()
rust/kernel.o: error: objtool: _RINvNtCs65fqAZscXCv_6kernel12module_param9set_paramtEB4_() falls through to next function _RINvNtCs65fqAZscXCv_6kernel12module_param9set_paramyEB4_()
rust/kernel.o: error: objtool: _RINvNtCs65fqAZscXCv_6kernel12module_param9set_paramyEB4_() falls through to next function _RINvNvNtCs65fqAZscXCv_6kernel6devres16register_foreign8callbackINtNtCsbtRzKR1rrUg_4core3pin3PinINtNtNtB6_5alloc4kbox3BoxNtNtNtB6_3pci3irq21IrqVectorRegistrationNtNtB1A_9allocator7KmallocEEEB6_()
rust/kernel.o: error: objtool: _RNvMNtCs65fqAZscXCv_6kernel6deviceNtB2_6Device10get_device() falls through to next function _RNvMNtCs65fqAZscXCv_6kernel6devresINtB2_6DevresNtNtB4_3i2c12RegistrationE19devres_node_releaseB4_()
rust/kernel.o: error: objtool: _RNvMNtCs65fqAZscXCv_6kernel6devresINtB2_6DevresNtNtB4_3i2c12RegistrationE19devres_node_releaseB4_() falls through to next function _RNvMNtCs65fqAZscXCv_6kernel6devresINtB2_6DevresNtNtB4_3i2c12RegistrationE21devres_node_free_nodeB4_()
I understand this is probably due to a new function in Rust stdlib that
is no_return, so probably not caused by this patch even if this is
triggering it.
Alice
^ permalink raw reply
* Re: [PATCH 1/1] rust: module_param: support bool parameters
From: Petr Pavlu @ 2026-07-10 14:10 UTC (permalink / raw)
To: Wenzhao Liao
Cc: mcgrof, da.gomez, samitolvanen, ojeda, linux-modules,
rust-for-linux, atomlin, boqun, gary, bjorn3_gh, lossin,
a.hindborg, aliceryhl, tmgross, dakr, linux-kernel, Greg KH
In-Reply-To: <20260411130254.3510128-2-wenzhaoliao@ruc.edu.cn>
On 4/11/26 3:02 PM, Wenzhao Liao wrote:
> Add support for parsing boolean module parameters in the Rust
> module! macro.
>
> Currently, only integer types are supported by the `module_param!`
> macros. This patch implements the `ModuleParam` trait for `bool`
> by delegating the string parsing to the existing C implementation
> via `kstrtobool_bytes()`. It also wires up `PARAM_OPS_BOOL` so that
> the Rust parameter system correctly links to the C `param_ops_bool`
> structure.
>
> For demonstration and verification, a boolean parameter is added
> to `samples/rust/rust_minimal.rs`.
>
> Assisted-by: Codex:GPT-5
> Signed-off-by: Wenzhao Liao <wenzhaoliao@ruc.edu.cn>
I've queued the patch on modules-next for v7.3-rc1, with the following
minor changes:
* Added a short description of the motivation to the commit message:
Support for boolean parameters will initially be used by the Rust
null block driver [1].
Link: https://lore.kernel.org/all/20260609-rnull-v6-19-rc5-send-v2-4-82c7404542e2@kernel.org/ [1]
* Rebased the patch on top of '[PATCH v2] rust: module_param: return
value by copy from `value`' [1], which required removing the
dereference from '*module_parameters::test_bool_parameter.value()' in
samples/rust/rust_minimal.rs.
[1] https://lore.kernel.org/linux-modules/20260601-modules-value-ref-v2-1-12ebbf0510c9@kernel.org/
--
Thanks,
Petr
^ permalink raw reply
* Re: [PATCH 2/2] module: Bring includes in linux/kmod.h up to date
From: Aaron Tomlin @ 2026-07-10 13:57 UTC (permalink / raw)
To: Petr Pavlu
Cc: Tony Luck, Borislav Petkov, Thomas Gleixner, Ingo Molnar,
Dave Hansen, x86, H. Peter Anvin, Philipp Reisner, Lars Ellenberg,
Christoph Böhmwalder, Jens Axboe, Johan Hovold, Alex Elder,
Greg Kroah-Hartman, Rafael J. Wysocki, Michal Januszewski,
Helge Deller, Alexander Viro, Christian Brauner, Jan Kara,
Trond Myklebust, Anna Schumaker, Chuck Lever, Jeff Layton,
NeilBrown, Olga Kornievskaia, Dai Ngo, Tom Talpey, Mark Fasheh,
Joel Becker, Joseph Qi, Tejun Heo, Johannes Weiner,
Michal Koutný, Luis Chamberlain, Daniel Gomez, Sami Tolvanen,
Pavel Machek, Len Brown, Andrew Morton, Danilo Krummrich,
Nikolay Aleksandrov, Ido Schimmel, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Simon Horman, David Howells,
Jarkko Sakkinen, Paul Moore, James Morris, Serge E. Hallyn,
Kentaro Takeda, Tetsuo Handa, linux-edac, linux-kernel, drbd-dev,
linux-block, greybus-dev, linuxppc-dev, linux-acpi, linux-fbdev,
dri-devel, linux-fsdevel, linux-nfs, ocfs2-devel, cgroups,
linux-modules, linux-pm, driver-core, bridge, netdev, keyrings,
linux-security-module
In-Reply-To: <20260708154510.6794-3-petr.pavlu@suse.com>
On Wed, Jul 08, 2026 at 05:44:30PM +0200, Petr Pavlu wrote:
> Including linux/kmod.h alone results in 1.5 MB of preprocessed output, even
> though it provides only a few functions and macros.
>
> The header currently depends on:
>
> * __printf() -> linux/compiler_attributes.h,
> * ENOSYS -> linux/errno.h,
> * bool -> linux/types.h.
>
> Include only these files, reducing the preprocessed output to 10 kB.
>
> Signed-off-by: Petr Pavlu <petr.pavlu@suse.com>
> ---
> include/linux/kmod.h | 12 ++----------
> 1 file changed, 2 insertions(+), 10 deletions(-)
>
> diff --git a/include/linux/kmod.h b/include/linux/kmod.h
> index 9a07c3215389..b9474a62a568 100644
> --- a/include/linux/kmod.h
> +++ b/include/linux/kmod.h
> @@ -2,17 +2,9 @@
> #ifndef __LINUX_KMOD_H__
> #define __LINUX_KMOD_H__
>
> -/*
> - * include/linux/kmod.h
> - */
> -
> -#include <linux/umh.h>
> -#include <linux/gfp.h>
> -#include <linux/stddef.h>
> +#include <linux/compiler_attributes.h>
> #include <linux/errno.h>
> -#include <linux/compiler.h>
> -#include <linux/workqueue.h>
> -#include <linux/sysctl.h>
> +#include <linux/types.h>
>
> #ifdef CONFIG_MODULES
> /* modprobe exit status on success, -ve on error. Return value
> --
> 2.54.0
>
LGTM. Thank you.
Reviewed-by: Aaron Tomlin <atomlin@atomlin.com>
--
Aaron Tomlin
^ permalink raw reply
* Re: [PATCH 4/4] module: Limit ELF includes in linux/module.h to uapi/linux/elf.h
From: Aaron Tomlin @ 2026-07-10 13:56 UTC (permalink / raw)
To: Petr Pavlu
Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
Heiko Carstens, Vasily Gorbik, Alexander Gordeev, Peter Huewe,
Jarkko Sakkinen, Luis Chamberlain, Daniel Gomez, Sami Tolvanen,
Xin Li, H. Peter Anvin, Andy Lutomirski, Christian Borntraeger,
Sven Schnelle, Janosch Frank, Claudio Imbrenda, David Hildenbrand,
Jason Gunthorpe, linux-s390, kvm, linux-integrity, linux-modules,
linux-kernel
In-Reply-To: <20260707153819.4172541-5-petr.pavlu@suse.com>
[-- Attachment #1: Type: text/plain, Size: 1631 bytes --]
On Tue, Jul 07, 2026 at 05:37:07PM +0200, Petr Pavlu wrote:
> The linux/module.h header is included by every *.mod.c file to provide
> `struct module` and other related definitions. This makes it a "hot"
> header, so it should avoid pulling in unnecessary definitions.
>
> The header currently includes linux/elf.h but this is largely unnecessary
> because it requires only the base ELF structures, such as Elf_Ehdr and
> Elf_Sym, which are declared in uapi/linux/elf.h.
>
> Replace the linux/elf.h include with uapi/linux/elf.h. On x86_64_defconfig,
> including linux/module.h alone results in 2.2 MB of preprocessed data. This
> change reduces that to 1.5 MB.
>
> Signed-off-by: Petr Pavlu <petr.pavlu@suse.com>
> ---
> include/linux/module.h | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/include/linux/module.h b/include/linux/module.h
> index 7566815fabbe..7661faadbedd 100644
> --- a/include/linux/module.h
> +++ b/include/linux/module.h
> @@ -17,7 +17,6 @@
> #include <linux/cleanup.h>
> #include <linux/kmod.h>
> #include <linux/init.h>
> -#include <linux/elf.h>
> #include <linux/stringify.h>
> #include <linux/kobject.h>
> #include <linux/moduleparam.h>
> @@ -29,8 +28,9 @@
> #include <linux/srcu.h>
> #include <linux/static_call_types.h>
> #include <linux/dynamic_debug.h>
> -
> #include <linux/percpu.h>
> +
> +#include <uapi/linux/elf.h>
> #include <asm/module.h>
>
> #define MODULE_NAME_LEN __MODULE_NAME_LEN
> --
> 2.54.0
>
Thank you Petr. LGTM.
Reviewed-by: Aaron Tomlin <atomlin@atomlin.com>
--
Aaron Tomlin
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH v2 1/2] module/kallsyms: fix nextval for data symbol lookup
From: Petr Pavlu @ 2026-07-10 12:36 UTC (permalink / raw)
To: Stanislaw Gruszka
Cc: linux-modules, Sami Tolvanen, Luis Chamberlain, 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>
On 3/27/26 12:00 PM, Stanislaw Gruszka wrote:
> 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>
I've queued this first patch on modules-fixes.
--
Thanks,
Petr
^ permalink raw reply
* Re: [PATCH v7 0/4] kallsyms: embed source file:line info in kernel stack traces
From: Geert Uytterhoeven @ 2026-07-10 10:04 UTC (permalink / raw)
To: Sasha Levin
Cc: Andrew Morton, Masahiro Yamada, Luis Chamberlain, Linus Torvalds,
Richard Weinberger, Juergen Gross, James Bottomley,
Jonathan Corbet, Nathan Chancellor, Nicolas Schier, Petr Pavlu,
Daniel Gomez, Greg KH, Petr Mladek, Steven Rostedt, Kees Cook,
Peter Zijlstra, Thorsten Leemhuis, Vlastimil Babka, Helge Deller,
Randy Dunlap, Laurent Pinchart, Vivian Wang, Zhen Lei,
Sami Tolvanen, linux-kernel, linux-kbuild, linux-modules,
linux-doc
In-Reply-To: <20260709163833.3851179-1-sashal@kernel.org>
Hi Sasha,
On Thu, 9 Jul 2026 at 18:38, Sasha Levin <sashal@kernel.org> wrote:
> Changes since v6
> ================
>
> - Address Sashiko AI review comments.
What does that mean?
Please list the changes, so reviewers know what to look at.
Thanks!
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply
* Re: [PATCH] module: validate string table section types
From: Markus Elfring @ 2026-07-10 8:23 UTC (permalink / raw)
To: Thiébaud Weksteen, linux-modules, Daniel Gomez,
Luis Chamberlain, Petr Pavlu, Sami Tolvanen
Cc: LKML, Aaron Tomlin, Siddharth Nayyar
In-Reply-To: <20260708012107.1621513-1-tweek@google.com>
…
> Validate that both string section headers are of type SHT_STRTAB before
> caching them.
How do you think about to add any tags (like “Fixes” and “Cc”) accordingly?
Regards,
Markus
^ permalink raw reply
* Re: [PATCH v2] rust: module_param: return value by copy from `value`
From: Petr Pavlu @ 2026-07-10 8:13 UTC (permalink / raw)
To: Miguel Ojeda
Cc: Andreas Hindborg, Miguel Ojeda, Luis Chamberlain, Daniel Gomez,
Sami Tolvanen, Aaron Tomlin, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Alice Ryhl, Trevor Gross,
Danilo Krummrich, linux-modules, linux-kernel, rust-for-linux
In-Reply-To: <CANiq72m3w7L5CmAsFvOpSh_jNk4jL8+A4vz8GEp+E8FY=TJcRA@mail.gmail.com>
On 7/9/26 4:13 PM, Miguel Ojeda wrote:
> On Thu, Jul 9, 2026 at 1:48 PM Petr Pavlu <petr.pavlu@suse.com> wrote:
>>
>> @Miguel, please let me know if I should take this patch on modules-next,
>> or if you'd prefer for it to go through the Rust tree.
>
> Please feel free to take it through modules-next, of course.
Thanks for the confirmation. Queued now on modules-next for v7.3-rc1.
-- Petr
^ permalink raw reply
* Re: [PATCH v2] module: procfs: fix signed integer overflow in module_total_size()
From: Petr Pavlu @ 2026-07-10 7:36 UTC (permalink / raw)
To: Sami Tolvanen
Cc: Naveen Kumar Chaudhary, mcgrof, da.gomez, atomlin, linux-modules
In-Reply-To: <CABCJKuczwBmtKXiyzh31Efwbt6VfwUpStLPjXkUFNHwwSPQA5g@mail.gmail.com>
On 7/9/26 5:25 PM, Sami Tolvanen wrote:
> On Thu, Jul 9, 2026 at 1:45 AM Petr Pavlu <petr.pavlu@suse.com> wrote:
>>
>> On 6/7/26 6:18 AM, Naveen Kumar Chaudhary wrote:
>>> module_total_size() returns unsigned int but uses a signed int
>>> accumulator. While the result is numerically correct, the type
>>> mismatch is misleading.
>>>
>>> Change the accumulator to unsigned int to match the return type.
>>>
>>> Signed-off-by: Naveen Kumar Chaudhary <naveen.osdev@gmail.com>
>>
>> Queued on modules-next for v7.3-rc1.
>
> Note that the commit title still claims to fix a signed integer
> overflow, which isn't accurate. Otherwise LGTM.
Thanks for noticing this. I tweaked the title to:
module: procfs: use matching type for accumulator in module_total_size()
-- Petr
^ permalink raw reply
* Re: [PATCH v19 04/40] dept: tie to lockdep and IRQ tracing
From: Byungchul Park @ 2026-07-10 6:54 UTC (permalink / raw)
To: sashiko-reviews
Cc: linux-rt-devel, linux-kernel, dlemoal, cassel, tytso, Chuck Lever,
Anna Schumaker, linux-modules, Jeff Layton, ojeda, dri-devel,
linux-ide, kernel_team
In-Reply-To: <20260707073322.A9C851F000E9@smtp.kernel.org>
On Tue, Jul 07, 2026 at 07:33:22AM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
> - [Critical] dept_key_destroy() synchronously recycles dept_class objects to the lockless pool before the RCU grace period completes, causing UAF and list traversal corruption for concurrent lockless readers.
> - [High] rwsem_acquire_read and rwsem_acquire incorrectly use ldt_lock(), breaking reader-writer dependency tracking and causing false-positive deadlocks.
> - [High] ldt_init() is called before the `key` parameter is validated, causing DEPT to dereference a garbage pointer if `key` is NULL.
> - [Medium] dept_key_init() and dept_key_destroy() are called on static keys before validation, erroneously disabling DEPT or corrupting tracking state.
> - [Low] STATIC_LOCKDEP_MAP_INIT creates double-quoted strings for map names due to `#n` stringification in DEPT_MAP_INITIALIZER.
> --
>
> commit 6c1a95ee3182a2a82a1d61196d75acd387d7f3e3
> Author: Byungchul Park <byungchul@sk.com>
>
> dept: tie to lockdep and IRQ tracing
>
> This patch integrates DEPT (Dependency Tracker) into the lockdep framework
> and IRQ tracing paths. It modifies various locking primitives and lockdep
> interfaces to initialize and track DEPT maps alongside lockdep maps.
>
> > diff --git a/include/linux/lockdep.h b/include/linux/lockdep.h
> > index 621566345406d..60b17a2ff555f 100644
> > --- a/include/linux/lockdep.h
> > +++ b/include/linux/lockdep.h
> [ ... ]
> > @@ -428,7 +431,8 @@ enum xhlock_context_t {
> > * Note that _name must not be NULL.
> > */
> > #define STATIC_LOCKDEP_MAP_INIT(_name, _key) \
> > - { .name = (_name), .key = (void *)(_key), }
> > + { .name = (_name), .key = (void *)(_key), \
> > + .dmap = DEPT_MAP_INITIALIZER(_name, _key) }
>
> [Severity: Low]
> Since _name is usually passed as a string literal to
> STATIC_LOCKDEP_MAP_INIT, does passing it to DEPT_MAP_INITIALIZER, which
> uses the stringification operator on its first argument, result in
> double-quoted strings for map names in DEPT's debug output?
I will check it. Thanks.
> [ ... ]
> > -#define rwsem_acquire(l, s, t, i) lock_acquire_exclusive(l, s, t, NULL, i)
> > -#define rwsem_acquire_nest(l, s, t, n, i) lock_acquire_exclusive(l, s, t, n, i)
> > -#define rwsem_acquire_read(l, s, t, i) lock_acquire_shared(l, s, t, NULL, i)
> > -#define rwsem_release(l, i) lock_release(l, i)
> [ ... ]
> > +#define rwsem_acquire(l, s, t, i) \
> > +do { \
> > + ldt_lock(&(l)->dmap, s, t, NULL, i); \
> > + lock_acquire_exclusive(l, s, t, NULL, i); \
> > +} while (0)
> > +#define rwsem_acquire_nest(l, s, t, n, i) \
> > +do { \
> > + ldt_lock(&(l)->dmap, s, t, n, i); \
> > + lock_acquire_exclusive(l, s, t, n, i); \
> > +} while (0)
> > +#define rwsem_acquire_read(l, s, t, i) \
> > +do { \
> > + ldt_lock(&(l)->dmap, s, t, NULL, i); \
> > + lock_acquire_shared(l, s, t, NULL, i); \
> > +} while (0)
>
> [Severity: High]
> Is it intentional that rwsem_acquire_read() uses ldt_lock() instead of
> ldt_rlock(), and rwsem_acquire() uses ldt_lock() instead of ldt_wlock()?
It's intended. rwsem is a queued lock for fairness. So it should be
considered as a normal lock to detect a potential deadlock.
> This appears to mistakenly model a shared lock as an exclusive lock. Does
> this cause DEPT to incorrectly model concurrent read locks as mutually
> exclusive, leading to false-positive circular dependency (deadlock) reports
> when concurrent readers take an rwsem and interact with other locks?
As I told you, rwsem is a queued lock. So this is not true.
> > diff --git a/kernel/locking/lockdep.c b/kernel/locking/lockdep.c
> > index 2d4c5bab5af88..dc97f2753ef81 100644
> > --- a/kernel/locking/lockdep.c
> > +++ b/kernel/locking/lockdep.c
> > @@ -1224,6 +1224,8 @@ void lockdep_register_key(struct lock_class_key *key)
> > struct hlist_head *hash_head;
> > struct lock_class_key *k;
> > unsigned long flags;
> >
> > + dept_key_init(&key->dkey);
> > +
> > if (WARN_ON_ONCE(static_obj(key)))
> > return;
>
> [Severity: Medium]
> In lockdep_register_key(), dept_key_init() is called before the
> static_obj() check. If a static key is improperly passed, lockdep gracefully
> rejects it by warning and returning early.
>
> However, processing a static key in dept_key_init() triggers a DEPT_STOP
> abort. Should the DEPT initialization occur after validating the key?
I will check it.
> [ ... ]
> > @@ -4940,6 +4950,8 @@ void lockdep_init_map_type(struct lockdep_map *lock, const char *name,
> > {
> > int i;
> >
> > + ldt_init(&lock->dmap, &key->dkey, subclass, name);
> > +
> > for (i = 0; i < NR_LOCKDEP_CACHING_CLASSES; i++)
> > lock->class_cache[i] = NULL;
>
> [Severity: High]
> In lockdep_init_map_type(), ldt_init() is called before the parameter
> validation checks for a NULL key further down in the function.
>
> If key is NULL, &key->dkey yields a non-NULL offset pointer. Does DEPT accept
> this garbage pointer in valid_key(), causing a kernel panic when
> check_new_class() dereferences it?
I will check it.
> [ ... ]
> > @@ -6588,6 +6608,8 @@ void lockdep_unregister_key(struct lock_class_key *key)
> > bool found = false;
> > bool need_callback = false;
> >
> > + dept_key_destroy(&key->dkey);
> > +
> > might_sleep();
> >
> > if (WARN_ON_ONCE(static_obj(key)))
> > return;
>
> [Severity: Medium]
> Similarly to lockdep_register_key(), dept_key_destroy() is called here
> unconditionally before the static_obj() check. If a driver incorrectly calls
> dynamic lock destruction on a static key, it eagerly destroys its DEPT state
> while lockdep retains it. Should this also be moved after the static_obj()
> check?
Sure. I will.
> [Severity: Critical]
> Additionally, in dept_key_destroy() called here, the function removes a class
> from the RCU hash table and drops its refcount to 0 via del_class(). This
> recycles the object to the DEPT pool before synchronize_rcu() is called.
No.
del_class() never drops refcount to 0. That happens only through:
disconnect_class()->call_rcu(del_dep_rcu)
// after the grace period
del_dep_rcu()->del_dep()->put_ecxt()->put_class()
del_dep_rcu()->del_dep()->put_wait()->put_class()
So it's safe.
> Can a concurrent lockless reader in hash_lookup_class() traversing the same
> bucket via hlist_for_each_entry_rcu() access this recycled object, leading to
> a use-after-free and hash list corruption?
No.
Byungchul
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20260706061928.66713-1-byungchul@sk.com?part=4
^ permalink raw reply
* Re: [PATCH v19 13/40] dept: apply sdt_might_sleep_{start, end}() to hashed-waitqueue wait
From: Byungchul Park @ 2026-07-10 6:42 UTC (permalink / raw)
To: sashiko-reviews
Cc: Chuck Lever, tytso, Anna Schumaker, linux-kernel, dlemoal,
dri-devel, Jeff Layton, linux-rt-devel, ojeda, linux-modules,
cassel, linux-ide, kernel_team
In-Reply-To: <20260707073317.37C831F000E9@smtp.kernel.org>
On Tue, Jul 07, 2026 at 07:33:16AM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> - [Medium] The `sdt_might_sleep_end()` macro is bypassed when `___wait_var_event` is interrupted, leaving the DEPT tracker in an inconsistent state.
> - [Medium] The `sdt_might_sleep_end()` macro is bypassed when `___wait_event` is interrupted, leaving the DEPT tracker in an inconsistent state.
> --
>
> commit 0bd2d42866fa6b852ee5317edcaf6890a90c3c65
> Author: Byungchul Park <byungchul@sk.com>
>
> dept: apply sdt_might_sleep_{start,end}() to hashed-waitqueue wait
>
> Make dept able to track dependencies by hashed-waitqueue waits.
>
> > diff --git a/include/linux/wait_bit.h b/include/linux/wait_bit.h
> > index 9e29d79fc790a..179a616ad245a 100644
> > --- a/include/linux/wait_bit.h
> > +++ b/include/linux/wait_bit.h
> [ ... ]
> > @@ -257,6 +258,7 @@ extern wait_queue_head_t *__var_waitqueue(void *p);
> > struct wait_bit_queue_entry __wbq_entry; \
> > long __ret = ret; /* explicit shadow */ \
> > \
> > + sdt_might_sleep_start(NULL); \
> > init_wait_var_entry(&__wbq_entry, var, \
> > exclusive ? WQ_FLAG_EXCLUSIVE : 0); \
> > for (;;) { \
> > @@ -274,6 +276,7 @@ extern wait_queue_head_t *__var_waitqueue(void *p);
> > cmd; \
> > } \
> > finish_wait(__wq_head, &__wbq_entry.wq_entry); \
> > + sdt_might_sleep_end(); \
> > __out: __ret; \
> > })
>
> [Severity: Medium]
> If this wait is interrupted by a signal, the loop executes a goto __out
> which bypasses sdt_might_sleep_end(). Does this leave the dept tracker
> in an inconsistent state where the task's stage remains permanently armed?
>
> This could cause false positive dependency tracking splats on subsequent
> waits, or fail to record new dependencies because the tracker is blinded by
> the stale stage.
>
> I noticed the exact same issue is present in ___wait_event():
>
> include/linux/wait.h:___wait_event() {
> ...
> if (___wait_is_interruptible(state) && __int) {
> __ret = __int;
> goto __out;
> }
> ...
> sdt_might_sleep_end();
> __out: __ret;
> })
I will fix it.
> [Severity: Medium]
> Does ___wait_event() also skip sdt_might_sleep_end() when interrupted?
> By missing the paired cleanup call, the task's stage could remain
> permanently armed here as well.
>
> Would it make sense to move the __out label above sdt_might_sleep_end(),
> or otherwise ensure the state is cleaned up on the interrupted paths for
> both of these macros?
Right. I will fix it.
Byungchul
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20260706061928.66713-1-byungchul@sk.com?part=13
^ permalink raw reply
* Re: [PATCH v19 19/40] dept: apply timeout consideration to hashed-waitqueue wait
From: Byungchul Park @ 2026-07-10 6:41 UTC (permalink / raw)
To: sashiko-reviews
Cc: ojeda, linux-ide, linux-kernel, Anna Schumaker, Chuck Lever,
dlemoal, linux-rt-devel, Jeff Layton, tytso, cassel,
linux-modules, dri-devel, kernel_team
In-Reply-To: <20260707073337.097CA1F000E9@smtp.kernel.org>
On Tue, Jul 07, 2026 at 07:33:36AM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] Missing `sdt_might_sleep_end()` cleanup in the error path of wait macros (e.g., `___wait_var_event`, `___wait_event`, `___swait_event`) bypasses `dept_clean_stage()`, leaking the staged wait event state and corrupting DEPT tracking.
> --
>
> commit cf04720896ab037609320947c88e8faa57dbddd7
> Author: Byungchul Park <byungchul@sk.com>
>
> dept: apply timeout consideration to hashed-waitqueue wait
>
> Now that CONFIG_DEPT_AGGRESSIVE_TIMEOUT_WAIT was introduced, apply the
> consideration to hashed-waitqueue wait, assuming an input 'ret' in
> ___wait_var_event() macro is used as a timeout value.
>
> > diff --git a/include/linux/wait_bit.h b/include/linux/wait_bit.h
> > index 179a616ad245a..9885ac4e1ded5 100644
> > --- a/include/linux/wait_bit.h
> > +++ b/include/linux/wait_bit.h
> > @@ -258,7 +258,7 @@ extern wait_queue_head_t *__var_waitqueue(void *p);
> > struct wait_bit_queue_entry __wbq_entry; \
> > long __ret = ret; /* explicit shadow */ \
> > \
> > - sdt_might_sleep_start(NULL); \
> > + sdt_might_sleep_start_timeout(NULL, __ret); \
>
> [Severity: High]
> Does this error path properly clean up the dept wait state?
>
> Lower in the ___wait_var_event() macro, if a signal is received it jumps to
> __out:
>
> if (___wait_is_interruptible(state) && __int) { \
> __ret = __int; \
> goto __out; \
> } \
>
> ...
>
> sdt_might_sleep_end(); \
> __out: __ret; \
>
> Because the goto __out branch skips sdt_might_sleep_end(), does it leave a
> dangling stage context in the dept subsystem's tracking graph?
>
> If the stage context remains populated, could a subsequent wait on the same
> task be incorrectly seen as a nested stage, potentially corrupting dept
> tracking or leading to false deadlock reports?
I will fix it.
> (Note: A similar goto __out pattern skipping sdt_might_sleep_end() also
> exists in ___wait_event() in include/linux/wait.h and ___swait_event() in
> include/linux/swait.h).
I will fix it.
Byungchul
>
> > init_wait_var_entry(&__wbq_entry, var, \
> > exclusive ? WQ_FLAG_EXCLUSIVE : 0); \
> > for (;;) { \
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20260706061928.66713-1-byungchul@sk.com?part=19
^ permalink raw reply
* Re: [PATCH v19 07/40] dept: distinguish each work from another
From: Byungchul Park @ 2026-07-10 6:40 UTC (permalink / raw)
To: sashiko-reviews
Cc: linux-rt-devel, cassel, linux-ide, tytso, linux-kernel,
Anna Schumaker, linux-modules, Chuck Lever, ojeda, dlemoal,
dri-devel, Jeff Layton, kernel_team
In-Reply-To: <20260707073334.0E5A61F000E9@smtp.kernel.org>
On Tue, Jul 07, 2026 at 07:33:33AM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] `dept_update_cxt()` hardcodes the process context ID, failing to track bottom-half (BH) work items in softirq context and corrupting the context of preempted tasks.
> --
>
> commit b8dda7d95e5377770d28c6712cf8fe7fafa98ade
> Author: Byungchul Park <byungchul@sk.com>
>
> dept: distinguish each work from another
>
> This commit updates DEPT to assign a different context ID to each work item.
> Since workqueues provide their own concurrency control, wait operations in a
> work item do not prevent events in other works from executing.
>
> > diff --git a/kernel/workqueue.c b/kernel/workqueue.c
> > index c6ea96d5b7167..4a4075d0697c7 100644
> > --- a/kernel/workqueue.c
> > +++ b/kernel/workqueue.c
> [ ... ]
> > @@ -3204,6 +3205,8 @@ __acquires(&pool->lock)
> >
> > lockdep_copy_map(&lockdep_map, &work->lockdep_map);
> > #endif
> > + dept_update_cxt();
> > +
>
> [Severity: Medium]
> Will this unconditionally modify the process context ID, even when tracking
> bottom-half (BH) work items running in softirq context?
Am I missing something? AFAIK, workqueue's workers run in process
context, not softirq context.
> When a BH workqueue executes multiple work items in a single softirq batch,
> process_one_work() executes them and calls dept_update_cxt(). However,
> dept_update_cxt() is implemented as:
It's becasue its not racing with other contexts.
> kernel/dependency/dept.c:dept_update_cxt() {
> struct dept_task *dt = dept_task();
> dt->cxt_id[DEPT_CXT_PROCESS] += 1UL << DEPT_CXTS_NR;
> }
>
> This hardcodes the increment to DEPT_CXT_PROCESS rather than checking
> cur_cxt() to increment DEPT_CXT_SIRQ. Does this cause DEPT to fail in
> distinguishing sequential BH work items and falsely report dependencies?
Are BH works running in softirq context? I will check it.
> Also, can this corrupt the continuous dependency tracking context of
> whatever task was preempted by the softirq?
Process context's cxt_id is referred only in the process context, and
irq context's cxt_id is referred only in the irq context. So no problem.
Byungchul
> > /* ensure we're on the correct CPU */
> > WARN_ON_ONCE(!(pool->flags & POOL_DISASSOCIATED) &&
> > raw_smp_processor_id() != pool->cpu);
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20260706061928.66713-1-byungchul@sk.com?part=7
^ permalink raw reply
* Re: [PATCH v19 28/40] dept: assign unique dept_key to each distinct dma fence caller
From: Byungchul Park @ 2026-07-10 6:24 UTC (permalink / raw)
To: sashiko-reviews
Cc: Chuck Lever, dlemoal, Jeff Layton, linux-rt-devel, linux-modules,
dri-devel, linux-ide, tytso, cassel, ojeda, Anna Schumaker,
linux-kernel, kernel_team
In-Reply-To: <20260707073344.7392B1F000E9@smtp.kernel.org>
On Tue, Jul 07, 2026 at 07:33:44AM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> - [Medium] The newly introduced statement expression macros for DMA fence waiting evaluate their timeout argument (`t`) multiple times.
> - [Medium] Local variable shadowing in the statement expression macros due to the unhygienic `__ret` variable declaration.
> --
>
> commit 4d4caad8d173eededcba1493c060f5ddecbd6b70
> Author: Byungchul Park <byungchul@sk.com>
>
> dept: assign unique dept_key to each distinct dma fence caller
>
> dma fence can be used at various points in the code and it's very hard
> to distinguish dma fences between different usages. Using a single
> dept_key for all the dma fences could trigger false positive reports.
>
> > diff --git a/include/linux/dma-fence.h b/include/linux/dma-fence.h
> > index d4c92fd350924..3732849a30b7e 100644
> > --- a/include/linux/dma-fence.h
> > +++ b/include/linux/dma-fence.h
>
> [ ... ]
>
> > @@ -370,8 +370,22 @@ bool dma_fence_check_and_signal_locked(struct dma_fence *fence);
> > void dma_fence_signal_locked(struct dma_fence *fence);
> > void dma_fence_signal_timestamp(struct dma_fence *fence, ktime_t timestamp);
> > void dma_fence_signal_timestamp_locked(struct dma_fence *fence, ktime_t timestamp);
> > -signed long dma_fence_default_wait(struct dma_fence *fence,
> > +signed long __dma_fence_default_wait(struct dma_fence *fence,
> > bool intr, signed long timeout);
> > +
> > +/*
> > + * Associate every caller with its own dept map.
> > + */
> > +#define dma_fence_default_wait(f, intr, t) \
> > +({ \
> > + signed long __ret; \
>
> [Severity: Medium]
> Could this local variable declaration lead to shadowing issues? If a caller
> passes arguments that reference a variable named __ret in their own scope,
> this uninitialized local variable will shadow the caller's variable, causing
> undefined behavior.
I will fix it.
> > + \
> > + sdt_might_sleep_start_timeout(NULL, t); \
> > + __ret = __dma_fence_default_wait(f, intr, t); \
>
> [Severity: Medium]
> Is it safe to evaluate the timeout argument t multiple times here?
>
> If a caller passes an expression with side effects, such as timeout--, it
> would be evaluated twice, which could lead to incorrect timeout logic.
Sure, I will fix it.
> > + sdt_might_sleep_end(); \
> > + __ret; \
> > +})
> > +
>
> [ ... ]
>
> > @@ -628,12 +642,37 @@ static inline ktime_t dma_fence_timestamp(struct dma_fence *fence)
> > return fence->timestamp;
> > }
> >
> > -signed long dma_fence_wait_timeout(struct dma_fence *,
> > +signed long __dma_fence_wait_timeout(struct dma_fence *,
> > bool intr, signed long timeout);
> > -signed long dma_fence_wait_any_timeout(struct dma_fence **fences,
> > +signed long __dma_fence_wait_any_timeout(struct dma_fence **fences,
> > uint32_t count,
> > bool intr, signed long timeout,
> > uint32_t *idx);
> > +/*
> > + * Associate every caller with its own dept map.
> > + */
> > +#define dma_fence_wait_timeout(f, intr, t) \
> > +({ \
> > + signed long __ret; \
> > + \
> > + sdt_might_sleep_start_timeout(NULL, t); \
> > + __ret = __dma_fence_wait_timeout(f, intr, t); \
>
> [Severity: Medium]
> Does this code also suffer from the same macro hygiene issues with shadowing
> __ret and evaluating the t argument twice?
Okay, I will fix it.
> > + sdt_might_sleep_end(); \
> > + __ret; \
> > +})
> > +
> > +/*
> > + * Associate every caller with its own dept map.
> > + */
> > +#define dma_fence_wait_any_timeout(fpp, count, intr, t, idx) \
> > +({ \
> > + signed long __ret; \
> > + \
> > + sdt_might_sleep_start_timeout(NULL, t); \
> > + __ret = __dma_fence_wait_any_timeout(fpp, count, intr, t, idx); \
>
> [Severity: Medium]
> Would it be safer to use a more unique variable name instead of __ret, and
Suggestion?
> evaluate t only once before passing it to sdt_might_sleep_start_timeout()
> and __dma_fence_wait_any_timeout(), similar to the other macros?
Sure.
Byungchul
> > + sdt_might_sleep_end(); \
> > + __ret; \
> > +})
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20260706061928.66713-1-byungchul@sk.com?part=28
^ permalink raw reply
* Re: [PATCH v19 34/40] rcu/update: fix same dept key collision between various types of RCU
From: Byungchul Park @ 2026-07-10 6:20 UTC (permalink / raw)
To: sashiko-reviews
Cc: dri-devel, linux-ide, linux-kernel, dlemoal, cassel,
Anna Schumaker, ojeda, tytso, Chuck Lever, Jeff Layton,
linux-rt-devel, linux-modules, kernel_team
In-Reply-To: <20260707073353.1CD771F00A3D@smtp.kernel.org>
On Tue, Jul 07, 2026 at 07:33:52AM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Low] Unused struct member `dkey` inflates stack footprint
> --
>
> commit de6aa9797881496b4a8d7d66626b1bfd19cfdd54
> Author: Yunseong Kim <ysk@kzalloc.com>
>
> rcu/update: fix same dept key collision between various types of RCU
>
> This commit addresses a false positive circular dependency report in dept
> by assigning separate dept keys and maps to each struct rcu_synchronize
> execution context.
>
> > diff --git a/include/linux/rcupdate_wait.h b/include/linux/rcupdate_wait.h
> > --- a/include/linux/rcupdate_wait.h
> > +++ b/include/linux/rcupdate_wait.h
> > @@ -19,17 +19,20 @@ struct rcu_synchronize {
> >
> > /* This is for debugging. */
> > struct rcu_gp_oldstate oldstate;
> > + struct dept_map dmap;
> > + struct dept_key dkey;
>
> [Severity: Low]
> Does this code ever use the dkey member of struct rcu_synchronize?
>
> The _wait_rcu_gp() macro defines its own static struct dept_key __key,
> which it passes as an argument to __wait_rcu_gp(). This leaves the dkey
> member of struct rcu_synchronize completely unused and uninitialized, while
> needlessly increasing the stack footprint of the rs_array allocated in the
> macro.
I will fix it.
Byungchul
> > };
> > void wakeme_after_rcu(struct rcu_head *head);
> >
> > void __wait_rcu_gp(bool checktiny, unsigned int state, int n, call_rcu_func_t *crcu_array,
> > - struct rcu_synchronize *rs_array);
> > + struct rcu_synchronize *rs_array, struct dept_key *dkey);
> >
> > #define _wait_rcu_gp(checktiny, state, ...) \
> > -do { \
> > - call_rcu_func_t __crcu_array[] = { __VA_ARGS__ }; \
> > - struct rcu_synchronize __rs_array[ARRAY_SIZE(__crcu_array)]; \
> > - __wait_rcu_gp(checktiny, state, ARRAY_SIZE(__crcu_array), __crcu_array, __rs_array); \
> > +do { \
> > + call_rcu_func_t __crcu_array[] = { __VA_ARGS__ }; \
> > + static struct dept_key __key; \
> > + struct rcu_synchronize __rs_array[ARRAY_SIZE(__crcu_array)]; \
> > + __wait_rcu_gp(checktiny, state, ARRAY_SIZE(__crcu_array), __crcu_array, __rs_array, &__key); \
> > } while (0)
>
> [ ... ]
>
> > diff --git a/kernel/rcu/update.c b/kernel/rcu/update.c
> > --- a/kernel/rcu/update.c
> > +++ b/kernel/rcu/update.c
>
> [ ... ]
>
> > @@ -426,7 +426,8 @@ void __wait_rcu_gp(bool checktiny, unsigned int state, int n, call_rcu_func_t *c
> > break;
> > if (j == i) {
> > init_rcu_head_on_stack(&rs_array[i].head);
> > - init_completion(&rs_array[i].completion);
> > + sdt_map_init_key(&rs_array[i].dmap, dkey);
> > + init_completion_dmap(&rs_array[i].completion, &rs_array[i].dmap);
> > (crcu_array[i])(&rs_array[i].head, wakeme_after_rcu);
> > }
> > }
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20260706061928.66713-1-byungchul@sk.com?part=34
^ permalink raw reply
* Re: [PATCH v19 32/40] completion, dept: introduce init_completion_dmap() API
From: Byungchul Park @ 2026-07-10 6:12 UTC (permalink / raw)
To: sashiko-reviews
Cc: dlemoal, Jeff Layton, linux-modules, dri-devel, linux-rt-devel,
cassel, linux-kernel, tytso, Chuck Lever, linux-ide,
Anna Schumaker, ojeda, kernel_team
In-Reply-To: <20260707073350.0965F1F00A3E@smtp.kernel.org>
On Tue, Jul 07, 2026 at 07:33:49AM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
> - [High] Double evaluation of arguments in wait_for_completion macros leads to skipped array indices and memory corruption.
> - [Medium] Unconditional struct growth of a widely used primitive for a debug feature.
> - [Medium] Wait event macros skip calling sdt_might_sleep_end() on signal interruptions, leaking DEPT tracking state and stacks.
> --
>
> commit 5b626a37ee940081b6d37187cde51c2f272e370c
> Author: Byungchul Park <byungchul@sk.com>
>
> completion, dept: introduce init_completion_dmap() API
>
> Currently, dept uses dept's map embedded in task_struct to track
> dependencies related to wait_for_completion() and its family. So it
> doesn't need an explicit map basically.
>
> However, for those who want to set the maps with customized class or
> key, introduce a new API to use external maps.
>
> > diff --git a/include/linux/completion.h b/include/linux/completion.h
> > index 4d8fb1d95c0a6..e50f7d9b4b974 100644
> > --- a/include/linux/completion.h
> > +++ b/include/linux/completion.h
> > @@ -27,17 +27,15 @@
> > struct completion {
> > unsigned int done;
> > struct swait_queue_head wait;
> > + struct dept_map *dmap;
> > };
>
> [Severity: Medium]
> Since struct completion is heavily embedded in many kernel data structures,
> does unconditionally adding the dmap pointer increase the global memory
> footprint even on production non-debug kernels? Should this new field be
> guarded by an #ifdef?
Good point. I will fix it, but with dkey. It will look like:
+ struct dept_key_ptr dkey_ptr;
> [ ... ]
>
> > @@ -136,13 +140,13 @@ extern void complete_all(struct completion *);
> >
> > #define wait_for_completion(x) \
> > ({ \
> > - sdt_might_sleep_start_timeout(NULL, -1L); \
> > + sdt_might_sleep_start_timeout((x)->dmap, -1L); \
> > __wait_for_completion(x); \
> > sdt_might_sleep_end(); \
> > })
>
> [Severity: High]
> Does evaluating the argument x multiple times in this macro lead to issues
> if callers pass an expression with side effects?
>
> For example, if a caller like mlx5_vdpa_exec_async_cmds() does:
>
> while (completed < issued)
> wait_for_completion(&cmds[completed++].cmd_done);
>
> Would completed++ be executed twice per iteration, skipping array elements
> and potentially causing out-of-bounds memory accesses?
Sure. I will fix it.
> [ ... ]
>
> [Severity: Medium]
> Do wait event macros such as ___wait_event() in include/linux/wait.h skip
> calling sdt_might_sleep_end() when interrupted by a signal?
>
> In include/linux/wait.h:___wait_event():
>
> if (___wait_is_interruptible(state) && __int) { \
> __ret = __int; \
> goto __out; \
> }
> ...
> sdt_might_sleep_end(); \
> __out: __ret; \
> })
>
> If the wait is interrupted by a signal, it escapes via goto __out. Because
> __out is placed after sdt_might_sleep_end(), it seems the cleanup is
> skipped.
>
> Does this permanently leak the tracking state and its underlying stack
> objects?
I will fix it.
Byungchul
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20260706061928.66713-1-byungchul@sk.com?part=32
^ permalink raw reply
* Re: [PATCH v19 29/40] dept: make dept aware of lockdep_set_lock_cmp_fn() annotation
From: Byungchul Park @ 2026-07-10 6:04 UTC (permalink / raw)
To: sashiko-reviews
Cc: linux-modules, Chuck Lever, Anna Schumaker, dri-devel,
linux-kernel, linux-rt-devel, dlemoal, cassel, tytso, Jeff Layton,
ojeda, linux-ide, kernel_team
In-Reply-To: <20260707073345.A320A1F00A3D@smtp.kernel.org>
On Tue, Jul 07, 2026 at 07:33:45AM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] DEPT falsely reports a deadlock when a task sequentially acquires three or more locks of the same class (or interleaves them with other locks), because the `lockdep_cmp_fn` bypass is improperly restricted to only the immediately preceding lock.
> --
>
> commit d03c5b926a945b89f5a90fd98986386e113df5af
> Author: Byungchul Park <byungchul@sk.com>
>
> dept: make dept aware of lockdep_set_lock_cmp_fn() annotation
>
> This commit updates the DEPT (Dependency Tracker) subsystem to respect the
> lockdep_set_lock_cmp_fn() annotation. It allows DEPT to avoid false positive
> deadlock reports by using the custom comparison function when evaluating
> dependencies between locks of the same class.
>
> > diff --git a/kernel/dependency/dept.c b/kernel/dependency/dept.c
> > --- a/kernel/dependency/dept.c
> > +++ b/kernel/dependency/dept.c
>
> [ ... ]
>
> > @@ -1658,8 +1688,13 @@ static void add_wait(struct dept_class *c, unsigned long ip,
> > if (!eh->ecxt)
> > continue;
> >
> > - if (eh->ecxt->class != c || eh->sub_l == sub_l)
> > - add_dep(eh->ecxt, w);
> > + if (eh->ecxt->class == c && eh->sub_l != sub_l)
> > + continue;
> > +
> > + if (i == dt->ecxt_held_pos - 1 && lockdep_cmp_fn(eh->map, m))
> > + continue;
>
> [Severity: Medium]
> Will this condition incorrectly bypass the comparison for earlier locks in
> the chain?
>
> By restricting the lockdep_cmp_fn() check to only the most recently
> acquired lock (i == dt->ecxt_held_pos - 1), it seems we might miss valid
> comparisons for older contexts.
No.
It's how lockdep works and it's intended by lockdep. The temporary code
just follows the rule.
> For example, if a task acquires lock A1, then lock B, and then lock A2
> (where A1 and A2 are the same class), the check against A1 will be skipped
> because A1 is no longer at dt->ecxt_held_pos - 1.
>
> This would result in a call to add_dep(A1, A2), creating a
> self-referential class dependency that triggers a false positive deadlock
> report.
No.
It's a real deadlock.
> Should lockdep_cmp_fn() be allowed to evaluate against any previously held
> lock in the chain, rather than just the immediate predecessor?
No.
Check the lockdep code first. See check_deadlock() and check_prev_add().
Byungchul
> > +
> > + add_dep(eh->ecxt, w);
> > }
> >
> > wg = next_wgen();
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20260706061928.66713-1-byungchul@sk.com?part=29
^ permalink raw reply
* Re: [PATCH v19 38/40] mm: percpu: increase PERCPU_DYNAMIC_SIZE_SHIFT on DEPT and large PAGE_SIZE
From: Byungchul Park @ 2026-07-10 5:55 UTC (permalink / raw)
To: sashiko-reviews
Cc: Jeff Layton, ojeda, tytso, Chuck Lever, Anna Schumaker,
linux-kernel, cassel, linux-rt-devel, dri-devel, dlemoal,
linux-ide, linux-modules, kernel_team
In-Reply-To: <20260707073357.1DE2D1F00A3F@smtp.kernel.org>
On Tue, Jul 07, 2026 at 07:33:56AM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] The patch addresses a compile-time BUILD_BUG_ON failure that no longer exists, referencing a deleted structure (`struct kmem_cache_cpu`) and function (`alloc_kmem_cache_cpus()`). Both have been removed from the upstream kernel. The only remaining BUILD_BUG_ON checking `PERCPU_DYNAMIC_EARLY_SIZE` in SLUB is in `alloc_kmem_cache_stats()`, which evaluates `sizeof(struct kmem_cache_stats)`. Because `struct kmem_cache_stats` consists solely of an integer array, its size is entirely unaffected by `CONFIG_DEPT` or `local_lock_t`. Thus, this patch needlessly increases the dynamically reserved per-CPU memory based on an obsolete and misleading justification.
> --
>
> commit 279fb8fb2919a407c18ab625ec67d2a3430b7b4d
> Author: Byungchul Park <byungchul@sk.com>
>
> mm: percpu: increase PERCPU_DYNAMIC_SIZE_SHIFT on DEPT and large PAGE_SIZE
>
> This commit increases PERCPU_DYNAMIC_SIZE_SHIFT to 11 when both CONFIG_DEPT
> and a PAGE_SIZE larger than 4KiB are enabled. It aims to prevent a build
> failure related to the size of struct kmem_cache_cpu.
>
> > Yunseong reported a build failure due to the BUILD_BUG_ON() statement in
> > alloc_kmem_cache_cpus(). In the following test:
> >
> > PERCPU_DYNAMIC_EARLY_SIZE < NR_KMALLOC_TYPES * KMALLOC_SHIFT_HIGH * sizeof(struct kmem_cache_cpu)
>
> [Severity: Medium]
> Are we referencing structures and functions that have been removed from the
> upstream kernel?
>
> Both alloc_kmem_cache_cpus() and struct kmem_cache_cpu no longer exist
> upstream. The only remaining BUILD_BUG_ON() checking PERCPU_DYNAMIC_EARLY_SIZE
> in SLUB is in alloc_kmem_cache_stats():
>
> mm/slub.c:alloc_kmem_cache_stats() {
> BUILD_BUG_ON(PERCPU_DYNAMIC_EARLY_SIZE <
> NR_KMALLOC_TYPES * KMALLOC_SHIFT_HIGH *
> sizeof(struct kmem_cache_stats));
> }
>
> Since struct kmem_cache_stats consists solely of an integer array, its size
> is entirely unaffected by CONFIG_DEPT or local_lock_t. Does this mean the
> increase in dynamically reserved per-CPU memory is no longer needed?
Agree. It seems no longer needed. I will fix it.
Byungchul
> > diff --git a/include/linux/percpu.h b/include/linux/percpu.h
> > index 85bf8dd9f0874..dd74321d4bbd0 100644
> > --- a/include/linux/percpu.h
> > +++ b/include/linux/percpu.h
> > @@ -43,7 +43,11 @@
> > # define PERCPU_DYNAMIC_SIZE_SHIFT 12
> > #endif /* LOCKDEP and PAGE_SIZE > 4KiB */
> > #else
> > +#if defined(CONFIG_DEPT) && !defined(CONFIG_PAGE_SIZE_4KB)
> > +#define PERCPU_DYNAMIC_SIZE_SHIFT 11
> > +#else
> > #define PERCPU_DYNAMIC_SIZE_SHIFT 10
> > +#endif /* DEPT and PAGE_SIZE > 4KiB */
> > #endif
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20260706061928.66713-1-byungchul@sk.com?part=38
^ permalink raw reply
page: next (older)
- 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