* [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
* Re: [PATCH v2 0/3] module: Add compile-time check for embedded NUL characters
From: Hans Verkuil @ 2025-11-04 10:35 UTC (permalink / raw)
To: Daniel Gomez, Kees Cook, 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: <ab56339a-8736-4d68-bf11-d27c8d591597@kernel.org>
On 04/11/2025 07:35, Daniel Gomez wrote:
>
>
> 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.
I'm fine. If you take it, then I'll drop the media patches from our tree (I merged the
media patches yesterday).
Let me know if you take them.
Regards,
Hans
^ permalink raw reply
* Re: [PATCH v2 0/3] module: Add compile-time check for embedded NUL characters
From: Daniel Gomez @ 2025-11-04 12:03 UTC (permalink / raw)
To: Hans Verkuil, Kees Cook, 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: <5dba319f-56bc-40bf-9137-acb90f3cc754@kernel.org>
On 04/11/2025 11.35, Hans Verkuil wrote:
> On 04/11/2025 07:35, Daniel Gomez wrote:
>>
>>
>> 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.
>
> I'm fine. If you take it, then I'll drop the media patches from our tree (I merged the
> media patches yesterday).
>
> Let me know if you take them.
Thanks, Hans. I merged patch 3 yesterday as well, but since patch order matters
in this case, it makes sense to take all of them through the modules tree.
Sorry for the trouble, and thanks Kees, for pointing this out.
>
> Regards,
>
> Hans
^ permalink raw reply
* Re: [PATCH v2 0/3] module: Add compile-time check for embedded NUL characters
From: Daniel Gomez @ 2025-11-04 20:35 UTC (permalink / raw)
To: Hans Verkuil, Kees Cook, 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: <032ef71c-fcbd-42a9-98ea-b2568d663978@kernel.org>
On 04/11/2025 13.03, Daniel Gomez wrote:
>
>
> On 04/11/2025 11.35, Hans Verkuil wrote:
>> On 04/11/2025 07:35, Daniel Gomez wrote:
>>>
>>>
>>> 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.
>>
>> I'm fine. If you take it, then I'll drop the media patches from our tree (I merged the
>> media patches yesterday).
>>
>> Let me know if you take them.
>
> Thanks, Hans. I merged patch 3 yesterday as well, but since patch order matters
> in this case, it makes sense to take all of them through the modules tree.
>
> Sorry for the trouble, and thanks Kees, for pointing this out.
Kees,
FYI, I have dropped patch 3 from modules. My intention is to merge all 3 patches
tomorrow.
I believe Hans has also dropped the patches from the media tree as I do not see
them here: https://git.linuxtv.org/media.git/log/
>
>>
>> Regards,
>>
>> Hans
^ permalink raw reply
* Re: [PATCH v2 0/3] module: Add compile-time check for embedded NUL characters
From: Hans Verkuil @ 2025-11-04 20:59 UTC (permalink / raw)
To: Daniel Gomez, Kees Cook, 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: <1eecc666-ac73-425b-9d11-676fce70592a@kernel.org>
On 04/11/2025 21:35, Daniel Gomez wrote:
>
>
> On 04/11/2025 13.03, Daniel Gomez wrote:
>>
>>
>> On 04/11/2025 11.35, Hans Verkuil wrote:
>>> On 04/11/2025 07:35, Daniel Gomez wrote:
>>>>
>>>>
>>>> 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.
>>>
>>> I'm fine. If you take it, then I'll drop the media patches from our tree (I merged the
>>> media patches yesterday).
>>>
>>> Let me know if you take them.
>>
>> Thanks, Hans. I merged patch 3 yesterday as well, but since patch order matters
>> in this case, it makes sense to take all of them through the modules tree.
>>
>> Sorry for the trouble, and thanks Kees, for pointing this out.
>
> Kees,
>
> FYI, I have dropped patch 3 from modules. My intention is to merge all 3 patches
> tomorrow.
>
> I believe Hans has also dropped the patches from the media tree as I do not see
> them here: https://git.linuxtv.org/media.git/log/
Correct, I dropped them. They are all yours :-)
Regards,
Hans
^ permalink raw reply
* Re: [PATCH v2] lsm,ima: new LSM hook security_kernel_module_read_file to access decompressed kernel module
From: Coiby Xu @ 2025-11-05 0:18 UTC (permalink / raw)
To: Paul Moore, Mimi Zohar
Cc: linux-integrity, linux-security-module, Karel Srot, James Morris,
Serge E. Hallyn, Luis Chamberlain, Petr Pavlu, Daniel Gomez,
Sami Tolvanen, Roberto Sassu, Dmitry Kasatkin, Eric Snowberg,
open list, open list:MODULE SUPPORT
In-Reply-To: <CAHC9VhToe-VNqbh6TY2iYnRvqTHRfQjnHYSRWYgt8K7NcLKMdg@mail.gmail.com>
On Sun, Nov 02, 2025 at 10:43:04AM -0500, Paul Moore wrote:
>On Sun, Nov 2, 2025 at 10:06 AM Mimi Zohar <zohar@linux.ibm.com> wrote:
>> On Sat, 2025-11-01 at 12:50 -0400, Paul Moore wrote:
>> > On Fri, Oct 31, 2025 at 3:41 AM Coiby Xu <coxu@redhat.com> wrote:
>> > >
>> > > Currently, when in-kernel module decompression (CONFIG_MODULE_DECOMPRESS)
>> > > is enabled, IMA has no way to verify the appended module signature as it
>> > > can't decompress the module.
>> > >
>> > > Define a new LSM hook security_kernel_module_read_file which will be
>> > > called after kernel module decompression is done so IMA can access the
>> > > decompressed kernel module to verify the appended signature.
>> > >
>> > > Since IMA can access both xattr and appended kernel module signature
>> > > with the new LSM hook, it no longer uses the security_kernel_post_read_file
>> > > LSM hook for kernel module loading.
>> > >
>> > > Before enabling in-kernel module decompression, a kernel module in
>> > > initramfs can still be loaded with ima_policy=secure_boot. So adjust the
>> > > kernel module rule in secure_boot policy to allow either an IMA
>> > > signature OR an appended signature i.e. to use
>> > > "appraise func=MODULE_CHECK appraise_type=imasig|modsig".
>> > >
>> > > Reported-by: Karel Srot <ksrot@redhat.com>
>> > > Suggested-by: Mimi Zohar <zohar@linux.ibm.com>
>> > > Signed-off-by: Coiby Xu <coxu@redhat.com>
>> > > ---
>> > > v1: https://lore.kernel.org/linux-integrity/20250928030358.3873311-1-coxu@redhat.com/
>> > >
>> > > include/linux/lsm_hook_defs.h | 2 ++
>> > > include/linux/security.h | 7 +++++++
>> > > kernel/module/main.c | 10 +++++++++-
>> > > security/integrity/ima/ima_main.c | 26 ++++++++++++++++++++++++++
>> > > security/integrity/ima/ima_policy.c | 2 +-
>> > > security/security.c | 17 +++++++++++++++++
>> > > 6 files changed, 62 insertions(+), 2 deletions(-)
>> >
>> > We don't really need a new LSM hook for this do we? Can't we just
>> > define a new file read type, e.g. READING_MODULE_DECOMPRESS, and do
>> > another call to security_kernel_post_read_file() after the module is
>> > unpacked? Something like the snippet below ...
>>
>> Yes, this is similar to my suggestion based on defining multiple enumerations:
>> READING_MODULE, READING_COMPRESSED_MODULE, and READING_DECOMPRESSED_MODULE.
>> With this solution, IMA would need to make an exception in the post kernel
>> module read for the READING_COMPRESSED_MODULE case, since the kernel module has
>> not yet been decompressed.
>>
>> Coiby suggested further simplification by moving the call later. At which point
>> either there is or isn't an appended signature for non-compressed and
>> decompressed kernel modules.
>>
>> As long as you don't have a problem calling the security_kernel_post_read_file()
>> hook again, could we move the call later and pass READING_MODULE_UNCOMPRESSED?
>
>It isn't clear from these comments if you are talking about moving
>only the second security_kernel_post_read_file() call that was
>proposed for init_module_from_file() to later in the function, leaving
>the call in kernel_read_file() intact, or something else?
Hi Paul and Mimi,
Thanks for sharing your feedback! Yes, you are right, there is no need
for a new LSM hook. Actually by not introducing a new LSM hook, we can
have a much simpler solution!
>
>I think we want to leave the hook calls in kernel_read_file() intact,
>in which case I'm not certain what advantage there is in moving the
>security_kernel_post_read_file() call to a location where it is called
>in init_module_from_file() regardless of if the module is compressed
>or not. In the uncompressed case you are calling the hook twice for
>no real benefit? It may be helpful to submit a patch with your
>proposal as a patch can be worth a thousand words ;)
>
>
>> > diff --git a/kernel/module/main.c b/kernel/module/main.c
>> > index c66b26184936..f127000d2e0a 100644
>> > --- a/kernel/module/main.c
>> > +++ b/kernel/module/main.c
>> > @@ -3693,6 +3693,14 @@ static int init_module_from_file(struct file *f, const ch
>> > ar __user * uargs, int
>> > mod_stat_add_long(len, &invalid_decompress_bytes);
>> > return err;
>> > }
>> > +
>> > + err = security_kernel_post_read_file(f,
>> > + (char *)info.hdr, info.len,
>> > + READING_MODULE_DECOMPRESS);
>> > + if (err) {
>> > + mod_stat_inc(&failed_kreads);
>> > + return err;
>> > + }
>> > } else {
>> > info.hdr = buf;
>> > info.len = len;
>>
>> == defer security_kernel_post_read_file() call to here ==
By moving security_kernel_post_read_file, I think what Mimi means is to
move security_kernel_post_read_file in init_module_from_file() to later
in the function,
diff --git a/kernel/module/main.c b/kernel/module/main.c
index c66b261849362a..66725e53fef0c1 100644
--- a/kernel/module/main.c
+++ b/kernel/module/main.c
@@ -3678,6 +3678,7 @@ static int init_module_from_file(struct file *f, const char __user * uargs, int
struct load_info info = { };
void *buf = NULL;
int len;
+ int err;
len = kernel_read_file(f, 0, &buf, INT_MAX, NULL, READING_MODULE);
if (len < 0) {
@@ -3686,7 +3687,7 @@ static int init_module_from_file(struct file *f, const char __user * uargs, int
}
if (flags & MODULE_INIT_COMPRESSED_FILE) {
- int err = module_decompress(&info, buf, len);
+ err = module_decompress(&info, buf, len);
vfree(buf); /* compressed data is no longer needed */
if (err) {
mod_stat_inc(&failed_decompress);
@@ -3698,6 +3699,14 @@ static int init_module_from_file(struct file *f, const char __user * uargs, int
info.len = len;
}
+ err = security_kernel_post_read_file(f, (char *)info.hdr, info.len,
+ READING_MODULE);
+ if (err) {
+ mod_stat_inc(&failed_kreads);
+ free_copy(&info, flags);
+ return err;
+ }
+
return load_module(&info, uargs, flags);
}
If we only call security_kernel_post_read_file the 2nd time for a
decompressed kernel module, IMA won't be sure what to do when
security_kernel_post_read_file is called for the 1st time because it
can't distinguish between a compressed module with appended signature or
a uncompressed module without appended signature. If it permits 1st
calling security_kernel_post_read_file, a uncompressed module without
appended signature can be loaded. If it doesn't permit 1st calling
security_kernel_post_read_file, there is no change to call
security_kernel_post_read_file again for decompressed module.
And you are right, there is no need to call
security_kernel_post_read_file twice. And from the perspective of IMA,
it simplifies reasoning if it is guaranteed that IMA will always access
uncompressed kernel module regardless regardless of its original
compression state.
So I think a better solution is to stop calling
security_kernel_post_read_file in kernel_read_file for READING_MODULE.
This can also avoiding introducing an unnecessary
READING_MODULE_UNCOMPRESSED/READING_COMPRESSED_MODULE enumeration and
can make the solution even simpler,
diff --git a/fs/kernel_read_file.c b/fs/kernel_read_file.c
index de32c95d823dbd..7c78e84def6ec7 100644
--- a/fs/kernel_read_file.c
+++ b/fs/kernel_read_file.c
@@ -107,7 +107,12 @@ ssize_t kernel_read_file(struct file *file, loff_t offset, void **buf,
goto out_free;
}
- ret = security_kernel_post_read_file(file, *buf, i_size, id);
+ /*
+ * security_kernel_post_read_file will be called later after
+ * a read kernel module is truly decompressed
+ */
+ if (id != READING_MODULE)
+ ret = security_kernel_post_read_file(file, *buf, i_size, id);
}
Btw, I notice IMA is the only user of security_kernel_post_read_file so
this change won't affect other LSMs. For a full patch, please visit
https://github.com/coiby/linux/commit/558d85779ab5d794874749ecfae0e48b890bf3e0.patch
If there are concerns that I'm unaware of and a new
READING_MODULE_UNCOMPRESSED/READING_COMPRESSED_MODULE enumeration is
necessary, here's another patch
https://github.com/coiby/linux/commit/cdd40317b6070f48ec871c6a89428084f38ca083.patch
>
>--
>paul-moore.com
>
--
Best regards,
Coiby
^ permalink raw reply related
* Re: [PATCH v2] lsm,ima: new LSM hook security_kernel_module_read_file to access decompressed kernel module
From: Paul Moore @ 2025-11-05 2:47 UTC (permalink / raw)
To: Coiby Xu
Cc: Mimi Zohar, linux-integrity, linux-security-module, Karel Srot,
James Morris, Serge E. Hallyn, Luis Chamberlain, Petr Pavlu,
Daniel Gomez, Sami Tolvanen, Roberto Sassu, Dmitry Kasatkin,
Eric Snowberg, open list, open list:MODULE SUPPORT
In-Reply-To: <fftfj4o3kqxmfu3hb655xczqcddoeqjv55llsnwkrdu5isdm4z@6sqe3k24a6kk>
On Tue, Nov 4, 2025 at 7:19 PM Coiby Xu <coxu@redhat.com> wrote:
> On Sun, Nov 02, 2025 at 10:43:04AM -0500, Paul Moore wrote:
> >On Sun, Nov 2, 2025 at 10:06 AM Mimi Zohar <zohar@linux.ibm.com> wrote:
> >> On Sat, 2025-11-01 at 12:50 -0400, Paul Moore wrote:
> >> > On Fri, Oct 31, 2025 at 3:41 AM Coiby Xu <coxu@redhat.com> wrote:
> >> > >
> >> > > Currently, when in-kernel module decompression (CONFIG_MODULE_DECOMPRESS)
> >> > > is enabled, IMA has no way to verify the appended module signature as it
> >> > > can't decompress the module.
> >> > >
> >> > > Define a new LSM hook security_kernel_module_read_file which will be
> >> > > called after kernel module decompression is done so IMA can access the
> >> > > decompressed kernel module to verify the appended signature.
> >> > >
> >> > > Since IMA can access both xattr and appended kernel module signature
> >> > > with the new LSM hook, it no longer uses the security_kernel_post_read_file
> >> > > LSM hook for kernel module loading.
> >> > >
> >> > > Before enabling in-kernel module decompression, a kernel module in
> >> > > initramfs can still be loaded with ima_policy=secure_boot. So adjust the
> >> > > kernel module rule in secure_boot policy to allow either an IMA
> >> > > signature OR an appended signature i.e. to use
> >> > > "appraise func=MODULE_CHECK appraise_type=imasig|modsig".
> >> > >
> >> > > Reported-by: Karel Srot <ksrot@redhat.com>
> >> > > Suggested-by: Mimi Zohar <zohar@linux.ibm.com>
> >> > > Signed-off-by: Coiby Xu <coxu@redhat.com>
> >> > > ---
> >> > > v1: https://lore.kernel.org/linux-integrity/20250928030358.3873311-1-coxu@redhat.com/
> >> > >
> >> > > include/linux/lsm_hook_defs.h | 2 ++
> >> > > include/linux/security.h | 7 +++++++
> >> > > kernel/module/main.c | 10 +++++++++-
> >> > > security/integrity/ima/ima_main.c | 26 ++++++++++++++++++++++++++
> >> > > security/integrity/ima/ima_policy.c | 2 +-
> >> > > security/security.c | 17 +++++++++++++++++
> >> > > 6 files changed, 62 insertions(+), 2 deletions(-)
> >> >
> >> > We don't really need a new LSM hook for this do we? Can't we just
> >> > define a new file read type, e.g. READING_MODULE_DECOMPRESS, and do
> >> > another call to security_kernel_post_read_file() after the module is
> >> > unpacked? Something like the snippet below ...
> >>
> >> Yes, this is similar to my suggestion based on defining multiple enumerations:
> >> READING_MODULE, READING_COMPRESSED_MODULE, and READING_DECOMPRESSED_MODULE.
> >> With this solution, IMA would need to make an exception in the post kernel
> >> module read for the READING_COMPRESSED_MODULE case, since the kernel module has
> >> not yet been decompressed.
> >>
> >> Coiby suggested further simplification by moving the call later. At which point
> >> either there is or isn't an appended signature for non-compressed and
> >> decompressed kernel modules.
> >>
> >> As long as you don't have a problem calling the security_kernel_post_read_file()
> >> hook again, could we move the call later and pass READING_MODULE_UNCOMPRESSED?
> >
> >It isn't clear from these comments if you are talking about moving
> >only the second security_kernel_post_read_file() call that was
> >proposed for init_module_from_file() to later in the function, leaving
> >the call in kernel_read_file() intact, or something else?
>
> Hi Paul and Mimi,
>
> Thanks for sharing your feedback! Yes, you are right, there is no need
> for a new LSM hook. Actually by not introducing a new LSM hook, we can
> have a much simpler solution!
>
> >
> >I think we want to leave the hook calls in kernel_read_file() intact,
> >in which case I'm not certain what advantage there is in moving the
> >security_kernel_post_read_file() call to a location where it is called
> >in init_module_from_file() regardless of if the module is compressed
> >or not. In the uncompressed case you are calling the hook twice for
> >no real benefit? It may be helpful to submit a patch with your
> >proposal as a patch can be worth a thousand words ;)
> >
> >
> >> > diff --git a/kernel/module/main.c b/kernel/module/main.c
> >> > index c66b26184936..f127000d2e0a 100644
> >> > --- a/kernel/module/main.c
> >> > +++ b/kernel/module/main.c
> >> > @@ -3693,6 +3693,14 @@ static int init_module_from_file(struct file *f, const ch
> >> > ar __user * uargs, int
> >> > mod_stat_add_long(len, &invalid_decompress_bytes);
> >> > return err;
> >> > }
> >> > +
> >> > + err = security_kernel_post_read_file(f,
> >> > + (char *)info.hdr, info.len,
> >> > + READING_MODULE_DECOMPRESS);
> >> > + if (err) {
> >> > + mod_stat_inc(&failed_kreads);
> >> > + return err;
> >> > + }
> >> > } else {
> >> > info.hdr = buf;
> >> > info.len = len;
> >>
> >> == defer security_kernel_post_read_file() call to here ==
>
> By moving security_kernel_post_read_file, I think what Mimi means is to
> move security_kernel_post_read_file in init_module_from_file() to later
> in the function,
>
> diff --git a/kernel/module/main.c b/kernel/module/main.c
> index c66b261849362a..66725e53fef0c1 100644
> --- a/kernel/module/main.c
> +++ b/kernel/module/main.c
> @@ -3678,6 +3678,7 @@ static int init_module_from_file(struct file *f, const char __user * uargs, int
> struct load_info info = { };
> void *buf = NULL;
> int len;
> + int err;
>
> len = kernel_read_file(f, 0, &buf, INT_MAX, NULL, READING_MODULE);
> if (len < 0) {
> @@ -3686,7 +3687,7 @@ static int init_module_from_file(struct file *f, const char __user * uargs, int
> }
>
> if (flags & MODULE_INIT_COMPRESSED_FILE) {
> - int err = module_decompress(&info, buf, len);
> + err = module_decompress(&info, buf, len);
> vfree(buf); /* compressed data is no longer needed */
> if (err) {
> mod_stat_inc(&failed_decompress);
> @@ -3698,6 +3699,14 @@ static int init_module_from_file(struct file *f, const char __user * uargs, int
> info.len = len;
> }
>
> + err = security_kernel_post_read_file(f, (char *)info.hdr, info.len,
> + READING_MODULE);
> + if (err) {
> + mod_stat_inc(&failed_kreads);
> + free_copy(&info, flags);
> + return err;
> + }
> +
> return load_module(&info, uargs, flags);
> }
>
> If we only call security_kernel_post_read_file the 2nd time for a
> decompressed kernel module, IMA won't be sure what to do when
> security_kernel_post_read_file is called for the 1st time because it
> can't distinguish between a compressed module with appended signature or
> a uncompressed module without appended signature. If it permits 1st
> calling security_kernel_post_read_file, a uncompressed module without
> appended signature can be loaded. If it doesn't permit 1st calling
> security_kernel_post_read_file, there is no change to call
> security_kernel_post_read_file again for decompressed module.
>
> And you are right, there is no need to call
> security_kernel_post_read_file twice. And from the perspective of IMA,
> it simplifies reasoning if it is guaranteed that IMA will always access
> uncompressed kernel module regardless regardless of its original
> compression state.
>
> So I think a better solution is to stop calling
> security_kernel_post_read_file in kernel_read_file for READING_MODULE.
> This can also avoiding introducing an unnecessary
> READING_MODULE_UNCOMPRESSED/READING_COMPRESSED_MODULE enumeration and
> can make the solution even simpler,
>
> diff --git a/fs/kernel_read_file.c b/fs/kernel_read_file.c
> index de32c95d823dbd..7c78e84def6ec7 100644
> --- a/fs/kernel_read_file.c
> +++ b/fs/kernel_read_file.c
> @@ -107,7 +107,12 @@ ssize_t kernel_read_file(struct file *file, loff_t offset, void **buf,
> goto out_free;
> }
>
> - ret = security_kernel_post_read_file(file, *buf, i_size, id);
> + /*
> + * security_kernel_post_read_file will be called later after
> + * a read kernel module is truly decompressed
> + */
> + if (id != READING_MODULE)
> + ret = security_kernel_post_read_file(file, *buf, i_size, id);
> }
Assuming I'm understanding the problem correctly, I think you're
making this harder than it needs to be. I believe something like this
should solve the problem without having to add more conditionals
around the hooks in kernel_read_file(), and limiting the multiple
security_kernel_post_read_file() calls to just the compressed case ...
and honestly in each of the _post_read_file() calls in the compressed
case, the buffer contents have changed so it somewhat makes sense.
Given the code below, IMA could simply ignore the
READING_MODULE_COMPRESSED case (or whatever it is the IMA needs to do
in that case) and focus on the READING_MODULE case as it does today.
I expect the associated IMA patch would be both trivial and small.
diff --git a/kernel/module/main.c b/kernel/module/main.c
index c66b26184936..b435c498ec01 100644
--- a/kernel/module/main.c
+++ b/kernel/module/main.c
@@ -3675,17 +3675,19 @@ static int idempotent_wait_for_completion(struct idempot
ent *u)
static int init_module_from_file(struct file *f, const char __user * uargs, int
flags)
{
+ bool compressed = !!(flags & MODULE_INIT_COMPRESSED_FILE);
struct load_info info = { };
void *buf = NULL;
int len;
- len = kernel_read_file(f, 0, &buf, INT_MAX, NULL, READING_MODULE);
+ len = kernel_read_file(f, 0, &buf, INT_MAX, NULL,
+ compressed ? READING_MODULE_COMPRESSED : READING_
MODULE);
if (len < 0) {
mod_stat_inc(&failed_kreads);
return len;
}
- if (flags & MODULE_INIT_COMPRESSED_FILE) {
+ if (compressed) {
int err = module_decompress(&info, buf, len);
vfree(buf); /* compressed data is no longer needed */
if (err) {
@@ -3693,6 +3695,14 @@ static int init_module_from_file(struct file *f, const ch
ar __user * uargs, int
mod_stat_add_long(len, &invalid_decompress_bytes);
return err;
}
+
+ err = security_kernel_post_read_file(f,
+ (char *)info.hdr, info.len,
+ READING_MODULE);
+ if (err) {
+ mod_stat_inc(&failed_kreads);
+ return err;
+ }
} else {
info.hdr = buf;
info.len = len;
--
paul-moore.com
^ permalink raw reply related
* Re: [PATCH v8 04/23] slab: add sheaf support for batching kfree_rcu() operations
From: Vlastimil Babka @ 2025-11-05 11:25 UTC (permalink / raw)
To: Harry Yoo, Daniel Gomez, Suren Baghdasaryan
Cc: 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: <aQge2rmgRvd1JKxc@harry>
On 11/3/25 04:17, Harry Yoo wrote:
> 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.
Hm so there are actually two possible sources of this. One is that the
module creates some kmem_cache and calls kmem_cache_destroy() on it before
unloading. That does kvfree_rcu_barrier() which iterates all caches via
flush_all_rcu_sheaves(), but in this case it shouldn't need to - we could
have a weaker form of kvfree_rcu_barrier() that only guarantees flushing of
that single cache.
The other source is codetag_unload_module(), and I'm afraid it's this one as
it's hooked to evey module unload. Do you have CONFIG_CODE_TAGGING enabled?
Disabling it should help in this case, if you don't need memory allocation
profiling for that stress test. I think there's some space for improvement -
when compiled in but memalloc profiling never enabled during the uptime,
this could probably be skipped? Suren?
> 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.
Yeah, also it's done under slab_mutex. Is the stress test trying to unload
multiple modules in parallel? That would make things worse, although I'd
expect there's a lot serialization in this area already.
Unfortunately it will get worse with sheaves extended to all caches. We
could probably mark caches once they allocate their first rcu_free sheaf
(should not add visible overhead) and keep skipping those that never did.
> Just being curious, do you have any serious workload that depends on
> the performance of module unload?
^ permalink raw reply
* Re: [PATCH v2 0/3] module: Add compile-time check for embedded NUL characters
From: Daniel Gomez @ 2025-11-05 13:03 UTC (permalink / raw)
To: Kees Cook, Luis Chamberlain
Cc: 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 10/10/2025 05.06, 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!
>
> A long time ago we had an issue with embedded NUL bytes in MODULE_INFO
> strings[1]. While this stands out pretty strongly when you look at the
> code, and we can't do anything about a binary module that just plain lies,
> we never actually implemented the trivial compile-time check needed to
> detect it.
>
> Add this check (and fix 2 instances of needless trailing semicolons that
> this change exposed).
>
> Note that these patches were produced as part of another LLM exercise.
> This time I wanted to try "what happens if I ask an LLM to go read
> a specific LWN article and write a patch based on a discussion?" It
> pretty effortlessly chose and implemented a suggested solution, tested
> the change, and fixed new build warnings in the process.
>
> Since this was a relatively short session, here's an overview of the
> prompts involved as I guided it through a clean change and tried to see
> how it would reason about static_assert vs _Static_assert. (It wanted
> to use what was most common, not what was the current style -- we may
> want to update the comment above the static_assert macro to suggest
> using _Static_assert directly these days...)
>
> I want to fix a weakness in the module info strings. Read about it
> here: https://lwn.net/Articles/82305/
>
> Since it's only "info" that we need to check, can you reduce the checks
> to just that instead of all the other stuff?
>
> I think the change to the comment is redundent, and that should be
> in a commit log instead. Let's just keep the change to the static assert.
>
> Is "static_assert" the idiomatic way to use a static assert in this
> code base? I've seen _Static_assert used sometimes.
>
> What's the difference between the two?
>
> Does Linux use C11 by default now?
>
> Then let's not use the wrapper any more.
>
> Do an "allmodconfig all -s" build to verify this works for all modules
> in the kernel.
>
>
> Thanks!
>
> -Kees
>
> [1] https://lwn.net/Articles/82305/
>
> Kees Cook (3):
> media: dvb-usb-v2: lmedm04: Fix firmware macro definitions
> media: radio: si470x: Fix DRIVER_AUTHOR macro definition
> module: Add compile-time check for embedded NUL characters
>
> include/linux/moduleparam.h | 3 +++
> drivers/media/radio/si470x/radio-si470x-i2c.c | 2 +-
> drivers/media/usb/dvb-usb-v2/lmedm04.c | 12 ++++++------
> 3 files changed, 10 insertions(+), 7 deletions(-)
>
Reviewed-by: Daniel Gomez <da.gomez@samsung.com>
I have also tested a build of v6.18-rc3 + patches using allmodconfig:
Tested-by: Daniel Gomez <da.gomez@samsung.com>
^ permalink raw reply
* Re: [PATCH v2 0/3] module: Add compile-time check for embedded NUL characters
From: Daniel Gomez @ 2025-11-05 13:06 UTC (permalink / raw)
To: Kees Cook, Luis Chamberlain
Cc: 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: <3dd1a00d-08f7-4801-a9f7-d6db61c0e0f3@kernel.org>
On 05/11/2025 14.03, Daniel Gomez wrote:
> On 10/10/2025 05.06, 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!
>>
>> A long time ago we had an issue with embedded NUL bytes in MODULE_INFO
>> strings[1]. While this stands out pretty strongly when you look at the
>> code, and we can't do anything about a binary module that just plain lies,
>> we never actually implemented the trivial compile-time check needed to
>> detect it.
>>
>> Add this check (and fix 2 instances of needless trailing semicolons that
>> this change exposed).
>>
>> Note that these patches were produced as part of another LLM exercise.
>> This time I wanted to try "what happens if I ask an LLM to go read
>> a specific LWN article and write a patch based on a discussion?" It
>> pretty effortlessly chose and implemented a suggested solution, tested
>> the change, and fixed new build warnings in the process.
>>
>> Since this was a relatively short session, here's an overview of the
>> prompts involved as I guided it through a clean change and tried to see
>> how it would reason about static_assert vs _Static_assert. (It wanted
>> to use what was most common, not what was the current style -- we may
>> want to update the comment above the static_assert macro to suggest
>> using _Static_assert directly these days...)
>>
>> I want to fix a weakness in the module info strings. Read about it
>> here: https://lwn.net/Articles/82305/
>>
>> Since it's only "info" that we need to check, can you reduce the checks
>> to just that instead of all the other stuff?
>>
>> I think the change to the comment is redundent, and that should be
>> in a commit log instead. Let's just keep the change to the static assert.
>>
>> Is "static_assert" the idiomatic way to use a static assert in this
>> code base? I've seen _Static_assert used sometimes.
>>
>> What's the difference between the two?
>>
>> Does Linux use C11 by default now?
>>
>> Then let's not use the wrapper any more.
>>
>> Do an "allmodconfig all -s" build to verify this works for all modules
>> in the kernel.
>>
>>
>> Thanks!
>>
>> -Kees
>>
>> [1] https://lwn.net/Articles/82305/
>>
>> Kees Cook (3):
>> media: dvb-usb-v2: lmedm04: Fix firmware macro definitions
>> media: radio: si470x: Fix DRIVER_AUTHOR macro definition
>> module: Add compile-time check for embedded NUL characters
>>
>> include/linux/moduleparam.h | 3 +++
>> drivers/media/radio/si470x/radio-si470x-i2c.c | 2 +-
>> drivers/media/usb/dvb-usb-v2/lmedm04.c | 12 ++++++------
>> 3 files changed, 10 insertions(+), 7 deletions(-)
>>
>
> Reviewed-by: Daniel Gomez <da.gomez@samsung.com>
>
> I have also tested a build of v6.18-rc3 + patches using allmodconfig:
>
> Tested-by: Daniel Gomez <da.gomez@samsung.com>
>
I forgot to mention it required the following patch for the build to succeed:
dmaengine: mmp_pdma: fix DMA mask handling
https://lore.kernel.org/all/176061935426.510550.684278188506408313.b4-ty@kernel.org/
^ permalink raw reply
* Re: [PATCH v2 0/3] module: Add compile-time check for embedded NUL characters
From: Daniel Gomez @ 2025-11-05 13:19 UTC (permalink / raw)
To: Luis Chamberlain, Kees Cook
Cc: Hans Verkuil, Malcolm Priestley, Mauro Carvalho Chehab,
Hans Verkuil, Uwe Kleine-König, Rusty Russell, Petr Pavlu,
Daniel Gomez, 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, thanks!
[1/3] media: dvb-usb-v2: lmedm04: Fix firmware macro definitions
(no commit info)
[2/3] media: radio: si470x: Fix DRIVER_AUTHOR macro definition
(no commit info)
[3/3] module: Add compile-time check for embedded NUL characters
(no commit info)
Best regards,
--
Daniel Gomez <da.gomez@samsung.com>
^ permalink raw reply
* Re: [PATCH v2] lsm,ima: new LSM hook security_kernel_module_read_file to access decompressed kernel module
From: Mimi Zohar @ 2025-11-05 14:07 UTC (permalink / raw)
To: Paul Moore, Coiby Xu
Cc: linux-integrity, linux-security-module, Karel Srot, James Morris,
Serge E. Hallyn, Luis Chamberlain, Petr Pavlu, Daniel Gomez,
Sami Tolvanen, Roberto Sassu, Dmitry Kasatkin, Eric Snowberg,
open list, open list:MODULE SUPPORT
In-Reply-To: <CAHC9VhRGwXvhU64Nk5jdmtPfrt9bbkzpLVqS0LRbtN3Q3HhnCw@mail.gmail.com>
On Tue, 2025-11-04 at 21:47 -0500, Paul Moore wrote:
> Assuming I'm understanding the problem correctly, I think you're
> making this harder than it needs to be. I believe something like this
> should solve the problem without having to add more conditionals
> around the hooks in kernel_read_file(), and limiting the multiple
> security_kernel_post_read_file() calls to just the compressed case ...
> and honestly in each of the _post_read_file() calls in the compressed
> case, the buffer contents have changed so it somewhat makes sense.
> Given the code below, IMA could simply ignore the
> READING_MODULE_COMPRESSED case (or whatever it is the IMA needs to do
> in that case) and focus on the READING_MODULE case as it does today.
> I expect the associated IMA patch would be both trivial and small.
>
> diff --git a/kernel/module/main.c b/kernel/module/main.c
> index c66b26184936..b435c498ec01 100644
> --- a/kernel/module/main.c
> +++ b/kernel/module/main.c
> @@ -3675,17 +3675,19 @@ static int idempotent_wait_for_completion(struct idempot
> ent *u)
>
> static int init_module_from_file(struct file *f, const char __user * uargs, int
> flags)
> {
> + bool compressed = !!(flags & MODULE_INIT_COMPRESSED_FILE);
> struct load_info info = { };
> void *buf = NULL;
> int len;
>
> - len = kernel_read_file(f, 0, &buf, INT_MAX, NULL, READING_MODULE);
> + len = kernel_read_file(f, 0, &buf, INT_MAX, NULL,
> + compressed ? READING_MODULE_COMPRESSED : READING_
> MODULE);
> if (len < 0) {
> mod_stat_inc(&failed_kreads);
> return len;
> }
>
> - if (flags & MODULE_INIT_COMPRESSED_FILE) {
> + if (compressed) {
> int err = module_decompress(&info, buf, len);
> vfree(buf); /* compressed data is no longer needed */
> if (err) {
> @@ -3693,6 +3695,14 @@ static int init_module_from_file(struct file *f, const ch
> ar __user * uargs, int
> mod_stat_add_long(len, &invalid_decompress_bytes);
> return err;
> }
> +
> + err = security_kernel_post_read_file(f,
> + (char *)info.hdr, info.len,
> + READING_MODULE);
Without changing the enumeration here, IMA would not be able to differentiate
the first call to security_kernel_post_read_file() and this one. The first call
would result in unnecessary error messages.
Adding an additional call to security_kernel_post_read_file() here, would
require defining 2 additional enumerations: READING_MODULE_COMPRESSED,
READING_MODULE_DECOMPRESSED.
> + if (err) {
> + mod_stat_inc(&failed_kreads);
> + return err;
> + }
> } else {
> info.hdr = buf;
> info.len = len;
Deferring the security_kernel_post_read_file() call to here, eliminates the need
for defining additional enumerations. (Coiby's first link.)
Adding an additional call to security_kernel_post_read_file() here, requires 1
additional enumeration. (Coiby's 2nd link.)
Mimi
^ permalink raw reply
* [PATCH 0/6] kallsyms: Prevent invalid access when showing module buildid
From: Petr Mladek @ 2025-11-05 14:23 UTC (permalink / raw)
To: Petr Pavlu, Steven Rostedt, Alexei Starovoitov, Andrew Morton,
Kees Cook
Cc: Daniel Borkmann, John Fastabend, Masami Hiramatsu, Mark Rutland,
Luis Chamberlain, Daniel Gomez, Sami Tolvanen, linux-kernel, bpf,
linux-modules, linux-trace-kernel, Petr Mladek
We have seen nested crashes in __sprint_symbol(), see below. They seems
to be caused by invalid pointer to "buildid".
I made an audit of __sprint_symbol() and found several situations
when the buildid might be wrong:
+ bpf_address_lookup() does not set @modbuildid
+ ftrace_mod_address_lookup() does not set @modbuildid
+ __sprint_symbol() does not take rcu_read_lock and
the related struct module might get removed before
mod->build_id is printed.
This patchset solves these problems:
+ 1st, 2nd patches are preparatory
+ 3rd, 4th, 6th patches fix the above problems
+ 5th patch cleans up a suspicious initialization code.
This is the backtrace, we have seen. But it is not really important.
The problems fixed by the patchset are obvious:
crash64> bt [62/2029]
PID: 136151 TASK: ffff9f6c981d4000 CPU: 367 COMMAND: "btrfs"
#0 [ffffbdb687635c28] machine_kexec at ffffffffb4c845b3
#1 [ffffbdb687635c80] __crash_kexec at ffffffffb4d86a6a
#2 [ffffbdb687635d08] hex_string at ffffffffb51b3b61
#3 [ffffbdb687635d40] crash_kexec at ffffffffb4d87964
#4 [ffffbdb687635d50] oops_end at ffffffffb4c41fc8
#5 [ffffbdb687635d70] do_trap at ffffffffb4c3e49a
#6 [ffffbdb687635db8] do_error_trap at ffffffffb4c3e6a4
#7 [ffffbdb687635df8] exc_stack_segment at ffffffffb5666b33
#8 [ffffbdb687635e20] asm_exc_stack_segment at ffffffffb5800cf9
#9 [ffffbdb687635ea8] hex_string at ffffffffb51b3b61
#10 [ffffbdb687635ef8] vsnprintf at ffffffffb51b7291
#11 [ffffbdb687635f50] sprintf at ffffffffb51b7541
#12 [ffffbdb687635fb8] __sprint_symbol at ffffffffb4d849d6
#13 [ffffbdb687636018] symbol_string at ffffffffb51b4588
#14 [ffffbdb687636168] vsnprintf at ffffffffb51b7291
#15 [ffffbdb6876361c0] vscnprintf at ffffffffb51b73b9
#16 [ffffbdb6876361d0] printk_sprint at ffffffffb4d2ae82
#17 [ffffbdb6876361f8] vprintk_store at ffffffffb4d2d06d
#18 [ffffbdb6876362c8] vprintk_emit at ffffffffb4d2d1bf
#19 [ffffbdb687636308] printk at ffffffffb565e5ce
#20 [ffffbdb687636370] show_trace_log_lvl at ffffffffb4c42374
#21 [ffffbdb687636478] __die_body at ffffffffb4c426ca
#22 [ffffbdb687636498] die at ffffffffb4c42778
#23 [ffffbdb6876364c0] do_trap at ffffffffb4c3e49a
#24 [ffffbdb687636508] do_error_trap at ffffffffb4c3e6a4
#25 [ffffbdb687636548] exc_stack_segment at ffffffffb5666b33
#26 [ffffbdb687636570] asm_exc_stack_segment at ffffffffb5800cf9
#27 [ffffbdb6876365f8] hex_string at ffffffffb51b3b61
#28 [ffffbdb687636648] vsnprintf at ffffffffb51b7291
#29 [ffffbdb6876366a0] sprintf at ffffffffb51b7541
#30 [ffffbdb687636708] __sprint_symbol at ffffffffb4d849d6
#31 [ffffbdb687636768] symbol_string at ffffffffb51b4588
#32 [ffffbdb6876368b8] vsnprintf at ffffffffb51b7291
#33 [ffffbdb687636910] vscnprintf at ffffffffb51b73b9
#34 [ffffbdb687636920] printk_sprint at ffffffffb4d2ae82
#35 [ffffbdb687636948] vprintk_store at ffffffffb4d2d06d
#36 [ffffbdb687636a18] vprintk_emit at ffffffffb4d2d1bf
#37 [ffffbdb687636a58] printk at ffffffffb565e5ce
#38 [ffffbdb687636ac0] show_trace_log_lvl at ffffffffb4c42374
#39 [ffffbdb687636bc8] __die_body at ffffffffb4c426ca
#40 [ffffbdb687636be8] die at ffffffffb4c42778
#41 [ffffbdb687636c10] do_trap at ffffffffb4c3e49a
#42 [ffffbdb687636c58] do_error_trap at ffffffffb4c3e6a4
#43 [ffffbdb687636c98] exc_stack_segment at ffffffffb5666b33
#44 [ffffbdb687636cc0] asm_exc_stack_segment at ffffffffb5800cf9
#45 [ffffbdb687636d48] hex_string at ffffffffb51b3b61
#46 [ffffbdb687636d98] vsnprintf at ffffffffb51b7291
#47 [ffffbdb687636df0] sprintf at ffffffffb51b7541
#48 [ffffbdb687636e58] __sprint_symbol at ffffffffb4d849d6
#49 [ffffbdb687636eb8] symbol_string at ffffffffb51b4588
#50 [ffffbdb687637008] vsnprintf at ffffffffb51b7291
#51 [ffffbdb687637060] vscnprintf at ffffffffb51b73b9
#52 [ffffbdb687637070] printk_sprint at ffffffffb4d2ae82
#53 [ffffbdb687637098] vprintk_store at ffffffffb4d2d06d
#54 [ffffbdb687637168] vprintk_emit at ffffffffb4d2d1bf
#55 [ffffbdb6876371a8] printk at ffffffffb565e5ce
#56 [ffffbdb687637210] show_trace_log_lvl at ffffffffb4c42374
#57 [ffffbdb687637318] __die_body at ffffffffb4c426ca
#58 [ffffbdb687637338] die at ffffffffb4c42778
#59 [ffffbdb687637360] do_trap at ffffffffb4c3e49a
#60 [ffffbdb6876373a8] do_error_trap at ffffffffb4c3e6a4
#61 [ffffbdb6876373e8] exc_stack_segment at ffffffffb5666b33
#62 [ffffbdb687637410] asm_exc_stack_segment at ffffffffb5800cf9
#63 [ffffbdb687637498] hex_string at ffffffffb51b3b61
#64 [ffffbdb6876374e8] vsnprintf at ffffffffb51b7291
#65 [ffffbdb687637540] sprintf at ffffffffb51b7541
#66 [ffffbdb6876375a8] __sprint_symbol at ffffffffb4d849d6
#67 [ffffbdb687637608] symbol_string at ffffffffb51b4588
#68 [ffffbdb687637758] vsnprintf at ffffffffb51b7291
#69 [ffffbdb6876377b0] vscnprintf at ffffffffb51b73b9
#70 [ffffbdb6876377c0] printk_sprint at ffffffffb4d2ae82
#71 [ffffbdb6876377e8] vprintk_store at ffffffffb4d2d06d
#72 [ffffbdb6876378b8] vprintk_emit at ffffffffb4d2d1bf
#73 [ffffbdb6876378f8] printk at ffffffffb565e5ce
#74 [ffffbdb687637960] show_trace_log_lvl at ffffffffb4c42374
#75 [ffffbdb687637a68] __warn at ffffffffb4cb0d4d
#76 [ffffbdb687637aa0] report_bug at ffffffffb51a73fb
#77 [ffffbdb687637ad8] handle_bug at ffffffffb5666817
#78 [ffffbdb687637ae8] exc_invalid_op at ffffffffb56669d3
#79 [ffffbdb687637b00] asm_exc_invalid_op at ffffffffb5800e0d
[exception RIP: btrfs_ioctl_send+0x26e]
RIP: ffffffffc06070ce RSP: ffffbdb687637bb8 RFLAGS: 00010282
RAX: ffff9f6e50160380 RBX: ffff9f8eda64f200 RCX: 0000000000000000
RDX: 0000000000000000 RSI: 0000000000000000 RDI: 0000000000000000
RBP: 0000000000000000 R8: 000000000000000a R9: ffff9f6c9e1c5b20
R10: 0000000000000075 R11: 0000000000000004 R12: ffff9f6d43a24000
R13: 0000000000000001 R14: 0000000000000000 R15: ffff9a2d65644d30
ORIG_RAX: ffffffffffffffff CS: 0010 SS: 0018
#80 [ffffbdb687637c78] _btrfs_ioctl_send at ffffffffc05c31d4 [btrfs]
#81 [ffffbdb687637ce8] btrfs_ioctl at ffffffffc05c80c4 [btrfs]
#82 [ffffbdb687637df8] __x64_sys_ioctl at ffffffffb4f776df
#83 [ffffbdb687637e38] do_syscall_64 at ffffffffb56663f8
RIP: 00007fbd339164a7 RSP: 00007ffde6a19888 RFLAGS: 00000246
RAX: ffffffffffffffda RBX: 0000000000000fe2 RCX: 00007fbd339164a7
RDX: 00007ffde6a19980 RSI: 0000000040489426 RDI: 0000000000000022
RBP: 00007ffde6a1ab80 R8: 00007ffde6a19980 R9: 00007fbd33808700
R10: 00007fbd338089d0 R11: 0000000000000246 R12: 0000000000000022
R13: 0000000000000001 R14: 0000000000000001 R15: 00005585428002b0
ORIG_RAX: 0000000000000010 CS: 0033 SS: 002b
Petr Mladek (6):
module: Add helper function for reading module_buildid()
kallsyms: Cleanup code for appending the module buildid
kallsyms/bpf: Set module buildid in bpf_address_lookup()
kallsyms/ftrace: Set module buildid in ftrace_mod_address_lookup()
kallsyms: Clean up @namebuf initialization in
kallsyms_lookup_buildid()
kallsyms: Prevent module removal when printing module name and buildid
include/linux/filter.h | 15 +++++++---
include/linux/ftrace.h | 6 ++--
include/linux/module.h | 9 ++++++
kernel/kallsyms.c | 60 ++++++++++++++++++++++++++++++----------
kernel/module/kallsyms.c | 9 ++----
kernel/trace/ftrace.c | 5 +++-
6 files changed, 76 insertions(+), 28 deletions(-)
--
2.51.1
^ permalink raw reply
* [PATCH 1/6] module: Add helper function for reading module_buildid()
From: Petr Mladek @ 2025-11-05 14:23 UTC (permalink / raw)
To: Petr Pavlu, Steven Rostedt, Alexei Starovoitov, Andrew Morton,
Kees Cook
Cc: Daniel Borkmann, John Fastabend, Masami Hiramatsu, Mark Rutland,
Luis Chamberlain, Daniel Gomez, Sami Tolvanen, linux-kernel, bpf,
linux-modules, linux-trace-kernel, Petr Mladek
In-Reply-To: <20251105142319.1139183-1-pmladek@suse.com>
Add a helper function for reading the optional "build_id" member
of struct module. It is going to be used also in
ftrace_mod_address_lookup().
Use "#ifdef" instead of "#if IS_ENABLED()" to match the declaration
of the optional field in struct module.
Signed-off-by: Petr Mladek <pmladek@suse.com>
---
include/linux/module.h | 9 +++++++++
kernel/module/kallsyms.c | 9 ++-------
2 files changed, 11 insertions(+), 7 deletions(-)
diff --git a/include/linux/module.h b/include/linux/module.h
index e135cc79acee..4decae2b1675 100644
--- a/include/linux/module.h
+++ b/include/linux/module.h
@@ -747,6 +747,15 @@ static inline void __module_get(struct module *module)
__mod ? __mod->name : "kernel"; \
})
+static inline const unsigned char *module_buildid(struct module *mod)
+{
+#ifdef CONFIG_STACKTRACE_BUILD_ID
+ return mod->build_id;
+#else
+ return NULL;
+#endif
+}
+
/* Dereference module function descriptor */
void *dereference_module_function_descriptor(struct module *mod, void *ptr);
diff --git a/kernel/module/kallsyms.c b/kernel/module/kallsyms.c
index 00a60796327c..0fc11e45df9b 100644
--- a/kernel/module/kallsyms.c
+++ b/kernel/module/kallsyms.c
@@ -334,13 +334,8 @@ int module_address_lookup(unsigned long addr,
if (mod) {
if (modname)
*modname = mod->name;
- if (modbuildid) {
-#if IS_ENABLED(CONFIG_STACKTRACE_BUILD_ID)
- *modbuildid = mod->build_id;
-#else
- *modbuildid = NULL;
-#endif
- }
+ if (modbuildid)
+ *modbuildid = module_buildid(mod);
sym = find_kallsyms_symbol(mod, addr, size, offset);
--
2.51.1
^ permalink raw reply related
* [PATCH 2/6] kallsyms: Cleanup code for appending the module buildid
From: Petr Mladek @ 2025-11-05 14:23 UTC (permalink / raw)
To: Petr Pavlu, Steven Rostedt, Alexei Starovoitov, Andrew Morton,
Kees Cook
Cc: Daniel Borkmann, John Fastabend, Masami Hiramatsu, Mark Rutland,
Luis Chamberlain, Daniel Gomez, Sami Tolvanen, linux-kernel, bpf,
linux-modules, linux-trace-kernel, Petr Mladek
In-Reply-To: <20251105142319.1139183-1-pmladek@suse.com>
Put the code for appending the optional "buildid" into a helper
function, It makes __sprint_symbol() better readable.
Also print a warning when the "modname" is set and the "buildid" isn't.
It might catch a situation when some lookup function in
kallsyms_lookup_buildid() does not handle the "buildid".
Use pr_*_once() to avoid an infinite recursion when the function
is called from printk(). The recursion is rather theoretical but
better be on the safe side.
Signed-off-by: Petr Mladek <pmladek@suse.com>
---
kernel/kallsyms.c | 42 +++++++++++++++++++++++++++++++++---------
1 file changed, 33 insertions(+), 9 deletions(-)
diff --git a/kernel/kallsyms.c b/kernel/kallsyms.c
index 1e7635864124..9455e3bb07fc 100644
--- a/kernel/kallsyms.c
+++ b/kernel/kallsyms.c
@@ -423,6 +423,37 @@ int lookup_symbol_name(unsigned long addr, char *symname)
return lookup_module_symbol_name(addr, symname);
}
+#ifdef CONFIG_STACKTRACE_BUILD_ID
+
+static int append_buildid(char *buffer, const char *modname,
+ const unsigned char *buildid)
+{
+ if (!modname)
+ return 0;
+
+ if (!buildid) {
+ pr_warn_once("Undefined buildid for the module %s\n", modname);
+ return 0;
+ }
+
+ /* build ID should match length of sprintf */
+#ifdef CONFIG_MODULES
+ static_assert(sizeof(typeof_member(struct module, build_id)) == 20);
+#endif
+
+ return sprintf(buffer, " %20phN", buildid);
+}
+
+#else /* CONFIG_STACKTRACE_BUILD_ID */
+
+static int append_buildid(char *buffer, const char *modname,
+ const unsigned char *buildid)
+{
+ return 0;
+}
+
+#endif /* CONFIG_STACKTRACE_BUILD_ID */
+
/* Look up a kernel symbol and return it in a text buffer. */
static int __sprint_symbol(char *buffer, unsigned long address,
int symbol_offset, int add_offset, int add_buildid)
@@ -445,15 +476,8 @@ static int __sprint_symbol(char *buffer, unsigned long address,
if (modname) {
len += sprintf(buffer + len, " [%s", modname);
-#if IS_ENABLED(CONFIG_STACKTRACE_BUILD_ID)
- if (add_buildid && buildid) {
- /* build ID should match length of sprintf */
-#if IS_ENABLED(CONFIG_MODULES)
- static_assert(sizeof(typeof_member(struct module, build_id)) == 20);
-#endif
- len += sprintf(buffer + len, " %20phN", buildid);
- }
-#endif
+ if (add_buildid)
+ len += append_buildid(buffer + len, modname, buildid);
len += sprintf(buffer + len, "]");
}
--
2.51.1
^ permalink raw reply related
* [PATCH 3/6] kallsyms/bpf: Set module buildid in bpf_address_lookup()
From: Petr Mladek @ 2025-11-05 14:23 UTC (permalink / raw)
To: Petr Pavlu, Steven Rostedt, Alexei Starovoitov, Andrew Morton,
Kees Cook
Cc: Daniel Borkmann, John Fastabend, Masami Hiramatsu, Mark Rutland,
Luis Chamberlain, Daniel Gomez, Sami Tolvanen, linux-kernel, bpf,
linux-modules, linux-trace-kernel, Petr Mladek
In-Reply-To: <20251105142319.1139183-1-pmladek@suse.com>
Make bpf_address_lookup() compatible with module_address_lookup()
and clear the pointer to @modbuildid together with @modname.
It is not strictly needed because __sprint_symbol() reads @modbuildid
only when @modname is set. But better be on the safe side and make
the API more safe.
Fixes: 9294523e3768 ("module: add printk formats to add module build ID to stacktraces")
Signed-off-by: Petr Mladek <pmladek@suse.com>
---
include/linux/filter.h | 15 +++++++++++----
kernel/kallsyms.c | 4 ++--
2 files changed, 13 insertions(+), 6 deletions(-)
diff --git a/include/linux/filter.h b/include/linux/filter.h
index f5c859b8131a..b7b95840250a 100644
--- a/include/linux/filter.h
+++ b/include/linux/filter.h
@@ -1362,12 +1362,18 @@ struct bpf_prog *bpf_prog_ksym_find(unsigned long addr);
static inline int
bpf_address_lookup(unsigned long addr, unsigned long *size,
- unsigned long *off, char **modname, char *sym)
+ unsigned long *off, char **modname,
+ const unsigned char **modbuildid, char *sym)
{
int ret = __bpf_address_lookup(addr, size, off, sym);
- if (ret && modname)
- *modname = NULL;
+ if (ret) {
+ if (modname)
+ *modname = NULL;
+ if (modbuildid)
+ *modbuildid = NULL;
+ }
+
return ret;
}
@@ -1433,7 +1439,8 @@ static inline struct bpf_prog *bpf_prog_ksym_find(unsigned long addr)
static inline int
bpf_address_lookup(unsigned long addr, unsigned long *size,
- unsigned long *off, char **modname, char *sym)
+ unsigned long *off, char **modname,
+ const unsigned char **modbuildid, char *sym)
{
return 0;
}
diff --git a/kernel/kallsyms.c b/kernel/kallsyms.c
index 9455e3bb07fc..efb12b077220 100644
--- a/kernel/kallsyms.c
+++ b/kernel/kallsyms.c
@@ -374,8 +374,8 @@ static int kallsyms_lookup_buildid(unsigned long addr,
ret = module_address_lookup(addr, symbolsize, offset,
modname, modbuildid, namebuf);
if (!ret)
- ret = bpf_address_lookup(addr, symbolsize,
- offset, modname, namebuf);
+ ret = bpf_address_lookup(addr, symbolsize, offset,
+ modname, modbuildid, namebuf);
if (!ret)
ret = ftrace_mod_address_lookup(addr, symbolsize,
--
2.51.1
^ permalink raw reply related
* [PATCH 4/6] kallsyms/ftrace: Set module buildid in ftrace_mod_address_lookup()
From: Petr Mladek @ 2025-11-05 14:23 UTC (permalink / raw)
To: Petr Pavlu, Steven Rostedt, Alexei Starovoitov, Andrew Morton,
Kees Cook
Cc: Daniel Borkmann, John Fastabend, Masami Hiramatsu, Mark Rutland,
Luis Chamberlain, Daniel Gomez, Sami Tolvanen, linux-kernel, bpf,
linux-modules, linux-trace-kernel, Petr Mladek
In-Reply-To: <20251105142319.1139183-1-pmladek@suse.com>
__sprint_symbol() might access an invalid pointer when
kallsyms_lookup_buildid() returns a symbol found by
ftrace_mod_address_lookup().
The ftrace lookup function must set both @modname and @modbuildid
the same way as module_address_lookup().
Fixes: 9294523e3768 ("module: add printk formats to add module build ID to stacktraces")
Signed-off-by: Petr Mladek <pmladek@suse.com>
---
include/linux/ftrace.h | 6 ++++--
kernel/kallsyms.c | 4 ++--
kernel/trace/ftrace.c | 5 ++++-
3 files changed, 10 insertions(+), 5 deletions(-)
diff --git a/include/linux/ftrace.h b/include/linux/ftrace.h
index 7ded7df6e9b5..a003cf1b32d0 100644
--- a/include/linux/ftrace.h
+++ b/include/linux/ftrace.h
@@ -87,11 +87,13 @@ struct ftrace_hash;
defined(CONFIG_DYNAMIC_FTRACE)
int
ftrace_mod_address_lookup(unsigned long addr, unsigned long *size,
- unsigned long *off, char **modname, char *sym);
+ unsigned long *off, char **modname,
+ const unsigned char **modbuildid, char *sym);
#else
static inline int
ftrace_mod_address_lookup(unsigned long addr, unsigned long *size,
- unsigned long *off, char **modname, char *sym)
+ unsigned long *off, char **modname,
+ const unsigned char **modbuildid, char *sym)
{
return 0;
}
diff --git a/kernel/kallsyms.c b/kernel/kallsyms.c
index efb12b077220..71868a76e9a1 100644
--- a/kernel/kallsyms.c
+++ b/kernel/kallsyms.c
@@ -378,8 +378,8 @@ static int kallsyms_lookup_buildid(unsigned long addr,
modname, modbuildid, namebuf);
if (!ret)
- ret = ftrace_mod_address_lookup(addr, symbolsize,
- offset, modname, namebuf);
+ ret = ftrace_mod_address_lookup(addr, symbolsize, offset,
+ modname, modbuildid, namebuf);
return ret;
}
diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index 42bd2ba68a82..11f5096fb60c 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -7678,7 +7678,8 @@ ftrace_func_address_lookup(struct ftrace_mod_map *mod_map,
int
ftrace_mod_address_lookup(unsigned long addr, unsigned long *size,
- unsigned long *off, char **modname, char *sym)
+ unsigned long *off, char **modname,
+ const unsigned char **modbuildid, char *sym)
{
struct ftrace_mod_map *mod_map;
int ret = 0;
@@ -7690,6 +7691,8 @@ ftrace_mod_address_lookup(unsigned long addr, unsigned long *size,
if (ret) {
if (modname)
*modname = mod_map->mod->name;
+ if (modbuildid)
+ *modbuildid = module_buildid(mod_map->mod);
break;
}
}
--
2.51.1
^ permalink raw reply related
* [PATCH 5/6] kallsyms: Clean up @namebuf initialization in kallsyms_lookup_buildid()
From: Petr Mladek @ 2025-11-05 14:23 UTC (permalink / raw)
To: Petr Pavlu, Steven Rostedt, Alexei Starovoitov, Andrew Morton,
Kees Cook
Cc: Daniel Borkmann, John Fastabend, Masami Hiramatsu, Mark Rutland,
Luis Chamberlain, Daniel Gomez, Sami Tolvanen, linux-kernel, bpf,
linux-modules, linux-trace-kernel, Petr Mladek
In-Reply-To: <20251105142319.1139183-1-pmladek@suse.com>
The function kallsyms_lookup_buildid() initializes the given @namebuf
by clearing the first and the last byte. It is not clear why.
The 1st byte makes sense because some callers ignore the return code
and expect that the buffer contains a valid string, for example:
- function_stat_show()
- kallsyms_lookup()
- kallsyms_lookup_buildid()
The initialization of the last byte does not make much sense because it
can later be overwritten. Fortunately, it seems that all called
functions behave correctly:
- kallsyms_expand_symbol() explicitly adds the trailing '\0'
at the end of the function.
- All *__address_lookup() functions either use the safe strscpy()
or they do not touch the buffer at all.
Document the reason for clearing the first byte. And remove the useless
initialization of the last byte.
Signed-off-by: Petr Mladek <pmladek@suse.com>
---
kernel/kallsyms.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/kernel/kallsyms.c b/kernel/kallsyms.c
index 71868a76e9a1..ff7017337535 100644
--- a/kernel/kallsyms.c
+++ b/kernel/kallsyms.c
@@ -352,7 +352,12 @@ static int kallsyms_lookup_buildid(unsigned long addr,
{
int ret;
- namebuf[KSYM_NAME_LEN - 1] = 0;
+ /*
+ * kallsyms_lookus() returns pointer to namebuf on success and
+ * NULL on error. But some callers ignore the return value.
+ * Instead they expect @namebuf filled either with valid
+ * or empty string.
+ */
namebuf[0] = 0;
if (is_ksym_addr(addr)) {
--
2.51.1
^ permalink raw reply related
* [PATCH 6/6] kallsyms: Prevent module removal when printing module name and buildid
From: Petr Mladek @ 2025-11-05 14:23 UTC (permalink / raw)
To: Petr Pavlu, Steven Rostedt, Alexei Starovoitov, Andrew Morton,
Kees Cook
Cc: Daniel Borkmann, John Fastabend, Masami Hiramatsu, Mark Rutland,
Luis Chamberlain, Daniel Gomez, Sami Tolvanen, linux-kernel, bpf,
linux-modules, linux-trace-kernel, Petr Mladek
In-Reply-To: <20251105142319.1139183-1-pmladek@suse.com>
kallsyms_lookup_buildid() copies the symbol name into the given buffer
so that it can be safely read anytime later. But it just copies pointers
to mod->name and mod->build_id which might get reused after the related
struct module gets removed.
The lifetime of struct module is synchronized using RCU. Take the rcu
read lock for the entire __sprint_symbol().
Signed-off-by: Petr Mladek <pmladek@suse.com>
---
kernel/kallsyms.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/kernel/kallsyms.c b/kernel/kallsyms.c
index ff7017337535..1fda06b6638c 100644
--- a/kernel/kallsyms.c
+++ b/kernel/kallsyms.c
@@ -468,6 +468,9 @@ static int __sprint_symbol(char *buffer, unsigned long address,
unsigned long offset, size;
int len;
+ /* Prevent module removal until modname and modbuildid are printed */
+ guard(rcu)();
+
address += symbol_offset;
len = kallsyms_lookup_buildid(address, &size, &offset, &modname, &buildid,
buffer);
--
2.51.1
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox