Linux Modules
 help / color / mirror / Atom feed
* 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] 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 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 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: 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: 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 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 v4] module: Extend module_blacklist parameter to built-in modules
From: Aaron Tomlin @ 2026-07-10 23:13 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Luis Chamberlain, Petr Pavlu, da.gomez, Sami Tolvanen,
	Peter Zijlstra, Andrew Morton, Masami Hiramatsu, neelx, da.anzani,
	sean, chjohnst, steve, mproche, nick.lane, Linux-Arch,
	linux-modules, linux-kernel
In-Reply-To: <33171ccd-038b-4bb8-b796-f104a131c0ee@app.fastmail.com>

On Fri, Jul 10, 2026 at 05:59:59PM +0200, Arnd Bergmann wrote:
> 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?

Hi Arnd,

Thank you for your feedback and for highlighting Andrew's request for
further context.

The primary motivation for this patch is to provide consistent
administrative control. From a system administrator's perspective, whether
a specific driver is configured as loadable or built-in is often an opaque,
distribution-level decision. If an administrator applies
module_blacklist=foo (or the modern module_denylist=foo) to disable a
problematic driver across a fleet of machines, they rightfully expect it to
be disabled. Currently, if a distribution later changes that module's
configuration from loadable to built-in, the boot parameter is silently
ignored. The driver will subsequently initialise, which can cause
unexpected operational regressions or security policy violations.

Regarding your suggestion to use initcall_blacklist=, while it is certainly
a capable mechanism, it is fundamentally considered a debugging facility
intended for developers. To utilise it, an administrator must know the
exact internal C function name of the driver's initialisation routine
(e.g., initcall_blacklist=foo_driver_init). This requires inspecting the
kernel source code and relies on internal symbols that are subject to
change between releases. Conversely, the module name itself provides a
stable, user-facing administrative interface.

Furthermore, I must credit Petr Pavlu, who offered excellent advice [1]
suggesting that extending module_blacklist= to encompass built-in modules
is the most logical and robust approach to solving this discrepancy.

[1] https://lore.kernel.org/lkml/79ace94f-31d3-4a5e-9a47-3fad69304fe5@suse.com/

> > 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?

Correct.

> That sounds more useful, but would better be done in a separate
> change, and also needs a proper changelog text.

Understood.


Kind regards,
-- 
Aaron Tomlin

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox