* Re: [PATCH v8 04/23] slab: add sheaf support for batching kfree_rcu() operations
From: Harry Yoo @ 2025-11-03 3:17 UTC (permalink / raw)
To: Daniel Gomez
Cc: Vlastimil Babka, Suren Baghdasaryan, Liam R. Howlett,
Christoph Lameter, David Rientjes, Roman Gushchin,
Uladzislau Rezki, Sidhartha Kumar, linux-mm, linux-kernel, rcu,
maple-tree, linux-modules, Luis Chamberlain, Petr Pavlu,
Sami Tolvanen, Aaron Tomlin, Lucas De Marchi
In-Reply-To: <0406562e-2066-4cf8-9902-b2b0616dd742@kernel.org>
On Fri, Oct 31, 2025 at 10:32:54PM +0100, Daniel Gomez wrote:
>
>
> On 10/09/2025 10.01, Vlastimil Babka wrote:
> > Extend the sheaf infrastructure for more efficient kfree_rcu() handling.
> > For caches with sheaves, on each cpu maintain a rcu_free sheaf in
> > addition to main and spare sheaves.
> >
> > kfree_rcu() operations will try to put objects on this sheaf. Once full,
> > the sheaf is detached and submitted to call_rcu() with a handler that
> > will try to put it in the barn, or flush to slab pages using bulk free,
> > when the barn is full. Then a new empty sheaf must be obtained to put
> > more objects there.
> >
> > It's possible that no free sheaves are available to use for a new
> > rcu_free sheaf, and the allocation in kfree_rcu() context can only use
> > GFP_NOWAIT and thus may fail. In that case, fall back to the existing
> > kfree_rcu() implementation.
> >
> > Expected advantages:
> > - batching the kfree_rcu() operations, that could eventually replace the
> > existing batching
> > - sheaves can be reused for allocations via barn instead of being
> > flushed to slabs, which is more efficient
> > - this includes cases where only some cpus are allowed to process rcu
> > callbacks (Android)
> >
> > Possible disadvantage:
> > - objects might be waiting for more than their grace period (it is
> > determined by the last object freed into the sheaf), increasing memory
> > usage - but the existing batching does that too.
> >
> > Only implement this for CONFIG_KVFREE_RCU_BATCHED as the tiny
> > implementation favors smaller memory footprint over performance.
> >
> > Also for now skip the usage of rcu sheaf for CONFIG_PREEMPT_RT as the
> > contexts where kfree_rcu() is called might not be compatible with taking
> > a barn spinlock or a GFP_NOWAIT allocation of a new sheaf taking a
> > spinlock - the current kfree_rcu() implementation avoids doing that.
> >
> > Teach kvfree_rcu_barrier() to flush all rcu_free sheaves from all caches
> > that have them. This is not a cheap operation, but the barrier usage is
> > rare - currently kmem_cache_destroy() or on module unload.
> >
> > Add CONFIG_SLUB_STATS counters free_rcu_sheaf and free_rcu_sheaf_fail to
> > count how many kfree_rcu() used the rcu_free sheaf successfully and how
> > many had to fall back to the existing implementation.
> >
> > Signed-off-by: Vlastimil Babka <vbabka@suse.cz>
>
> Hi Vlastimil,
>
> This patch increases kmod selftest (stress module loader) runtime by about
> ~50-60%, from ~200s to ~300s total execution time. My tested kernel has
> CONFIG_KVFREE_RCU_BATCHED enabled. Any idea or suggestions on what might be
> causing this, or how to address it?
This is likely due to increased kvfree_rcu_barrier() during module unload.
It currently iterates over all CPUs x slab caches (that enabled sheaves,
there should be only a few now) pair to make sure rcu sheaf is flushed
by the time kvfree_rcu_barrier() returns.
Just being curious, do you have any serious workload that depends on
the performance of module unload?
--
Cheers,
Harry / Hyeonggon
^ permalink raw reply
* Re: [PATCH v2 3/3] module: Add compile-time check for embedded NUL characters
From: Hans Verkuil @ 2025-11-03 8:54 UTC (permalink / raw)
To: Kees Cook, Luis Chamberlain
Cc: Rusty Russell, Petr Pavlu, Daniel Gomez, Sami Tolvanen,
linux-modules, Malcolm Priestley, Mauro Carvalho Chehab,
Hans Verkuil, Uwe Kleine-König, linux-kernel, linux-media,
linux-hardening
In-Reply-To: <20251010030610.3032147-3-kees@kernel.org>
On 10/10/2025 05:06, Kees Cook wrote:
> Long ago, the kernel module license checks were bypassed by embedding a
> NUL character in the MODULE_LICENSE() string[1]. By using a string like
> "GPL\0proprietary text", the kernel would only read "GPL" due to C string
> termination at the NUL byte, allowing proprietary modules to avoid kernel
> tainting and access GPL-only symbols.
>
> The MODULE_INFO() macro stores these strings in the .modinfo ELF
> section, and get_next_modinfo() uses strcmp()-family functions
> which stop at the first NUL. This split the embedded string into two
> separate .modinfo entries, with only the first part being processed by
> license_is_gpl_compatible().
>
> Add a compile-time check using static_assert that compares the full
> string length (sizeof - 1) against __builtin_strlen(), which stops at
> the first NUL. If they differ, compilation fails with a clear error
> message.
>
> While this check can still be circumvented by modifying the ELF binary
> post-compilation, it prevents accidental embedded NULs and forces
> intentional abuse to require deliberate binary manipulation rather than
> simple source-level tricks.
>
> Build tested with test modules containing both valid and invalid license
> strings. The check correctly rejects:
>
> MODULE_LICENSE("GPL\0proprietary")
>
> while accepting normal declarations:
>
> MODULE_LICENSE("GPL")
Who will take this series? I can take the first two media patches and
someone else can take this last patch, or I can take all, or someone
else can take all patches. The media patches already have my 'Reviewed-by'.
Any preferences?
Regards,
Hans
>
> Link: https://lwn.net/Articles/82305/ [1]
> Suggested-by: Rusty Russell <rusty@rustcorp.com.au>
> Signed-off-by: Kees Cook <kees@kernel.org>
> ---
> Cc: Luis Chamberlain <mcgrof@kernel.org>
> Cc: Petr Pavlu <petr.pavlu@suse.com>
> Cc: Daniel Gomez <da.gomez@kernel.org>
> Cc: Sami Tolvanen <samitolvanen@google.com>
> Cc: <linux-modules@vger.kernel.org>
> ---
> include/linux/moduleparam.h | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/include/linux/moduleparam.h b/include/linux/moduleparam.h
> index 6907aedc4f74..915f32f7d888 100644
> --- a/include/linux/moduleparam.h
> +++ b/include/linux/moduleparam.h
> @@ -26,6 +26,9 @@
>
> /* Generic info of form tag = "info" */
> #define MODULE_INFO(tag, info) \
> + static_assert( \
> + sizeof(info) - 1 == __builtin_strlen(info), \
> + "MODULE_INFO(" #tag ", ...) contains embedded NUL byte"); \
> static const char __UNIQUE_ID(modinfo)[] \
> __used __section(".modinfo") __aligned(1) \
> = __MODULE_INFO_PREFIX __stringify(tag) "=" info
^ permalink raw reply
* Re: [PATCH v2 3/3] module: Add compile-time check for embedded NUL characters
From: Daniel Gomez @ 2025-11-03 8:58 UTC (permalink / raw)
To: Hans Verkuil, Kees Cook, Luis Chamberlain
Cc: Rusty Russell, Petr Pavlu, Sami Tolvanen, linux-modules,
Malcolm Priestley, Mauro Carvalho Chehab, Hans Verkuil,
Uwe Kleine-König, linux-kernel, linux-media, linux-hardening
In-Reply-To: <91866583-037f-4607-b148-4ddf38ffaf51@kernel.org>
On 03/11/2025 09.54, Hans Verkuil wrote:
> On 10/10/2025 05:06, Kees Cook wrote:
>> Long ago, the kernel module license checks were bypassed by embedding a
>> NUL character in the MODULE_LICENSE() string[1]. By using a string like
>> "GPL\0proprietary text", the kernel would only read "GPL" due to C string
>> termination at the NUL byte, allowing proprietary modules to avoid kernel
>> tainting and access GPL-only symbols.
>>
>> The MODULE_INFO() macro stores these strings in the .modinfo ELF
>> section, and get_next_modinfo() uses strcmp()-family functions
>> which stop at the first NUL. This split the embedded string into two
>> separate .modinfo entries, with only the first part being processed by
>> license_is_gpl_compatible().
>>
>> Add a compile-time check using static_assert that compares the full
>> string length (sizeof - 1) against __builtin_strlen(), which stops at
>> the first NUL. If they differ, compilation fails with a clear error
>> message.
>>
>> While this check can still be circumvented by modifying the ELF binary
>> post-compilation, it prevents accidental embedded NULs and forces
>> intentional abuse to require deliberate binary manipulation rather than
>> simple source-level tricks.
>>
>> Build tested with test modules containing both valid and invalid license
>> strings. The check correctly rejects:
>>
>> MODULE_LICENSE("GPL\0proprietary")
>>
>> while accepting normal declarations:
>>
>> MODULE_LICENSE("GPL")
>
> Who will take this series? I can take the first two media patches and
> someone else can take this last patch, or I can take all, or someone
> else can take all patches. The media patches already have my 'Reviewed-by'.
>
> Any preferences?
I will take patch 3 in modules' tree.
>
> Regards,
>
> Hans
>
>>
>> Link: https://lwn.net/Articles/82305/ [1]
>> Suggested-by: Rusty Russell <rusty@rustcorp.com.au>
>> Signed-off-by: Kees Cook <kees@kernel.org>
Reviewed-by: Daniel Gomez <da.gomez@samsung.com>
^ permalink raw reply
* Re: [PATCH v2 3/3] module: Add compile-time check for embedded NUL characters
From: Hans Verkuil @ 2025-11-03 9:05 UTC (permalink / raw)
To: Daniel Gomez, Kees Cook, Luis Chamberlain
Cc: Rusty Russell, Petr Pavlu, Sami Tolvanen, linux-modules,
Malcolm Priestley, Mauro Carvalho Chehab, Hans Verkuil,
Uwe Kleine-König, linux-kernel, linux-media, linux-hardening
In-Reply-To: <1c3e9bc8-0d25-46b2-98a3-643157f78b21@kernel.org>
On 03/11/2025 09:58, Daniel Gomez wrote:
> On 03/11/2025 09.54, Hans Verkuil wrote:
>> On 10/10/2025 05:06, Kees Cook wrote:
>>> Long ago, the kernel module license checks were bypassed by embedding a
>>> NUL character in the MODULE_LICENSE() string[1]. By using a string like
>>> "GPL\0proprietary text", the kernel would only read "GPL" due to C string
>>> termination at the NUL byte, allowing proprietary modules to avoid kernel
>>> tainting and access GPL-only symbols.
>>>
>>> The MODULE_INFO() macro stores these strings in the .modinfo ELF
>>> section, and get_next_modinfo() uses strcmp()-family functions
>>> which stop at the first NUL. This split the embedded string into two
>>> separate .modinfo entries, with only the first part being processed by
>>> license_is_gpl_compatible().
>>>
>>> Add a compile-time check using static_assert that compares the full
>>> string length (sizeof - 1) against __builtin_strlen(), which stops at
>>> the first NUL. If they differ, compilation fails with a clear error
>>> message.
>>>
>>> While this check can still be circumvented by modifying the ELF binary
>>> post-compilation, it prevents accidental embedded NULs and forces
>>> intentional abuse to require deliberate binary manipulation rather than
>>> simple source-level tricks.
>>>
>>> Build tested with test modules containing both valid and invalid license
>>> strings. The check correctly rejects:
>>>
>>> MODULE_LICENSE("GPL\0proprietary")
>>>
>>> while accepting normal declarations:
>>>
>>> MODULE_LICENSE("GPL")
>>
>> Who will take this series? I can take the first two media patches and
>> someone else can take this last patch, or I can take all, or someone
>> else can take all patches. The media patches already have my 'Reviewed-by'.
>>
>> Any preferences?
>
> I will take patch 3 in modules' tree.
OK, then I'll take patches 1 and 2 in the media tree.
Regards,
Hans
>
>
>>
>> Regards,
>>
>> Hans
>>
>>>
>>> Link: https://lwn.net/Articles/82305/ [1]
>>> Suggested-by: Rusty Russell <rusty@rustcorp.com.au>
>>> Signed-off-by: Kees Cook <kees@kernel.org>
>
> Reviewed-by: Daniel Gomez <da.gomez@samsung.com>
^ permalink raw reply
* Re: [PATCH] docs: ABI: sysfs-module: update modules taint flags
From: Petr Pavlu @ 2025-11-03 9:19 UTC (permalink / raw)
To: Randy Dunlap
Cc: Jonathan Corbet, linux-doc, Greg Kroah-Hartman, Luis Chamberlain,
Daniel Gomez, Sami Tolvanen, linux-modules, linux-kernel
In-Reply-To: <20251102060458.517911-1-rdunlap@infradead.org>
On 11/2/25 7:04 AM, Randy Dunlap wrote:
> Add missing taint flags for loadable modules, as pointed out by
> Petr Pavlu [1].
>
> Signed-off-by: Randy Dunlap <rdunlap@infradead.org>
> [1] https://lore.kernel.org/all/c58152f1-0fbe-4f50-bb61-e2f4c0584025@suse.com/
> ---
Looks ok to me. I would only move the "[1]" line before the
"Signed-off-by" tag in the commit message and separate them by an empty
line. Some tooling might rely on the tags being separately at the end.
I guess this can be directly adjusted by a maintainer that picks up the
patch and there is no need to resend it.
Reviewed-by: Petr Pavlu <petr.pavlu@suse.com>
--
Thanks,
Petr
> Cc: Petr Pavlu <petr.pavlu@suse.com>
> Cc: Jonathan Corbet <corbet@lwn.net>
> Cc: linux-doc@vger.kernel.org
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: Luis Chamberlain <mcgrof@kernel.org>
> Cc: Daniel Gomez <da.gomez@kernel.org>
> Cc: Sami Tolvanen <samitolvanen@google.com>
> Cc: linux-modules@vger.kernel.org
> ---
> Documentation/ABI/testing/sysfs-module | 2 ++
> 1 file changed, 2 insertions(+)
>
> --- linux-next-20251031.orig/Documentation/ABI/testing/sysfs-module
> +++ linux-next-20251031/Documentation/ABI/testing/sysfs-module
> @@ -59,6 +59,8 @@ Description: Module taint flags:
> F force-loaded module
> C staging driver module
> E unsigned module
> + K livepatch module
> + N in-kernel test module
> == =====================
>
> What: /sys/module/grant_table/parameters/free_per_iteration
^ permalink raw reply
* Re: [PATCH v3 22/28] x86: Use RCU in all users of __module_address().
From: Michal Pecio @ 2025-11-03 10:08 UTC (permalink / raw)
To: bigeasy
Cc: bp, da.gomez, dave.hansen, hpa, jpoimboe, linux-kernel,
linux-modules, mcgrof, mingo, paulmck, peterz, petr.pavlu,
samitolvanen, tglx, x86
In-Reply-To: <20250108090457.512198-23-bigeasy@linutronix.de>
> x86: Use RCU in all users of __module_address().
>
> __module_address() can be invoked within a RCU section, there is no
> requirement to have preemption disabled.
>
> Replace the preempt_disable() section around __module_address() with
> RCU.
>
> Cc: H. Peter Anvin <hpa@zytor.com>
> Cc: Borislav Petkov <bp@alien8.de>
> Cc: Dave Hansen <dave.hansen@linux.intel.com>
> Cc: Ingo Molnar <mingo@redhat.com>
> Cc: Josh Poimboeuf <jpoimboe@kernel.org>
> Cc: Peter Zijlstra <peterz@infradead.org>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: x86@kernel.org
> Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
> Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> ---
> arch/x86/kernel/callthunks.c | 3 +--
> arch/x86/kernel/unwind_orc.c | 4 +---
> 2 files changed, 2 insertions(+), 5 deletions(-)
>
> diff --git a/arch/x86/kernel/callthunks.c b/arch/x86/kernel/callthunks.c
> index f17d166078823..276b5368ff6b0 100644
> --- a/arch/x86/kernel/callthunks.c
> +++ b/arch/x86/kernel/callthunks.c
> @@ -98,11 +98,10 @@ static inline bool within_module_coretext(void *addr)
> #ifdef CONFIG_MODULES
> struct module *mod;
>
> - preempt_disable();
> + guard(rcu)();
> mod = __module_address((unsigned long)addr);
> if (mod && within_module_core((unsigned long)addr, mod))
> ret = true;
> - preempt_enable();
> #endif
> return ret;
> }
> diff --git a/arch/x86/kernel/unwind_orc.c b/arch/x86/kernel/unwind_orc.c
> index d4705a348a804..977ee75e047c8 100644
> --- a/arch/x86/kernel/unwind_orc.c
> +++ b/arch/x86/kernel/unwind_orc.c
> @@ -476,7 +476,7 @@ bool unwind_next_frame(struct unwind_state *state)
> return false;
>
> /* Don't let modules unload while we're reading their ORC data. */
> - preempt_disable();
> + guard(rcu)();
>
> /* End-of-stack check for user tasks: */
> if (state->regs && user_mode(state->regs))
> @@ -669,14 +669,12 @@ bool unwind_next_frame(struct unwind_state *state)
> goto err;
> }
>
> - preempt_enable();
> return true;
Hi,
There is a regression report on a distribution forum which involves
an out of tree module on a patched kernel (yes, I know) calling
stack_trace_save() in task context, which arrives here and apparently
calls the various deref_stack_xxx() functions with preemption enabled,
which in turn call stack_access_ok() leading to a BUG:
Nov 02 21:44:30 ArchBasement kernel: BUG: using smp_processor_id() in preemptible [00000000] code: Xorg/1183
Nov 02 21:44:30 ArchBasement kernel: caller is in_entry_stack+0x11/0x60
Nov 02 21:44:30 ArchBasement kernel: CPU: 0 UID: 1000 PID: 1183 Comm: Xorg Tainted: P OE 6.16.12-hardened1-1-hardened #1 PREEMPT(full) 6edb90a7a07fab33bbee72d6d5ef53ba6eec3b9c
Nov 02 21:44:30 ArchBasement kernel: Tainted: [P]=PROPRIETARY_MODULE, [O]=OOT_MODULE, [E]=UNSIGNED_MODULE
Nov 02 21:44:30 ArchBasement kernel: Hardware name: ASUS All Series/Z97-E, BIOS 0803 02/23/2016
Nov 02 21:44:30 ArchBasement kernel: Call Trace:
Nov 02 21:44:30 ArchBasement kernel: <TASK>
Nov 02 21:44:30 ArchBasement kernel: dump_stack_lvl+0x5d/0x80
Nov 02 21:44:30 ArchBasement kernel: check_preemption_disabled+0xe5/0xf0
Nov 02 21:44:30 ArchBasement kernel: in_entry_stack+0x11/0x60
Nov 02 21:44:30 ArchBasement kernel: get_stack_info+0x2c/0x80
Nov 02 21:44:30 ArchBasement kernel: stack_access_ok+0x51/0xa0
Nov 02 21:44:30 ArchBasement kernel: unwind_next_frame+0x1cb/0x7b0
Nov 02 21:44:30 ArchBasement kernel: ? _nv003168kms+0x42/0x50 [nvidia_modeset 90775ea8a26c5e58b97ef4b3f46eb45efa040eb2]
Nov 02 21:44:30 ArchBasement kernel: ? __pfx_stack_trace_consume_entry+0x10/0x10
Nov 02 21:44:30 ArchBasement kernel: arch_stack_walk+0xa6/0x110
Nov 02 21:44:30 ArchBasement kernel: ? _nv003168kms+0x42/0x50 [nvidia_modeset 90775ea8a26c5e58b97ef4b3f46eb45efa040eb2]
Nov 02 21:44:30 ArchBasement kernel: stack_trace_save+0x4d/0x70
Is this nvidia doing something wrong, or a problem with this commit?
The removed code suggests that preemption is allowed here, and as far
as I see, this call trace is still possible on vanilla 6.18. Perhaps
preempt_disable() needs to be restored around this code?
Regards,
Michal
^ permalink raw reply
* Re: [PATCH v3 22/28] x86: Use RCU in all users of __module_address().
From: Sebastian Andrzej Siewior @ 2025-11-03 10:34 UTC (permalink / raw)
To: Michal Pecio
Cc: bp, da.gomez, dave.hansen, hpa, jpoimboe, linux-kernel,
linux-modules, mcgrof, mingo, paulmck, peterz, petr.pavlu,
samitolvanen, tglx, x86
In-Reply-To: <20251103110835.1dca378c.michal.pecio@gmail.com>
On 2025-11-03 11:08:35 [+0100], Michal Pecio wrote:
> Hi,
Hi,
> There is a regression report on a distribution forum which involves
> an out of tree module on a patched kernel (yes, I know) calling
> stack_trace_save() in task context, which arrives here and apparently
> calls the various deref_stack_xxx() functions with preemption enabled,
> which in turn call stack_access_ok() leading to a BUG:
>
> Nov 02 21:44:30 ArchBasement kernel: BUG: using smp_processor_id() in preemptible [00000000] code: Xorg/1183
> Nov 02 21:44:30 ArchBasement kernel: caller is in_entry_stack+0x11/0x60
> Nov 02 21:44:30 ArchBasement kernel: CPU: 0 UID: 1000 PID: 1183 Comm: Xorg Tainted: P OE 6.16.12-hardened1-1-hardened #1 PREEMPT(full) 6edb90a7a07fab33bbee72d6d5ef53ba6eec3b9c
> Nov 02 21:44:30 ArchBasement kernel: Tainted: [P]=PROPRIETARY_MODULE, [O]=OOT_MODULE, [E]=UNSIGNED_MODULE
> Nov 02 21:44:30 ArchBasement kernel: Hardware name: ASUS All Series/Z97-E, BIOS 0803 02/23/2016
> Nov 02 21:44:30 ArchBasement kernel: Call Trace:
> Nov 02 21:44:30 ArchBasement kernel: <TASK>
> Nov 02 21:44:30 ArchBasement kernel: dump_stack_lvl+0x5d/0x80
> Nov 02 21:44:30 ArchBasement kernel: check_preemption_disabled+0xe5/0xf0
> Nov 02 21:44:30 ArchBasement kernel: in_entry_stack+0x11/0x60
> Nov 02 21:44:30 ArchBasement kernel: get_stack_info+0x2c/0x80
> Nov 02 21:44:30 ArchBasement kernel: stack_access_ok+0x51/0xa0
> Nov 02 21:44:30 ArchBasement kernel: unwind_next_frame+0x1cb/0x7b0
> Nov 02 21:44:30 ArchBasement kernel: ? _nv003168kms+0x42/0x50 [nvidia_modeset 90775ea8a26c5e58b97ef4b3f46eb45efa040eb2]
> Nov 02 21:44:30 ArchBasement kernel: ? __pfx_stack_trace_consume_entry+0x10/0x10
> Nov 02 21:44:30 ArchBasement kernel: arch_stack_walk+0xa6/0x110
> Nov 02 21:44:30 ArchBasement kernel: ? _nv003168kms+0x42/0x50 [nvidia_modeset 90775ea8a26c5e58b97ef4b3f46eb45efa040eb2]
> Nov 02 21:44:30 ArchBasement kernel: stack_trace_save+0x4d/0x70
>
> Is this nvidia doing something wrong, or a problem with this commit?
>
> The removed code suggests that preemption is allowed here, and as far
> as I see, this call trace is still possible on vanilla 6.18. Perhaps
> preempt_disable() needs to be restored around this code?
Do you have the complete backtrace? Is this SMP or UP build?
> Regards,
> Michal
Sebastian
^ permalink raw reply
* Re: [PATCH v3 22/28] x86: Use RCU in all users of __module_address().
From: Michal Pecio @ 2025-11-03 10:39 UTC (permalink / raw)
To: Sebastian Andrzej Siewior
Cc: bp, da.gomez, dave.hansen, hpa, jpoimboe, linux-kernel,
linux-modules, mcgrof, mingo, paulmck, peterz, petr.pavlu,
samitolvanen, tglx, x86
In-Reply-To: <20251103103434.D6m23Iud@linutronix.de>
On Mon, 3 Nov 2025 11:34:34 +0100, Sebastian Andrzej Siewior wrote:
> On 2025-11-03 11:08:35 [+0100], Michal Pecio wrote:
> > Hi,
> Hi,
>
> > There is a regression report on a distribution forum which involves
> > an out of tree module on a patched kernel (yes, I know) calling
> > stack_trace_save() in task context, which arrives here and apparently
> > calls the various deref_stack_xxx() functions with preemption enabled,
> > which in turn call stack_access_ok() leading to a BUG:
> >
> > Nov 02 21:44:30 ArchBasement kernel: BUG: using smp_processor_id() in preemptible [00000000] code: Xorg/1183
> > Nov 02 21:44:30 ArchBasement kernel: caller is in_entry_stack+0x11/0x60
> > Nov 02 21:44:30 ArchBasement kernel: CPU: 0 UID: 1000 PID: 1183 Comm: Xorg Tainted: P OE 6.16.12-hardened1-1-hardened #1 PREEMPT(full) 6edb90a7a07fab33bbee72d6d5ef53ba6eec3b9c
> > Nov 02 21:44:30 ArchBasement kernel: Tainted: [P]=PROPRIETARY_MODULE, [O]=OOT_MODULE, [E]=UNSIGNED_MODULE
> > Nov 02 21:44:30 ArchBasement kernel: Hardware name: ASUS All Series/Z97-E, BIOS 0803 02/23/2016
> > Nov 02 21:44:30 ArchBasement kernel: Call Trace:
> > Nov 02 21:44:30 ArchBasement kernel: <TASK>
> > Nov 02 21:44:30 ArchBasement kernel: dump_stack_lvl+0x5d/0x80
> > Nov 02 21:44:30 ArchBasement kernel: check_preemption_disabled+0xe5/0xf0
> > Nov 02 21:44:30 ArchBasement kernel: in_entry_stack+0x11/0x60
> > Nov 02 21:44:30 ArchBasement kernel: get_stack_info+0x2c/0x80
> > Nov 02 21:44:30 ArchBasement kernel: stack_access_ok+0x51/0xa0
> > Nov 02 21:44:30 ArchBasement kernel: unwind_next_frame+0x1cb/0x7b0
> > Nov 02 21:44:30 ArchBasement kernel: ? _nv003168kms+0x42/0x50 [nvidia_modeset 90775ea8a26c5e58b97ef4b3f46eb45efa040eb2]
> > Nov 02 21:44:30 ArchBasement kernel: ? __pfx_stack_trace_consume_entry+0x10/0x10
> > Nov 02 21:44:30 ArchBasement kernel: arch_stack_walk+0xa6/0x110
> > Nov 02 21:44:30 ArchBasement kernel: ? _nv003168kms+0x42/0x50 [nvidia_modeset 90775ea8a26c5e58b97ef4b3f46eb45efa040eb2]
> > Nov 02 21:44:30 ArchBasement kernel: stack_trace_save+0x4d/0x70
> >
> > Is this nvidia doing something wrong, or a problem with this commit?
> >
> > The removed code suggests that preemption is allowed here, and as far
> > as I see, this call trace is still possible on vanilla 6.18. Perhaps
> > preempt_disable() needs to be restored around this code?
>
> Do you have the complete backtrace? Is this SMP or UP build?
Sorry, I forgot to include the link. There is also a similar warning
regarding __this_cpu_read(). Pretty sure the kernel is SMP.
https://bbs.archlinux.org/viewtopic.php?id=309960
^ permalink raw reply
* Re: [PATCH v3 22/28] x86: Use RCU in all users of __module_address().
From: Sebastian Andrzej Siewior @ 2025-11-03 11:37 UTC (permalink / raw)
To: Michal Pecio
Cc: bp, da.gomez, dave.hansen, hpa, jpoimboe, linux-kernel,
linux-modules, mcgrof, mingo, paulmck, peterz, petr.pavlu,
samitolvanen, tglx, x86
In-Reply-To: <20251103113907.4e647f33.michal.pecio@gmail.com>
On 2025-11-03 11:39:07 [+0100], Michal Pecio wrote:
> Sorry, I forgot to include the link. There is also a similar warning
> regarding __this_cpu_read(). Pretty sure the kernel is SMP.
>
> https://bbs.archlinux.org/viewtopic.php?id=309960
The stack trace is a bit odd. The compressed version is:
| BUG: using smp_processor_id() in preemptible [00000000] code: Xorg/1183
| caller is in_entry_stack+0x11/0x60
| CPU: 3 UID: 1000 PID: 1183 Comm: Xorg Tainted: P OE 6.16.12-hardened1-1-hardened #1 PREEMPT(full) 6edb90a7a07fab33bbee72d6d5ef53ba6eec3b9c
| Tainted: [P]=PROPRIETARY_MODULE, [O]=OOT_MODULE, [E]=UNSIGNED_MODULE
| Hardware name: ASUS All Series/Z97-E, BIOS 0803 02/23/2016
| Call Trace:
| <TASK>
| dump_stack_lvl+0x5d/0x80
| check_preemption_disabled+0xe5/0xf0
| in_entry_stack+0x11/0x60
| get_stack_info+0x2c/0x80
| stack_access_ok+0x51/0xa0
| unwind_next_frame+0x1cb/0x7b0
| arch_stack_walk+0xa6/0x110
| stack_trace_save+0x4d/0x70
| __kfence_alloc+0xb7/0x6f0
| __kmalloc_noprof+0x520/0x560
| os_alloc_mem+0x108/0x120 [nvidia 9746d397d5c5bffeb186e829669bb24c0846a4a7]
| _nv015295rm+0x34/0x50 [nvidia 9746d397d5c5bffeb186e829669bb24c0846a4a7]
| _nv015297rm+0x2b/0xd0 [nvidia 9746d397d5c5bffeb186e829669bb24c0846a4a7]
| _nv016352rm+0x1c/0x90 [nvidia 9746d397d5c5bffeb186e829669bb24c0846a4a7]
| _nv059298rm+0x65/0xb0 [nvidia 9746d397d5c5bffeb186e829669bb24c0846a4a7]
| _nv054041rm+0x20f/0x360 [nvidia 9746d397d5c5bffeb186e829669bb24c0846a4a7]
| _nv056165rm+0x54/0xd0 [nvidia 9746d397d5c5bffeb186e829669bb24c0846a4a7]
| _nv056096rm+0xa0/0x500 [nvidia 9746d397d5c5bffeb186e829669bb24c0846a4a7]
| _nv015919rm+0x424/0x680 [nvidia 9746d397d5c5bffeb186e829669bb24c0846a4a7]
| _nv054015rm+0x69/0xd0 [nvidia 9746d397d5c5bffeb186e829669bb24c0846a4a7]
| _nv014185rm+0x86/0xa0 [nvidia 9746d397d5c5bffeb186e829669bb24c0846a4a7]
| _nv000652rm+0x5e/0x70 [nvidia 9746d397d5c5bffeb186e829669bb24c0846a4a7]
| rm_kernel_rmapi_op+0x167/0x273 [nvidia 9746d397d5c5bffeb186e829669bb24c0846a4a7]
| nvkms_call_rm+0x4c/0x80 [nvidia_modeset 90775ea8a26c5e58b97ef4b3f46eb45efa040eb2]
| _nv003168kms+0x42/0x50 [nvidia_modeset 90775ea8a26c5e58b97ef4b3f46eb45efa040eb2]
| ? do_syscall_64+0x82/0x8d0
| ? entry_SYSCALL_64_after_hwframe+0x76/0x7e
| </TASK>
The last two entries start with a '?' which means it did not originate
from the "stack unwind" but was laying around while passing through.
I would expect the last two entries to be there without the '?' because
userland (as in X here) enters the kernel via a proper syscall entry
which should be part of the stack strace.
Now, get_stack_info() where the warning originates: It starts with a
check to see if the stack pointer belongs to the current task's stack
frame which it does not. Then it checks if the task found is the
currently running task. That it does. So in that case, we must be
serving an exception (such as an IRQ) because the stack does not belong
to the current task. However preemption is not disabled which indicates
that we do not do this.
This in turn suggests that nvidia replaced the stack from while entering
the syscall probably in _nv003168kms() or the binary blob which invokes
the kernel function does not have a proper ORC entry which leads to a
wrong turn in the process.
So the warning is well deserved.
Sebastian
^ permalink raw reply
* Re: [PATCH v18 0/7] rust: extend `module!` macro with integer parameter support
From: Daniel Gomez @ 2025-11-03 14:47 UTC (permalink / raw)
To: Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Alice Ryhl, Masahiro Yamada,
Nathan Chancellor, Luis Chamberlain, Danilo Krummrich,
Benno Lossin, Benno Lossin, Nicolas Schier, Andreas Hindborg
Cc: Trevor Gross, Adam Bratschi-Kaye, rust-for-linux, linux-kernel,
linux-kbuild, Petr Pavlu, Sami Tolvanen, Daniel Gomez,
Simona Vetter, Greg KH, Fiona Behrens, Daniel Almeida,
linux-modules
In-Reply-To: <20250924-module-params-v3-v18-0-bf512c35d910@kernel.org>
On Wed, 24 Sep 2025 14:39:23 +0200, Andreas Hindborg wrote:
> Extend the `module!` macro with support module parameters. Also add some
> string to integer parsing functions.
>
> Based on the original module parameter support by Miguel [1],
> later extended and generalized by Adam for more types [2][3].
> Originally tracked at [4].
>
> [...]
Applied, thanks!
[1/7] rust: sync: add `SetOnce`
commit: 821fe7bf16c57d690f4f92997f4e51abb93e0347
[2/7] rust: str: add radix prefixed integer parsing functions
commit: 51d9ee90ea9060be240830eb28f5f117ad00318c
[3/7] rust: introduce module_param module
commit: 0b08fc292842a13aa496413b48c1efb83573b8c6
[4/7] rust: module: use a reference in macros::module::module
commit: 3809d7a89fe550bf4065c04adff6dac610daddad
[5/7] rust: module: update the module macro with module parameter support
commit: 0b24f9740f26ac7ad91ac0f4de27717c14de91bd
[6/7] rust: samples: add a module parameter to the rust_minimal sample
commit: e119c2fe8c78632188f6cdeae620951a7032855a
[7/7] modules: add rust modules files to MAINTAINERS
commit: ee3b8134b2bae848e03e56c090ceca4ae76cee06
Best regards,
--
Daniel Gomez <da.gomez@kernel.org>
^ permalink raw reply
* [PATCH v3 0/8] scalable symbol flags with __kflagstab
From: Siddharth Nayyar @ 2025-11-03 16:19 UTC (permalink / raw)
To: petr.pavlu, corbet
Cc: arnd, linux-arch, linux-kbuild, linux-kernel, linux-modules,
mcgrof, nathan, nicolas.schier, samitolvanen, sidnayyar, maennich,
gprocida
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.
This series previously added symbol import protection which aims to
restrict the use of "protected symbol" exported by vmlinux, as a use
case for which __kflagstab is being introduced. Such symbols are only
allowed to be imported by signed modules when symbol protection is
enabled. This functionality requires more thought and discussion [1],
and therefore I will create a separate patch series for it. In the
meantime, this series now only focuses on introduction of __kflagstab
which right is an improvement to the module loader's code health [2].
Thank you Petr Pavlu and Jonathan Corbet for their valuable feedback.
---
Changes from v2:
- removed symbol import protection to spin off into its own series
v2:
https://lore.kernel.org/20251013153918.2206045-1-sidnayyar@google.com/
Changes from v1:
- added a check to ensure __kflagstab is present
- added warnings for the obsolete *_gpl sections
- moved protected symbol check before ref_module() call
- moved protected symbol check failure warning to issue detection point
v1:
https://lore.kernel.org/20250829105418.3053274-1-sidnayyar@google.com/
[1] https://lore.kernel.org/cac5ed5e-3320-4db0-99d8-ea5e97e56bfb@suse.com/
[2] https://lore.kernel.org/2bf54830-ea9c-4962-a7ef-653fbed8f8c0@suse.com/
Siddharth Nayyar (8):
define kernel symbol flags
linker: add kflagstab section to vmlinux and modules
modpost: create entries for kflagstab
module loader: use kflagstab instead of *_gpl sections
modpost: put all exported symbols in ksymtab section
module loader: remove references of *_gpl sections
linker: remove *_gpl sections from vmlinux and modules
remove references to *_gpl sections in documentation
Documentation/kbuild/modules.rst | 11 ++--
include/asm-generic/vmlinux.lds.h | 21 ++----
include/linux/export-internal.h | 28 +++++---
include/linux/module.h | 4 +-
include/linux/module_symbol.h | 5 ++
kernel/module/internal.h | 4 +-
kernel/module/main.c | 102 ++++++++++++++----------------
scripts/mod/modpost.c | 16 +++--
scripts/module.lds.S | 3 +-
9 files changed, 99 insertions(+), 95 deletions(-)
--
2.51.1.930.gacf6e81ea2-goog
^ permalink raw reply
* [PATCH v3 1/8] define kernel symbol flags
From: Siddharth Nayyar @ 2025-11-03 16:19 UTC (permalink / raw)
To: petr.pavlu, corbet
Cc: arnd, linux-arch, linux-kbuild, linux-kernel, linux-modules,
mcgrof, nathan, nicolas.schier, samitolvanen, sidnayyar, maennich,
gprocida
In-Reply-To: <20251103161954.1351784-1-sidnayyar@google.com>
Symbol flags is an enumeration used to represent flags as a bitset, for
example a flag to tell if a symbols GPL only.
Signed-off-by: Siddharth Nayyar <sidnayyar@google.com>
Reviewed-by: Petr Pavlu <petr.pavlu@suse.com>
---
include/linux/module_symbol.h | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/include/linux/module_symbol.h b/include/linux/module_symbol.h
index 77c9895b9ddb..574609aced99 100644
--- a/include/linux/module_symbol.h
+++ b/include/linux/module_symbol.h
@@ -2,6 +2,11 @@
#ifndef _LINUX_MODULE_SYMBOL_H
#define _LINUX_MODULE_SYMBOL_H
+/* Kernel symbol flags bitset. */
+enum ksym_flags {
+ KSYM_FLAG_GPL_ONLY = 1 << 0,
+};
+
/* This ignores the intensely annoying "mapping symbols" found in ELF files. */
static inline bool is_mapping_symbol(const char *str)
{
--
2.51.1.930.gacf6e81ea2-goog
^ permalink raw reply related
* [PATCH v3 2/8] linker: add kflagstab section to vmlinux and modules
From: Siddharth Nayyar @ 2025-11-03 16:19 UTC (permalink / raw)
To: petr.pavlu, corbet
Cc: arnd, linux-arch, linux-kbuild, linux-kernel, linux-modules,
mcgrof, nathan, nicolas.schier, samitolvanen, sidnayyar, maennich,
gprocida
In-Reply-To: <20251103161954.1351784-1-sidnayyar@google.com>
This section will contain read-only kernel symbol flag values in the
form of a 8-bit bitset.
Signed-off-by: Siddharth Nayyar <sidnayyar@google.com>
Reviewed-by: Petr Pavlu <petr.pavlu@suse.com>
---
include/asm-generic/vmlinux.lds.h | 7 +++++++
scripts/module.lds.S | 1 +
2 files changed, 8 insertions(+)
diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
index ae2d2359b79e..310e2de56211 100644
--- a/include/asm-generic/vmlinux.lds.h
+++ b/include/asm-generic/vmlinux.lds.h
@@ -518,6 +518,13 @@ defined(CONFIG_AUTOFDO_CLANG) || defined(CONFIG_PROPELLER_CLANG)
__stop___kcrctab_gpl = .; \
} \
\
+ /* Kernel symbol flags table */ \
+ __kflagstab : AT(ADDR(__kflagstab) - LOAD_OFFSET) { \
+ __start___kflagstab = .; \
+ KEEP(*(SORT(___kflagstab+*))) \
+ __stop___kflagstab = .; \
+ } \
+ \
/* Kernel symbol table: strings */ \
__ksymtab_strings : AT(ADDR(__ksymtab_strings) - LOAD_OFFSET) { \
*(__ksymtab_strings) \
diff --git a/scripts/module.lds.S b/scripts/module.lds.S
index ee79c41059f3..9a8a3b6d1569 100644
--- a/scripts/module.lds.S
+++ b/scripts/module.lds.S
@@ -23,6 +23,7 @@ SECTIONS {
__ksymtab_gpl 0 : ALIGN(8) { *(SORT(___ksymtab_gpl+*)) }
__kcrctab 0 : ALIGN(4) { *(SORT(___kcrctab+*)) }
__kcrctab_gpl 0 : ALIGN(4) { *(SORT(___kcrctab_gpl+*)) }
+ __kflagstab 0 : ALIGN(1) { *(SORT(___kflagstab+*)) }
.ctors 0 : ALIGN(8) { *(SORT(.ctors.*)) *(.ctors) }
.init_array 0 : ALIGN(8) { *(SORT(.init_array.*)) *(.init_array) }
--
2.51.1.930.gacf6e81ea2-goog
^ permalink raw reply related
* [PATCH v3 3/8] modpost: create entries for kflagstab
From: Siddharth Nayyar @ 2025-11-03 16:19 UTC (permalink / raw)
To: petr.pavlu, corbet
Cc: arnd, linux-arch, linux-kbuild, linux-kernel, linux-modules,
mcgrof, nathan, nicolas.schier, samitolvanen, sidnayyar, maennich,
gprocida
In-Reply-To: <20251103161954.1351784-1-sidnayyar@google.com>
Signed-off-by: Siddharth Nayyar <sidnayyar@google.com>
Reviewed-by: Petr Pavlu <petr.pavlu@suse.com>
---
include/linux/export-internal.h | 7 +++++++
scripts/mod/modpost.c | 8 ++++++++
2 files changed, 15 insertions(+)
diff --git a/include/linux/export-internal.h b/include/linux/export-internal.h
index d445705ac13c..4123c7592404 100644
--- a/include/linux/export-internal.h
+++ b/include/linux/export-internal.h
@@ -69,4 +69,11 @@
".long " #crc "\n" \
".previous" "\n")
+#define SYMBOL_FLAGS(sym, flags) \
+ asm(" .section \"___kflagstab+" #sym "\",\"a\"" "\n" \
+ "__flags_" #sym ":" "\n" \
+ " .byte " #flags "\n" \
+ " .previous" "\n" \
+ )
+
#endif /* __LINUX_EXPORT_INTERNAL_H__ */
diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
index 5ca7c268294e..f5ce7aeed52d 100644
--- a/scripts/mod/modpost.c
+++ b/scripts/mod/modpost.c
@@ -244,6 +244,11 @@ static struct symbol *alloc_symbol(const char *name)
return s;
}
+static uint8_t get_symbol_flags(const struct symbol *sym)
+{
+ return sym->is_gpl_only ? KSYM_FLAG_GPL_ONLY : 0;
+}
+
/* For the hash of exported symbols */
static void hash_add_symbol(struct symbol *sym)
{
@@ -1865,6 +1870,9 @@ static void add_exported_symbols(struct buffer *buf, struct module *mod)
buf_printf(buf, "KSYMTAB_%s(%s, \"%s\", \"%s\");\n",
sym->is_func ? "FUNC" : "DATA", sym->name,
sym->is_gpl_only ? "_gpl" : "", sym->namespace);
+
+ buf_printf(buf, "SYMBOL_FLAGS(%s, 0x%02x);\n",
+ sym->name, get_symbol_flags(sym));
}
if (!modversions)
--
2.51.1.930.gacf6e81ea2-goog
^ permalink raw reply related
* [PATCH v3 4/8] module loader: use kflagstab instead of *_gpl sections
From: Siddharth Nayyar @ 2025-11-03 16:19 UTC (permalink / raw)
To: petr.pavlu, corbet
Cc: arnd, linux-arch, linux-kbuild, linux-kernel, linux-modules,
mcgrof, nathan, nicolas.schier, samitolvanen, sidnayyar, maennich,
gprocida
In-Reply-To: <20251103161954.1351784-1-sidnayyar@google.com>
Read __kflagstab section for vmlinux and modules to determine whether
kernel symbols are GPL only.
Signed-off-by: Siddharth Nayyar <sidnayyar@google.com>
Reviewed-by: Petr Pavlu <petr.pavlu@suse.com>
---
include/linux/module.h | 1 +
kernel/module/internal.h | 1 +
kernel/module/main.c | 55 +++++++++++++++++++++-------------------
3 files changed, 31 insertions(+), 26 deletions(-)
diff --git a/include/linux/module.h b/include/linux/module.h
index 3319a5269d28..9ba6ce433ac3 100644
--- a/include/linux/module.h
+++ b/include/linux/module.h
@@ -415,6 +415,7 @@ struct module {
/* Exported symbols */
const struct kernel_symbol *syms;
const u32 *crcs;
+ const u8 *flagstab;
unsigned int num_syms;
#ifdef CONFIG_ARCH_USES_CFI_TRAPS
diff --git a/kernel/module/internal.h b/kernel/module/internal.h
index 618202578b42..69b84510e097 100644
--- a/kernel/module/internal.h
+++ b/kernel/module/internal.h
@@ -57,6 +57,7 @@ extern const struct kernel_symbol __start___ksymtab_gpl[];
extern const struct kernel_symbol __stop___ksymtab_gpl[];
extern const u32 __start___kcrctab[];
extern const u32 __start___kcrctab_gpl[];
+extern const u8 __start___kflagstab[];
#define KMOD_PATH_LEN 256
extern char modprobe_path[];
diff --git a/kernel/module/main.c b/kernel/module/main.c
index c66b26184936..4197af526087 100644
--- a/kernel/module/main.c
+++ b/kernel/module/main.c
@@ -11,6 +11,7 @@
#include <linux/extable.h>
#include <linux/moduleloader.h>
#include <linux/module_signature.h>
+#include <linux/module_symbol.h>
#include <linux/trace_events.h>
#include <linux/init.h>
#include <linux/kallsyms.h>
@@ -87,7 +88,7 @@ struct mod_tree_root mod_tree __cacheline_aligned = {
struct symsearch {
const struct kernel_symbol *start, *stop;
const u32 *crcs;
- enum mod_license license;
+ const u8 *flagstab;
};
/*
@@ -364,19 +365,21 @@ static bool find_exported_symbol_in_section(const struct symsearch *syms,
struct find_symbol_arg *fsa)
{
struct kernel_symbol *sym;
-
- if (!fsa->gplok && syms->license == GPL_ONLY)
- return false;
+ u8 sym_flags;
sym = bsearch(fsa->name, syms->start, syms->stop - syms->start,
sizeof(struct kernel_symbol), cmp_name);
if (!sym)
return false;
+ sym_flags = *(syms->flagstab + (sym - syms->start));
+ if (!fsa->gplok && (sym_flags & KSYM_FLAG_GPL_ONLY))
+ return false;
+
fsa->owner = owner;
fsa->crc = symversion(syms->crcs, sym - syms->start);
fsa->sym = sym;
- fsa->license = syms->license;
+ fsa->license = (sym_flags & KSYM_FLAG_GPL_ONLY) ? GPL_ONLY : NOT_GPL_ONLY;
return true;
}
@@ -387,36 +390,31 @@ static bool find_exported_symbol_in_section(const struct symsearch *syms,
*/
bool find_symbol(struct find_symbol_arg *fsa)
{
- static const struct symsearch arr[] = {
- { __start___ksymtab, __stop___ksymtab, __start___kcrctab,
- NOT_GPL_ONLY },
- { __start___ksymtab_gpl, __stop___ksymtab_gpl,
- __start___kcrctab_gpl,
- GPL_ONLY },
+ const struct symsearch syms = {
+ .start = __start___ksymtab,
+ .stop = __stop___ksymtab,
+ .crcs = __start___kcrctab,
+ .flagstab = __start___kflagstab,
};
struct module *mod;
- unsigned int i;
- for (i = 0; i < ARRAY_SIZE(arr); i++)
- if (find_exported_symbol_in_section(&arr[i], NULL, fsa))
- return true;
+ if (find_exported_symbol_in_section(&syms, NULL, fsa))
+ return true;
list_for_each_entry_rcu(mod, &modules, list,
lockdep_is_held(&module_mutex)) {
- struct symsearch arr[] = {
- { mod->syms, mod->syms + mod->num_syms, mod->crcs,
- NOT_GPL_ONLY },
- { mod->gpl_syms, mod->gpl_syms + mod->num_gpl_syms,
- mod->gpl_crcs,
- GPL_ONLY },
+ const struct symsearch syms = {
+ .start = mod->syms,
+ .stop = mod->syms + mod->num_syms,
+ .crcs = mod->crcs,
+ .flagstab = mod->flagstab,
};
if (mod->state == MODULE_STATE_UNFORMED)
continue;
- for (i = 0; i < ARRAY_SIZE(arr); i++)
- if (find_exported_symbol_in_section(&arr[i], mod, fsa))
- return true;
+ if (find_exported_symbol_in_section(&syms, mod, fsa))
+ return true;
}
pr_debug("Failed to find symbol %s\n", fsa->name);
@@ -2607,6 +2605,7 @@ static int find_module_sections(struct module *mod, struct load_info *info)
sizeof(*mod->gpl_syms),
&mod->num_gpl_syms);
mod->gpl_crcs = section_addr(info, "__kcrctab_gpl");
+ mod->flagstab = section_addr(info, "__kflagstab");
#ifdef CONFIG_CONSTRUCTORS
mod->ctors = section_objs(info, ".ctors",
@@ -2810,8 +2809,12 @@ static int move_module(struct module *mod, struct load_info *info)
return ret;
}
-static int check_export_symbol_versions(struct module *mod)
+static int check_export_symbol_sections(struct module *mod)
{
+ if (mod->num_syms && !mod->flagstab) {
+ pr_err("%s: no flags for exported symbols\n", mod->name);
+ return -ENOEXEC;
+ }
#ifdef CONFIG_MODVERSIONS
if ((mod->num_syms && !mod->crcs) ||
(mod->num_gpl_syms && !mod->gpl_crcs)) {
@@ -3427,7 +3430,7 @@ static int load_module(struct load_info *info, const char __user *uargs,
if (err)
goto free_unload;
- err = check_export_symbol_versions(mod);
+ err = check_export_symbol_sections(mod);
if (err)
goto free_unload;
--
2.51.1.930.gacf6e81ea2-goog
^ permalink raw reply related
* [PATCH v3 5/8] modpost: put all exported symbols in ksymtab section
From: Siddharth Nayyar @ 2025-11-03 16:19 UTC (permalink / raw)
To: petr.pavlu, corbet
Cc: arnd, linux-arch, linux-kbuild, linux-kernel, linux-modules,
mcgrof, nathan, nicolas.schier, samitolvanen, sidnayyar, maennich,
gprocida
In-Reply-To: <20251103161954.1351784-1-sidnayyar@google.com>
Since the modules loader determines whether an exported symbol is GPL
only from the kflagstab section data, modpost can put all symbols in the
regular ksymtab and stop using the *_gpl versions of the ksymtab and
kcrctab.
Signed-off-by: Siddharth Nayyar <sidnayyar@google.com>
Reviewed-by: Petr Pavlu <petr.pavlu@suse.com>
---
include/linux/export-internal.h | 21 +++++++++++----------
scripts/mod/modpost.c | 8 ++++----
2 files changed, 15 insertions(+), 14 deletions(-)
diff --git a/include/linux/export-internal.h b/include/linux/export-internal.h
index 4123c7592404..726054614752 100644
--- a/include/linux/export-internal.h
+++ b/include/linux/export-internal.h
@@ -37,14 +37,14 @@
* section flag requires it. Use '%progbits' instead of '@progbits' since the
* former apparently works on all arches according to the binutils source.
*/
-#define __KSYMTAB(name, sym, sec, ns) \
+#define __KSYMTAB(name, sym, ns) \
asm(" .section \"__ksymtab_strings\",\"aMS\",%progbits,1" "\n" \
"__kstrtab_" #name ":" "\n" \
" .asciz \"" #name "\"" "\n" \
"__kstrtabns_" #name ":" "\n" \
" .asciz \"" ns "\"" "\n" \
" .previous" "\n" \
- " .section \"___ksymtab" sec "+" #name "\", \"a\"" "\n" \
+ " .section \"___ksymtab+" #name "\", \"a\"" "\n" \
__KSYM_ALIGN "\n" \
"__ksymtab_" #name ":" "\n" \
__KSYM_REF(sym) "\n" \
@@ -59,15 +59,16 @@
#define KSYM_FUNC(name) name
#endif
-#define KSYMTAB_FUNC(name, sec, ns) __KSYMTAB(name, KSYM_FUNC(name), sec, ns)
-#define KSYMTAB_DATA(name, sec, ns) __KSYMTAB(name, name, sec, ns)
+#define KSYMTAB_FUNC(name, ns) __KSYMTAB(name, KSYM_FUNC(name), ns)
+#define KSYMTAB_DATA(name, ns) __KSYMTAB(name, name, ns)
-#define SYMBOL_CRC(sym, crc, sec) \
- asm(".section \"___kcrctab" sec "+" #sym "\",\"a\"" "\n" \
- ".balign 4" "\n" \
- "__crc_" #sym ":" "\n" \
- ".long " #crc "\n" \
- ".previous" "\n")
+#define SYMBOL_CRC(sym, crc) \
+ asm(" .section \"___kcrctab+" #sym "\",\"a\"" "\n" \
+ " .balign 4" "\n" \
+ "__crc_" #sym ":" "\n" \
+ " .long " #crc "\n" \
+ " .previous" "\n" \
+ )
#define SYMBOL_FLAGS(sym, flags) \
asm(" .section \"___kflagstab+" #sym "\",\"a\"" "\n" \
diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
index f5ce7aeed52d..8936db84779b 100644
--- a/scripts/mod/modpost.c
+++ b/scripts/mod/modpost.c
@@ -1867,9 +1867,9 @@ static void add_exported_symbols(struct buffer *buf, struct module *mod)
if (trim_unused_exports && !sym->used)
continue;
- buf_printf(buf, "KSYMTAB_%s(%s, \"%s\", \"%s\");\n",
+ buf_printf(buf, "KSYMTAB_%s(%s, \"%s\");\n",
sym->is_func ? "FUNC" : "DATA", sym->name,
- sym->is_gpl_only ? "_gpl" : "", sym->namespace);
+ sym->namespace);
buf_printf(buf, "SYMBOL_FLAGS(%s, 0x%02x);\n",
sym->name, get_symbol_flags(sym));
@@ -1890,8 +1890,8 @@ static void add_exported_symbols(struct buffer *buf, struct module *mod)
sym->name, mod->name, mod->is_vmlinux ? "" : ".ko",
sym->name);
- buf_printf(buf, "SYMBOL_CRC(%s, 0x%08x, \"%s\");\n",
- sym->name, sym->crc, sym->is_gpl_only ? "_gpl" : "");
+ buf_printf(buf, "SYMBOL_CRC(%s, 0x%08x);\n",
+ sym->name, sym->crc);
}
}
--
2.51.1.930.gacf6e81ea2-goog
^ permalink raw reply related
* [PATCH v3 6/8] module loader: remove references of *_gpl sections
From: Siddharth Nayyar @ 2025-11-03 16:19 UTC (permalink / raw)
To: petr.pavlu, corbet
Cc: arnd, linux-arch, linux-kbuild, linux-kernel, linux-modules,
mcgrof, nathan, nicolas.schier, samitolvanen, sidnayyar, maennich,
gprocida
In-Reply-To: <20251103161954.1351784-1-sidnayyar@google.com>
The *_gpl section are not being used populated by modpost anymore. Hence
the module loader doesn't need to find and process these sections in
modules.
Signed-off-by: Siddharth Nayyar <sidnayyar@google.com>
Reviewed-by: Petr Pavlu <petr.pavlu@suse.com>
---
include/linux/module.h | 3 ---
kernel/module/internal.h | 3 ---
kernel/module/main.c | 47 ++++++++++++++++------------------------
3 files changed, 19 insertions(+), 34 deletions(-)
diff --git a/include/linux/module.h b/include/linux/module.h
index 9ba6ce433ac3..1a9c41318e22 100644
--- a/include/linux/module.h
+++ b/include/linux/module.h
@@ -431,9 +431,6 @@ struct module {
unsigned int num_kp;
/* GPL-only exported symbols. */
- unsigned int num_gpl_syms;
- const struct kernel_symbol *gpl_syms;
- const u32 *gpl_crcs;
bool using_gplonly_symbols;
#ifdef CONFIG_MODULE_SIG
diff --git a/kernel/module/internal.h b/kernel/module/internal.h
index 69b84510e097..061161cc79d9 100644
--- a/kernel/module/internal.h
+++ b/kernel/module/internal.h
@@ -53,10 +53,7 @@ extern const size_t modinfo_attrs_count;
/* Provided by the linker */
extern const struct kernel_symbol __start___ksymtab[];
extern const struct kernel_symbol __stop___ksymtab[];
-extern const struct kernel_symbol __start___ksymtab_gpl[];
-extern const struct kernel_symbol __stop___ksymtab_gpl[];
extern const u32 __start___kcrctab[];
-extern const u32 __start___kcrctab_gpl[];
extern const u8 __start___kflagstab[];
#define KMOD_PATH_LEN 256
diff --git a/kernel/module/main.c b/kernel/module/main.c
index 4197af526087..f5f9872dc070 100644
--- a/kernel/module/main.c
+++ b/kernel/module/main.c
@@ -1464,29 +1464,18 @@ EXPORT_SYMBOL_GPL(__symbol_get);
*/
static int verify_exported_symbols(struct module *mod)
{
- unsigned int i;
const struct kernel_symbol *s;
- struct {
- const struct kernel_symbol *sym;
- unsigned int num;
- } arr[] = {
- { mod->syms, mod->num_syms },
- { mod->gpl_syms, mod->num_gpl_syms },
- };
-
- for (i = 0; i < ARRAY_SIZE(arr); i++) {
- for (s = arr[i].sym; s < arr[i].sym + arr[i].num; s++) {
- struct find_symbol_arg fsa = {
- .name = kernel_symbol_name(s),
- .gplok = true,
- };
- if (find_symbol(&fsa)) {
- pr_err("%s: exports duplicate symbol %s"
- " (owned by %s)\n",
- mod->name, kernel_symbol_name(s),
- module_name(fsa.owner));
- return -ENOEXEC;
- }
+ for (s = mod->syms; s < mod->syms + mod->num_syms; s++) {
+ struct find_symbol_arg fsa = {
+ .name = kernel_symbol_name(s),
+ .gplok = true,
+ };
+ if (find_symbol(&fsa)) {
+ pr_err("%s: exports duplicate symbol %s"
+ " (owned by %s)\n",
+ mod->name, kernel_symbol_name(s),
+ module_name(fsa.owner));
+ return -ENOEXEC;
}
}
return 0;
@@ -2601,12 +2590,15 @@ static int find_module_sections(struct module *mod, struct load_info *info)
mod->syms = section_objs(info, "__ksymtab",
sizeof(*mod->syms), &mod->num_syms);
mod->crcs = section_addr(info, "__kcrctab");
- mod->gpl_syms = section_objs(info, "__ksymtab_gpl",
- sizeof(*mod->gpl_syms),
- &mod->num_gpl_syms);
- mod->gpl_crcs = section_addr(info, "__kcrctab_gpl");
mod->flagstab = section_addr(info, "__kflagstab");
+ if (section_addr(info, "__ksymtab_gpl"))
+ pr_warn("%s: ignoring obsolete section __ksymtab_gpl\n",
+ mod->name);
+ if (section_addr(info, "__kcrctab_gpl"))
+ pr_warn("%s: ignoring obsolete section __kcrctab_gpl\n",
+ mod->name);
+
#ifdef CONFIG_CONSTRUCTORS
mod->ctors = section_objs(info, ".ctors",
sizeof(*mod->ctors), &mod->num_ctors);
@@ -2816,8 +2808,7 @@ static int check_export_symbol_sections(struct module *mod)
return -ENOEXEC;
}
#ifdef CONFIG_MODVERSIONS
- if ((mod->num_syms && !mod->crcs) ||
- (mod->num_gpl_syms && !mod->gpl_crcs)) {
+ if (mod->num_syms && !mod->crcs) {
return try_to_force_load(mod,
"no versions for exported symbols");
}
--
2.51.1.930.gacf6e81ea2-goog
^ permalink raw reply related
* [PATCH v3 7/8] linker: remove *_gpl sections from vmlinux and modules
From: Siddharth Nayyar @ 2025-11-03 16:19 UTC (permalink / raw)
To: petr.pavlu, corbet
Cc: arnd, linux-arch, linux-kbuild, linux-kernel, linux-modules,
mcgrof, nathan, nicolas.schier, samitolvanen, sidnayyar, maennich,
gprocida
In-Reply-To: <20251103161954.1351784-1-sidnayyar@google.com>
These sections are not used anymore and can be removed from vmlinux and
modules.
Signed-off-by: Siddharth Nayyar <sidnayyar@google.com>
Reviewed-by: Petr Pavlu <petr.pavlu@suse.com>
---
include/asm-generic/vmlinux.lds.h | 18 ++----------------
scripts/module.lds.S | 2 --
2 files changed, 2 insertions(+), 18 deletions(-)
diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
index 310e2de56211..6490b93d23b1 100644
--- a/include/asm-generic/vmlinux.lds.h
+++ b/include/asm-generic/vmlinux.lds.h
@@ -490,34 +490,20 @@ defined(CONFIG_AUTOFDO_CLANG) || defined(CONFIG_PROPELLER_CLANG)
\
PRINTK_INDEX \
\
- /* Kernel symbol table: Normal symbols */ \
+ /* Kernel symbol table */ \
__ksymtab : AT(ADDR(__ksymtab) - LOAD_OFFSET) { \
__start___ksymtab = .; \
KEEP(*(SORT(___ksymtab+*))) \
__stop___ksymtab = .; \
} \
\
- /* Kernel symbol table: GPL-only symbols */ \
- __ksymtab_gpl : AT(ADDR(__ksymtab_gpl) - LOAD_OFFSET) { \
- __start___ksymtab_gpl = .; \
- KEEP(*(SORT(___ksymtab_gpl+*))) \
- __stop___ksymtab_gpl = .; \
- } \
- \
- /* Kernel symbol table: Normal symbols */ \
+ /* Kernel symbol CRC table */ \
__kcrctab : AT(ADDR(__kcrctab) - LOAD_OFFSET) { \
__start___kcrctab = .; \
KEEP(*(SORT(___kcrctab+*))) \
__stop___kcrctab = .; \
} \
\
- /* Kernel symbol table: GPL-only symbols */ \
- __kcrctab_gpl : AT(ADDR(__kcrctab_gpl) - LOAD_OFFSET) { \
- __start___kcrctab_gpl = .; \
- KEEP(*(SORT(___kcrctab_gpl+*))) \
- __stop___kcrctab_gpl = .; \
- } \
- \
/* Kernel symbol flags table */ \
__kflagstab : AT(ADDR(__kflagstab) - LOAD_OFFSET) { \
__start___kflagstab = .; \
diff --git a/scripts/module.lds.S b/scripts/module.lds.S
index 9a8a3b6d1569..1841a43d8771 100644
--- a/scripts/module.lds.S
+++ b/scripts/module.lds.S
@@ -20,9 +20,7 @@ SECTIONS {
}
__ksymtab 0 : ALIGN(8) { *(SORT(___ksymtab+*)) }
- __ksymtab_gpl 0 : ALIGN(8) { *(SORT(___ksymtab_gpl+*)) }
__kcrctab 0 : ALIGN(4) { *(SORT(___kcrctab+*)) }
- __kcrctab_gpl 0 : ALIGN(4) { *(SORT(___kcrctab_gpl+*)) }
__kflagstab 0 : ALIGN(1) { *(SORT(___kflagstab+*)) }
.ctors 0 : ALIGN(8) { *(SORT(.ctors.*)) *(.ctors) }
--
2.51.1.930.gacf6e81ea2-goog
^ permalink raw reply related
* [PATCH v3 8/8] remove references to *_gpl sections in documentation
From: Siddharth Nayyar @ 2025-11-03 16:19 UTC (permalink / raw)
To: petr.pavlu, corbet
Cc: arnd, linux-arch, linux-kbuild, linux-kernel, linux-modules,
mcgrof, nathan, nicolas.schier, samitolvanen, sidnayyar, maennich,
gprocida
In-Reply-To: <20251103161954.1351784-1-sidnayyar@google.com>
Signed-off-by: Siddharth Nayyar <sidnayyar@google.com>
Reviewed-by: Petr Pavlu <petr.pavlu@suse.com>
---
Documentation/kbuild/modules.rst | 11 ++++++-----
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/Documentation/kbuild/modules.rst b/Documentation/kbuild/modules.rst
index d0703605bfa4..b3a26a36ee17 100644
--- a/Documentation/kbuild/modules.rst
+++ b/Documentation/kbuild/modules.rst
@@ -426,11 +426,12 @@ Symbols From the Kernel (vmlinux + modules)
Version Information Formats
---------------------------
- Exported symbols have information stored in __ksymtab or __ksymtab_gpl
- sections. Symbol names and namespaces are stored in __ksymtab_strings,
- using a format similar to the string table used for ELF. If
- CONFIG_MODVERSIONS is enabled, the CRCs corresponding to exported
- symbols will be added to the __kcrctab or __kcrctab_gpl.
+ Exported symbols have information stored in the __ksymtab and
+ __kflagstab sections. Symbol names and namespaces are stored in
+ __ksymtab_strings section, using a format similar to the string
+ table used for ELF. If CONFIG_MODVERSIONS is enabled, the CRCs
+ corresponding to exported symbols will be added to the
+ __kcrctab section.
If CONFIG_BASIC_MODVERSIONS is enabled (default with
CONFIG_MODVERSIONS), imported symbols will have their symbol name and
--
2.51.1.930.gacf6e81ea2-goog
^ permalink raw reply related
* Re: [PATCH v3 22/28] x86: Use RCU in all users of __module_address().
From: Michal Pecio @ 2025-11-03 17:37 UTC (permalink / raw)
To: Sebastian Andrzej Siewior
Cc: bp, da.gomez, dave.hansen, hpa, jpoimboe, linux-kernel,
linux-modules, mcgrof, mingo, paulmck, peterz, petr.pavlu,
samitolvanen, tglx, x86
In-Reply-To: <20251103113750.qam3KIkT@linutronix.de>
On Mon, 3 Nov 2025 12:37:50 +0100, Sebastian Andrzej Siewior wrote:
> Now, get_stack_info() where the warning originates: It starts with a
> check to see if the stack pointer belongs to the current task's stack
> frame which it does not. Then it checks if the task found is the
> currently running task. That it does. So in that case, we must be
> serving an exception (such as an IRQ) because the stack does not
> belong to the current task. However preemption is not disabled which
> indicates that we do not do this.
> This in turn suggests that nvidia replaced the stack from while
> entering the syscall probably in _nv003168kms() or the binary blob
> which invokes the kernel function does not have a proper ORC entry
> which leads to a wrong turn in the process.
OK, I see, preemption should only be enabled in the first case, so
others are free to assume it's disabled. No bug.
Thank you.
^ permalink raw reply
* Re: [PATCH v2 0/3] module: Add compile-time check for embedded NUL characters
From: Daniel Gomez @ 2025-11-03 19:49 UTC (permalink / raw)
To: Luis Chamberlain, Kees Cook
Cc: Daniel Gomez, Hans Verkuil, Malcolm Priestley,
Mauro Carvalho Chehab, Hans Verkuil, Uwe Kleine-König,
Rusty Russell, Petr Pavlu, Sami Tolvanen, linux-kernel,
linux-media, linux-modules, linux-hardening
In-Reply-To: <20251010030348.it.784-kees@kernel.org>
On Thu, 09 Oct 2025 20:06:06 -0700, Kees Cook wrote:
> v2:
> - use static_assert instead of _Static_assert
> - add Hans's Reviewed-by's
> v1: https://lore.kernel.org/lkml/20251008033844.work.801-kees@kernel.org/
>
> Hi!
>
> [...]
Applied patch 3, thanks!
[3/3] module: Add compile-time check for embedded NUL characters
commit: 913359754ea821c4d6f6a77e0449b29984099663
Best regards,
--
Daniel Gomez <da.gomez@samsung.com>
^ permalink raw reply
* Re: [PATCH v2 0/3] module: Add compile-time check for embedded NUL characters
From: Kees Cook @ 2025-11-04 0:13 UTC (permalink / raw)
To: Daniel Gomez
Cc: Luis Chamberlain, Hans Verkuil, Malcolm Priestley,
Mauro Carvalho Chehab, Hans Verkuil, Uwe Kleine-König,
Rusty Russell, Petr Pavlu, Sami Tolvanen, linux-kernel,
linux-media, linux-modules, linux-hardening
In-Reply-To: <176219902728.2668573.8447418880394997824.b4-ty@kernel.org>
On Mon, Nov 03, 2025 at 08:49:43PM +0100, Daniel Gomez wrote:
>
> On Thu, 09 Oct 2025 20:06:06 -0700, Kees Cook wrote:
> > v2:
> > - use static_assert instead of _Static_assert
> > - add Hans's Reviewed-by's
> > v1: https://lore.kernel.org/lkml/20251008033844.work.801-kees@kernel.org/
> >
> > Hi!
> >
> > [...]
>
> Applied patch 3, thanks!
>
> [3/3] module: Add compile-time check for embedded NUL characters
> commit: 913359754ea821c4d6f6a77e0449b29984099663
I'm nervous about this going in alone -- it breaks allmodconfig builds
without the media fixes. My intention was to have the media fixes land
first...
Should I send the media fixes to linus right away?
-Kees
--
Kees Cook
^ permalink raw reply
* Re: [PATCH] module: Only declare set_module_sig_enforced when CONFIG_MODULE_SIG=y
From: Coiby Xu @ 2025-11-04 3:04 UTC (permalink / raw)
To: Daniel Gomez
Cc: linux-modules, linux-integrity, kernel test robot,
Luis Chamberlain, Petr Pavlu, Sami Tolvanen,
open list:MODULE SUPPORT
In-Reply-To: <3bf85718-8cea-4982-944d-b4c7a4faaf8f@kernel.org>
On Sat, Nov 01, 2025 at 11:10:51PM +0100, Daniel Gomez wrote:
>On 31/10/2025 09.09, Coiby Xu wrote:
>> Currently, set_module_sig_enforced is declared as long as CONFIG_MODULES
>> is enabled. This can lead to a linking error if
>> set_module_sig_enforced is called with CONFIG_MODULE_SIG=n,
>>
>> ld: security/integrity/ima/ima_appraise.o: in function `ima_appraise_measurement':
>> security/integrity/ima/ima_appraise.c:587:(.text+0xbbb): undefined reference to `set_module_sig_enforced'
>
>It's a bit unclear whether you're referring to a current upstream issue (which I
>couldn't find as of -rc3), or if this is just a hypothetical scenario.
Hi Daniel,
Yes, this issue is hypothetical and currently doesn't cause any real
trouble. lkp found this issue in one of my proposed patches
https://lore.kernel.org/lkml/20250928030358.3873311-1-coxu@redhat.com/
But I'll use a different solution so the above patch will be abandoned
and will not be applied.
>
>>
>> So only declare set_module_sig_enforced when CONFIG_MODULE_SIG is
>> enabled.
>
>I only see cases where code has a safeguard like in
>security/integrity/ima/ima_efi.c:71
>
> if (IS_ENABLED(CONFIG_MODULE_SIG))
> set_module_sig_enforced();
>
>>
>> Note this issue hasn't caused a real problem because all current callers
>> of set_module_sig_enforced e.g. security/integrity/ima/ima_efi.c
>> depend on CONFIG_MODULE_SIG=y.
>
>I think the correct term we should use here is runtime safeguard. The code does
>not actually depend on that config, nor is there any dep in Kconfig.
Thanks for correcting me! Sorry I didn't realize the constant folding
compiler optimization and made a false claim while forgetting the fact
security/integrity/ima/ima_efi.c also explicitly use
"#if !IS_ENABLED(CONFIG_MODULE_SIG)".
>
>>
>> Reported-by: kernel test robot <lkp@intel.com>
>> Closes: https://lore.kernel.org/oe-kbuild-all/202510030029.VRKgik99-lkp@intel.com/
>> Signed-off-by: Coiby Xu <coxu@redhat.com>
>
>
>Just minor nits regarding the commit message structure. This change should allow
>us to remove the safeguard from users of set_module_sig_enforced().
Thanks for the suggestion! Does the following commit address address you
concern?
module: Only declare set_module_sig_enforced when CONFIG_MODULE_SIG=y
Currently if set_module_sig_enforced is called with CONFIG_MODULE_SIG=n
e.g. [1], it can lead to a linking error,
ld: security/integrity/ima/ima_appraise.o: in function `ima_appraise_measurement':
security/integrity/ima/ima_appraise.c:587:(.text+0xbbb): undefined reference to `set_module_sig_enforced'
This happens because the actual implementation of
set_module_sig_enforced comes from CONFIG_MODULE_SIG but both the
function declaration and the empty stub definition are tied to
CONFIG_MODULES.
So bind set_module_sig_enforced to CONFIG_MODULE_SIG instead. This
allows (future) users to call set_module_sig_enforced directly without
the "if IS_ENABLED(CONFIG_MODULE_SIG)" safeguard.
Note this issue hasn't caused a real problem because all current callers
of set_module_sig_enforced e.g. security/integrity/ima/ima_efi.c
use "if IS_ENABLED(CONFIG_MODULE_SIG)" safeguard.
[1] https://lore.kernel.org/lkml/20250928030358.3873311-1-coxu@redhat.com/
Reported-by: kernel test robot <lkp@intel.com>
Closes: https://lore.kernel.org/oe-kbuild-all/202510030029.VRKgik99-lkp@intel.com/
>
>
>Other than that, LGTM,
>
>Reviewed-by: Daniel Gomez <da.gomez@samsung.com>
Thanks for reviewing the patch!
>
--
Best regards,
Coiby
^ permalink raw reply
* Re: [PATCH] module: Only declare set_module_sig_enforced when CONFIG_MODULE_SIG=y
From: Coiby Xu @ 2025-11-04 3:05 UTC (permalink / raw)
To: Aaron Tomlin
Cc: linux-modules, linux-integrity, kernel test robot,
Luis Chamberlain, Petr Pavlu, Daniel Gomez, Sami Tolvanen,
open list:MODULE SUPPORT
In-Reply-To: <w4vqvjighjl33a32gvwnu7xlzmp6yztx42gbjixrhic3wt34j6@5flsq5omspn7>
On Fri, Oct 31, 2025 at 04:09:27PM -0400, Aaron Tomlin wrote:
>On Fri, Oct 31, 2025 at 04:09:48PM +0800, Coiby Xu wrote:
>> Currently, set_module_sig_enforced is declared as long as CONFIG_MODULES
>> is enabled. This can lead to a linking error if
>> set_module_sig_enforced is called with CONFIG_MODULE_SIG=n,
>>
>> ld: security/integrity/ima/ima_appraise.o: in function `ima_appraise_measurement':
>> security/integrity/ima/ima_appraise.c:587:(.text+0xbbb): undefined reference to `set_module_sig_enforced'
>>
>> So only declare set_module_sig_enforced when CONFIG_MODULE_SIG is
>> enabled.
>>
>> Note this issue hasn't caused a real problem because all current callers
>> of set_module_sig_enforced e.g. security/integrity/ima/ima_efi.c
>> depend on CONFIG_MODULE_SIG=y.
>>
>> Reported-by: kernel test robot <lkp@intel.com>
>> Closes: https://lore.kernel.org/oe-kbuild-all/202510030029.VRKgik99-lkp@intel.com/
>> Signed-off-by: Coiby Xu <coxu@redhat.com>
>> ---
>> include/linux/module.h | 12 ++++++------
>> 1 file changed, 6 insertions(+), 6 deletions(-)
>>
>> diff --git a/include/linux/module.h b/include/linux/module.h
>> index e135cc79acee..fa251958b138 100644
>> --- a/include/linux/module.h
>> +++ b/include/linux/module.h
>> @@ -769,8 +769,6 @@ static inline bool is_livepatch_module(struct module *mod)
>> #endif
>> }
>>
>> -void set_module_sig_enforced(void);
>> -
>> void module_for_each_mod(int(*func)(struct module *mod, void *data), void *data);
>>
>> #else /* !CONFIG_MODULES... */
>> @@ -865,10 +863,6 @@ static inline bool module_requested_async_probing(struct module *module)
>> }
>>
>>
>> -static inline void set_module_sig_enforced(void)
>> -{
>> -}
>> -
>> /* Dereference module function descriptor */
>> static inline
>> void *dereference_module_function_descriptor(struct module *mod, void *ptr)
>> @@ -924,6 +918,8 @@ static inline bool retpoline_module_ok(bool has_retpoline)
>> #ifdef CONFIG_MODULE_SIG
>> bool is_module_sig_enforced(void);
>>
>> +void set_module_sig_enforced(void);
>> +
>> static inline bool module_sig_ok(struct module *module)
>> {
>> return module->sig_ok;
>> @@ -934,6 +930,10 @@ static inline bool is_module_sig_enforced(void)
>> return false;
>> }
>>
>> +static inline void set_module_sig_enforced(void)
>> +{
>> +}
>> +
>> static inline bool module_sig_ok(struct module *module)
>> {
>> return true;
>>
>> base-commit: e53642b87a4f4b03a8d7e5f8507fc3cd0c595ea6
>> --
>> 2.51.0
>>
>>
>
>Looks good to me.
>
>Reviewed-by: Aaron Tomlin <atomlin@atomlin.com>
Hi Aaron,
Thanks for reviewing the patch!
>
>--
>Aaron Tomlin
>
--
Best regards,
Coiby
^ permalink raw reply
* Re: [PATCH v2 0/3] module: Add compile-time check for embedded NUL characters
From: Daniel Gomez @ 2025-11-04 6:35 UTC (permalink / raw)
To: Kees Cook, Hans Verkuil, Mauro Carvalho Chehab
Cc: Luis Chamberlain, Malcolm Priestley, Hans Verkuil,
Uwe Kleine-König, Rusty Russell, Petr Pavlu, Sami Tolvanen,
linux-kernel, linux-media, linux-modules, linux-hardening
In-Reply-To: <202511031612.8A05E2FD1C@keescook>
On 04/11/2025 01.13, Kees Cook wrote:
> On Mon, Nov 03, 2025 at 08:49:43PM +0100, Daniel Gomez wrote:
>>
>> On Thu, 09 Oct 2025 20:06:06 -0700, Kees Cook wrote:
>>> v2:
>>> - use static_assert instead of _Static_assert
>>> - add Hans's Reviewed-by's
>>> v1: https://lore.kernel.org/lkml/20251008033844.work.801-kees@kernel.org/
>>>
>>> Hi!
>>>
>>> [...]
>>
>> Applied patch 3, thanks!
>>
>> [3/3] module: Add compile-time check for embedded NUL characters
>> commit: 913359754ea821c4d6f6a77e0449b29984099663
>
> I'm nervous about this going in alone -- it breaks allmodconfig builds
> without the media fixes. My intention was to have the media fixes land
> first...
>
> Should I send the media fixes to linus right away?
>
> -Kees
>
I can take both patches. But I think it'd make sense to drop patch 3 first and
then, apply all 3.
Please, Kees, Hans and Mauro, let me know if this is okay with you.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox