* Re: [PATCH v3 4/4] module: separate vermagic and livepatch checks
From: Petr Pavlu @ 2025-09-01 9:34 UTC (permalink / raw)
To: Jinchao Wang
Cc: Luis Chamberlain, Daniel Gomez, Sami Tolvanen, linux-modules,
linux-kernel
In-Reply-To: <20250829084927.156676-5-wangjinchao600@gmail.com>
On 8/29/25 10:49 AM, Jinchao Wang wrote:
> Rename check_modinfo() to check_modinfo_vermagic() to clarify it only
> checks vermagic compatibility.
>
> Move livepatch check to happen after vermagic check in early_mod_check(),
> creating proper separation of concerns.
> No functional changes, just clearer code organization.
>
> Signed-off-by: Jinchao Wang <wangjinchao600@gmail.com>
Reviewed-by: Petr Pavlu <petr.pavlu@suse.com>
--
Thanks,
Petr
^ permalink raw reply
* Re: [PATCH v3 3/4] module: centralize no-versions force load check
From: Petr Pavlu @ 2025-09-01 9:30 UTC (permalink / raw)
To: Jinchao Wang
Cc: Luis Chamberlain, Daniel Gomez, Sami Tolvanen, linux-modules,
linux-kernel
In-Reply-To: <20250829084927.156676-4-wangjinchao600@gmail.com>
On 8/29/25 10:49 AM, Jinchao Wang wrote:
> Move try_to_force_load() call from check_version() to
> check_modstruct_version() to handle "no versions" case only once before
> other version checks.
>
> Suggested-by: Petr Pavlu <petr.pavlu@suse.com>
> Signed-off-by: Jinchao Wang <wangjinchao600@gmail.com>
> ---
> kernel/module/version.c | 10 ++++++++--
> 1 file changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/kernel/module/version.c b/kernel/module/version.c
> index 2beefeba82d9..7a458c681049 100644
> --- a/kernel/module/version.c
> +++ b/kernel/module/version.c
> @@ -41,9 +41,9 @@ int check_version(const struct load_info *info,
> return 1;
> }
>
> - /* No versions at all? modprobe --force does this. */
> + /* No versions? Ok, already tainted in check_modstruct_version(). */
> if (versindex == 0)
> - return try_to_force_load(mod, symname) == 0;
> + return 1;
>
> versions = (void *)sechdrs[versindex].sh_addr;
> num_versions = sechdrs[versindex].sh_size
> @@ -80,6 +80,7 @@ int check_modstruct_version(const struct load_info *info,
> .gplok = true,
> };
> bool have_symbol;
> + char *reason;
>
> /*
> * Since this should be found in kernel (which can't be removed), no
> @@ -90,6 +91,11 @@ int check_modstruct_version(const struct load_info *info,
> have_symbol = find_symbol(&fsa);
> BUG_ON(!have_symbol);
>
> + /* No versions at all? modprobe --force does this. */
> + if (!info->index.vers && !info->index.vers_ext_crc) {
> + reason = "no versions for imported symbols";
> + return try_to_force_load(mod, reason) == 0;
> + }
> return check_version(info, "module_layout", mod, fsa.crc);
> }
>
Nit: I would prefer this to be formatted as:
+ /* No versions at all? modprobe --force does this. */
+ if (!info->index.vers && !info->index.vers_ext_crc)
+ return try_to_force_load(
+ mod, "no versions for imported symbols") == 0;
+
Nonetheless, it looks ok to me functionality-wise.
Reviewed-by: Petr Pavlu <petr.pavlu@suse.com>
--
Thanks,
Petr
^ permalink raw reply
* Re: [PATCH v3 2/4] module: show why force load fails
From: Petr Pavlu @ 2025-09-01 9:26 UTC (permalink / raw)
To: Jinchao Wang
Cc: Luis Chamberlain, Daniel Gomez, Sami Tolvanen, linux-modules,
linux-kernel
In-Reply-To: <20250829084927.156676-3-wangjinchao600@gmail.com>
On 8/29/25 10:49 AM, Jinchao Wang wrote:
> Include reason in error message when force loading is disabled.
>
> Signed-off-by: Jinchao Wang <wangjinchao600@gmail.com>
Reviewed-by: Petr Pavlu <petr.pavlu@suse.com>
--
Thanks,
Petr
^ permalink raw reply
* Re: [PATCH v3 1/4] module: signing: Use pr_err for signature rejection
From: Petr Pavlu @ 2025-09-01 9:25 UTC (permalink / raw)
To: Jinchao Wang
Cc: Luis Chamberlain, Daniel Gomez, Sami Tolvanen, linux-modules,
linux-kernel
In-Reply-To: <20250829084927.156676-2-wangjinchao600@gmail.com>
On 8/29/25 10:49 AM, Jinchao Wang wrote:
> Make module signature rejection messages more visible by using pr_err
> instead of pr_notice.
>
> Signed-off-by: Jinchao Wang <wangjinchao600@gmail.com>
Reviewed-by: Petr Pavlu <petr.pavlu@suse.com>
--
Thanks,
Petr
^ permalink raw reply
* [PATCH 10/10] module loader: enforce symbol import protection
From: Siddharth Nayyar @ 2025-08-29 10:54 UTC (permalink / raw)
To: Nathan Chancellor, Luis Chamberlain, Sami Tolvanen
Cc: Nicolas Schier, Petr Pavlu, Arnd Bergmann, linux-kbuild,
linux-arch, linux-modules, linux-kernel, Siddharth Nayyar
In-Reply-To: <20250829105418.3053274-1-sidnayyar@google.com>
The module loader will reject unsigned modules from loading if such a
module attempts to import a symbol which has the import protection bit
set in the kflagstab entry for the symbol.
Signed-off-by: Siddharth Nayyar <sidnayyar@google.com>
---
kernel/module/internal.h | 1 +
kernel/module/main.c | 14 ++++++++++++--
2 files changed, 13 insertions(+), 2 deletions(-)
diff --git a/kernel/module/internal.h b/kernel/module/internal.h
index 061161cc79d9..98faaf8900aa 100644
--- a/kernel/module/internal.h
+++ b/kernel/module/internal.h
@@ -108,6 +108,7 @@ struct find_symbol_arg {
const u32 *crc;
const struct kernel_symbol *sym;
enum mod_license license;
+ bool is_protected;
};
/* modules using other modules */
diff --git a/kernel/module/main.c b/kernel/module/main.c
index 4437c2a451ea..ece074a6ba7b 100644
--- a/kernel/module/main.c
+++ b/kernel/module/main.c
@@ -380,6 +380,7 @@ static bool find_exported_symbol_in_section(const struct symsearch *syms,
fsa->crc = symversion(syms->crcs, sym - syms->start);
fsa->sym = sym;
fsa->license = (sym_flags & KSYM_FLAG_GPL_ONLY) ? GPL_ONLY : NOT_GPL_ONLY;
+ fsa->is_protected = sym_flags & KSYM_FLAG_PROTECTED;
return true;
}
@@ -1273,6 +1274,11 @@ static const struct kernel_symbol *resolve_symbol(struct module *mod,
goto getname;
}
+ if (fsa.is_protected && !mod->sig_ok) {
+ fsa.sym = ERR_PTR(-EACCES);
+ goto getname;
+ }
+
getname:
/* We must make copy under the lock if we failed to get ref. */
strscpy(ownername, module_name(fsa.owner), MODULE_NAME_LEN);
@@ -1550,8 +1556,12 @@ static int simplify_symbols(struct module *mod, const struct load_info *info)
break;
ret = PTR_ERR(ksym) ?: -ENOENT;
- pr_warn("%s: Unknown symbol %s (err %d)\n",
- mod->name, name, ret);
+ if (ret == -EACCES)
+ pr_warn("%s: Protected symbol %s (err %d)\n",
+ mod->name, name, ret);
+ else
+ pr_warn("%s: Unknown symbol %s (err %d)\n",
+ mod->name, name, ret);
break;
default:
--
2.51.0.338.gd7d06c2dae-goog
^ permalink raw reply related
* [PATCH 09/10] modpost: add symbol import protection flag to kflagstab
From: Siddharth Nayyar @ 2025-08-29 10:54 UTC (permalink / raw)
To: Nathan Chancellor, Luis Chamberlain, Sami Tolvanen
Cc: Nicolas Schier, Petr Pavlu, Arnd Bergmann, linux-kbuild,
linux-arch, linux-modules, linux-kernel, Siddharth Nayyar
In-Reply-To: <20250829105418.3053274-1-sidnayyar@google.com>
When the unused exports whitelist is provided, the symbol protection bit
is set for symbols not present in the unused exports whitelist.
The flag will be used in the following commit to prevent unsigned
modules from the using symbols other than those explicitly declared by
the such modules ahead of time.
Signed-off-by: Siddharth Nayyar <sidnayyar@google.com>
---
include/linux/module_symbol.h | 3 ++-
scripts/mod/modpost.c | 13 +++++++++++--
2 files changed, 13 insertions(+), 3 deletions(-)
diff --git a/include/linux/module_symbol.h b/include/linux/module_symbol.h
index 574609aced99..96fe3f4d7424 100644
--- a/include/linux/module_symbol.h
+++ b/include/linux/module_symbol.h
@@ -3,8 +3,9 @@
#define _LINUX_MODULE_SYMBOL_H
/* Kernel symbol flags bitset. */
-enum ksym_flags {
+enum symbol_flags {
KSYM_FLAG_GPL_ONLY = 1 << 0,
+ KSYM_FLAG_PROTECTED = 1 << 1,
};
/* This ignores the intensely annoying "mapping symbols" found in ELF files. */
diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
index 8936db84779b..8d360bab50d6 100644
--- a/scripts/mod/modpost.c
+++ b/scripts/mod/modpost.c
@@ -61,6 +61,9 @@ static bool extra_warn;
bool target_is_big_endian;
bool host_is_big_endian;
+/* Are symbols protected against being used by unsigned modules? */
+static bool default_symbol_protected_status;
+
/*
* Cut off the warnings when there are too many. This typically occurs when
* vmlinux is missing. ('make modules' without building vmlinux.)
@@ -225,6 +228,7 @@ struct symbol {
bool is_func;
bool is_gpl_only; /* exported by EXPORT_SYMBOL_GPL */
bool used; /* there exists a user of this symbol */
+ bool protected; /* this symbol cannot be used by unsigned modules */
char name[];
};
@@ -246,7 +250,8 @@ static struct symbol *alloc_symbol(const char *name)
static uint8_t get_symbol_flags(const struct symbol *sym)
{
- return sym->is_gpl_only ? KSYM_FLAG_GPL_ONLY : 0;
+ return (sym->is_gpl_only ? KSYM_FLAG_GPL_ONLY : 0) |
+ (sym->protected ? KSYM_FLAG_PROTECTED : 0);
}
/* For the hash of exported symbols */
@@ -370,6 +375,7 @@ static struct symbol *sym_add_exported(const char *name, struct module *mod,
s->namespace = xstrdup(namespace);
list_add_tail(&s->list, &mod->exported_symbols);
hash_add_symbol(s);
+ s->protected = default_symbol_protected_status;
return s;
}
@@ -1785,8 +1791,10 @@ static void handle_white_list_exports(const char *white_list)
while ((name = strsep(&p, "\n"))) {
struct symbol *sym = find_symbol(name);
- if (sym)
+ if (sym) {
sym->used = true;
+ sym->protected = false;
+ }
}
free(buf);
@@ -2294,6 +2302,7 @@ int main(int argc, char **argv)
break;
case 'u':
unused_exports_white_list = optarg;
+ default_symbol_protected_status = true;
break;
case 'W':
extra_warn = true;
--
2.51.0.338.gd7d06c2dae-goog
^ permalink raw reply related
* [PATCH 08/10] remove references to *_gpl sections in documentation
From: Siddharth Nayyar @ 2025-08-29 10:54 UTC (permalink / raw)
To: Nathan Chancellor, Luis Chamberlain, Sami Tolvanen
Cc: Nicolas Schier, Petr Pavlu, Arnd Bergmann, linux-kbuild,
linux-arch, linux-modules, linux-kernel, Siddharth Nayyar
In-Reply-To: <20250829105418.3053274-1-sidnayyar@google.com>
Signed-off-by: Siddharth Nayyar <sidnayyar@google.com>
---
Documentation/kbuild/modules.rst | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/Documentation/kbuild/modules.rst b/Documentation/kbuild/modules.rst
index d0703605bfa4..f2022fa2342f 100644
--- a/Documentation/kbuild/modules.rst
+++ b/Documentation/kbuild/modules.rst
@@ -426,11 +426,11 @@ 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,
+ Exported symbols have information stored in the __ksymtab section.
+ 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 or __kcrctab_gpl.
+ 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.0.338.gd7d06c2dae-goog
^ permalink raw reply related
* [PATCH 07/10] linker: remove *_gpl sections from vmlinux and modules
From: Siddharth Nayyar @ 2025-08-29 10:54 UTC (permalink / raw)
To: Nathan Chancellor, Luis Chamberlain, Sami Tolvanen
Cc: Nicolas Schier, Petr Pavlu, Arnd Bergmann, linux-kbuild,
linux-arch, linux-modules, linux-kernel, Siddharth Nayyar
In-Reply-To: <20250829105418.3053274-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>
---
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.0.338.gd7d06c2dae-goog
^ permalink raw reply related
* [PATCH 06/10] module loader: remove references of *_gpl sections
From: Siddharth Nayyar @ 2025-08-29 10:54 UTC (permalink / raw)
To: Nathan Chancellor, Luis Chamberlain, Sami Tolvanen
Cc: Nicolas Schier, Petr Pavlu, Arnd Bergmann, linux-kbuild,
linux-arch, linux-modules, linux-kernel, Siddharth Nayyar
In-Reply-To: <20250829105418.3053274-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>
---
include/linux/module.h | 3 ---
kernel/module/internal.h | 3 ---
kernel/module/main.c | 40 ++++++++++++----------------------------
3 files changed, 12 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 400d59a7f44b..4437c2a451ea 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,10 +2590,6 @@ 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");
#ifdef CONFIG_CONSTRUCTORS
@@ -2812,8 +2797,7 @@ static int move_module(struct module *mod, struct load_info *info)
static int check_export_symbol_versions(struct module *mod)
{
#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.0.338.gd7d06c2dae-goog
^ permalink raw reply related
* [PATCH 05/10] modpost: put all exported symbols in ksymtab section
From: Siddharth Nayyar @ 2025-08-29 10:54 UTC (permalink / raw)
To: Nathan Chancellor, Luis Chamberlain, Sami Tolvanen
Cc: Nicolas Schier, Petr Pavlu, Arnd Bergmann, linux-kbuild,
linux-arch, linux-modules, linux-kernel, Siddharth Nayyar
In-Reply-To: <20250829105418.3053274-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>
---
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.0.338.gd7d06c2dae-goog
^ permalink raw reply related
* [PATCH 04/10] module loader: use kflagstab instead of *_gpl sections
From: Siddharth Nayyar @ 2025-08-29 10:54 UTC (permalink / raw)
To: Nathan Chancellor, Luis Chamberlain, Sami Tolvanen
Cc: Nicolas Schier, Petr Pavlu, Arnd Bergmann, linux-kbuild,
linux-arch, linux-modules, linux-kernel, Siddharth Nayyar
In-Reply-To: <20250829105418.3053274-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>
---
include/linux/module.h | 1 +
kernel/module/internal.h | 1 +
kernel/module/main.c | 47 ++++++++++++++++++++--------------------
3 files changed, 25 insertions(+), 24 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..400d59a7f44b 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",
--
2.51.0.338.gd7d06c2dae-goog
^ permalink raw reply related
* [PATCH 03/10] modpost: create entries for kflagstab
From: Siddharth Nayyar @ 2025-08-29 10:54 UTC (permalink / raw)
To: Nathan Chancellor, Luis Chamberlain, Sami Tolvanen
Cc: Nicolas Schier, Petr Pavlu, Arnd Bergmann, linux-kbuild,
linux-arch, linux-modules, linux-kernel, Siddharth Nayyar
In-Reply-To: <20250829105418.3053274-1-sidnayyar@google.com>
Signed-off-by: Siddharth Nayyar <sidnayyar@google.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.0.338.gd7d06c2dae-goog
^ permalink raw reply related
* [PATCH 02/10] linker: add kflagstab section to vmlinux and modules
From: Siddharth Nayyar @ 2025-08-29 10:54 UTC (permalink / raw)
To: Nathan Chancellor, Luis Chamberlain, Sami Tolvanen
Cc: Nicolas Schier, Petr Pavlu, Arnd Bergmann, linux-kbuild,
linux-arch, linux-modules, linux-kernel, Siddharth Nayyar
In-Reply-To: <20250829105418.3053274-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>
---
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.0.338.gd7d06c2dae-goog
^ permalink raw reply related
* [PATCH 01/10] define kernel symbol flags
From: Siddharth Nayyar @ 2025-08-29 10:54 UTC (permalink / raw)
To: Nathan Chancellor, Luis Chamberlain, Sami Tolvanen
Cc: Nicolas Schier, Petr Pavlu, Arnd Bergmann, linux-kbuild,
linux-arch, linux-modules, linux-kernel, Siddharth Nayyar
In-Reply-To: <20250829105418.3053274-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>
---
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.0.338.gd7d06c2dae-goog
^ permalink raw reply related
* [RFC PATCH 00/10] scalable symbol flags with __kflagstab
From: Siddharth Nayyar @ 2025-08-29 10:54 UTC (permalink / raw)
To: Nathan Chancellor, Luis Chamberlain, Sami Tolvanen
Cc: Nicolas Schier, Petr Pavlu, Arnd Bergmann, linux-kbuild,
linux-arch, linux-modules, linux-kernel, Siddharth Nayyar
Hi everyone,
This patch series proposes a new, scalable mechanism to represent
boolean flags for exported kernel symbols.
Problem Statement:
The core architectural issue with kernel symbol flags is our reliance on
splitting the main symbol table, ksymtab. To handle a single boolean
property, such as GPL-only, all exported symbols are split across two
separate tables: __ksymtab and __ksymtab_gpl.
This design forces the module loader to perform a separate search on
each of these tables for every symbol it needs, for vmlinux and for all
previously loaded modules.
This approach is fundamentally not scalable. If we were to introduce a
second flag, we would need four distinct symbol tables. For n boolean
flags, this model requires an exponential growth to 2^n tables,
dramatically increasing complexity.
Another consequence of this fragmentation is degraded performance. For
example, a binary search on the symbol table of vmlinux, that would take
only 14 comparison steps (assuming ~2^14 or 16K symbols) in a unified
table, can require up to 26 steps when spread across two tables
(assuming both tables have ~2^13 symbols). This performance penalty
worsens as more flags are added.
Proposed Solution:
This series introduces a __kflagstab section to store symbol flags in a
dedicated data structure, similar to how CRCs are handled in the
__kcrctab.
The flags for a given symbol in __kflagstab will be located at the same
index as the symbol's entry in __ksymtab and its CRC in __kcrctab. This
design decouples the flags from the symbol table itself, allowing us to
maintain a single, sorted __ksymtab. As a result, the symbol search
remains an efficient, single lookup, regardless of the number of flags
we add in the future.
The motivation for this change comes from the Android kernel, which uses
an additional symbol flag to restrict the use of certain exported
symbols by unsigned modules, thereby enhancing kernel security. This
__kflagstab can be implemented as a bitmap to efficiently manage which
symbols are available for general use versus those restricted to signed
modules only.
Patch Series Overview:
* Patch 1-8: Introduce the __kflagstab, migrate the existing GPL-only
flag to this new mechanism, and clean up the old __ksymtab_gpl
infrastructure.
* Patch 9-10: Add a "symbol import protection" flag,
which disallows unsigned modules from importing symbols marked with
this flag.
This is an RFC, and I am seeking feedback on the overall approach and
implementation before moving forward.
Thanks,
Siddharth Nayyar
Siddharth Nayyar (10):
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
modpost: add symbol import protection flag to kflagstab
module loader: enforce symbol import protection
Documentation/kbuild/modules.rst | 6 +-
include/asm-generic/vmlinux.lds.h | 21 +++----
include/linux/export-internal.h | 28 ++++++---
include/linux/module.h | 4 +-
include/linux/module_symbol.h | 6 ++
kernel/module/internal.h | 5 +-
kernel/module/main.c | 101 ++++++++++++++----------------
scripts/mod/modpost.c | 27 ++++++--
scripts/module.lds.S | 3 +-
9 files changed, 107 insertions(+), 94 deletions(-)
--
2.51.0.338.gd7d06c2dae-goog
^ permalink raw reply
* [PATCH v3 4/4] module: separate vermagic and livepatch checks
From: Jinchao Wang @ 2025-08-29 8:49 UTC (permalink / raw)
To: Luis Chamberlain, Petr Pavlu, Daniel Gomez, Sami Tolvanen,
linux-modules, linux-kernel
Cc: Jinchao Wang
In-Reply-To: <20250829084927.156676-1-wangjinchao600@gmail.com>
Rename check_modinfo() to check_modinfo_vermagic() to clarify it only
checks vermagic compatibility.
Move livepatch check to happen after vermagic check in early_mod_check(),
creating proper separation of concerns.
No functional changes, just clearer code organization.
Signed-off-by: Jinchao Wang <wangjinchao600@gmail.com>
---
kernel/module/main.c | 13 +++++++------
1 file changed, 7 insertions(+), 6 deletions(-)
diff --git a/kernel/module/main.c b/kernel/module/main.c
index e7484c6ce6e3..98a678a18300 100644
--- a/kernel/module/main.c
+++ b/kernel/module/main.c
@@ -2571,7 +2571,8 @@ static void module_augment_kernel_taints(struct module *mod, struct load_info *i
}
-static int check_modinfo(struct module *mod, struct load_info *info, int flags)
+static int check_modinfo_vermagic(struct module *mod, struct load_info *info,
+ int flags)
{
const char *modmagic = get_modinfo(info, "vermagic");
int err;
@@ -2590,10 +2591,6 @@ static int check_modinfo(struct module *mod, struct load_info *info, int flags)
return -ENOEXEC;
}
- err = check_modinfo_livepatch(mod, info);
- if (err)
- return err;
-
return 0;
}
@@ -3334,7 +3331,11 @@ static int early_mod_check(struct load_info *info, int flags)
if (!check_modstruct_version(info, info->mod))
return -ENOEXEC;
- err = check_modinfo(info->mod, info, flags);
+ err = check_modinfo_vermagic(info->mod, info, flags);
+ if (err)
+ return err;
+
+ err = check_modinfo_livepatch(info->mod, info);
if (err)
return err;
--
2.43.0
^ permalink raw reply related
* [PATCH v3 3/4] module: centralize no-versions force load check
From: Jinchao Wang @ 2025-08-29 8:49 UTC (permalink / raw)
To: Luis Chamberlain, Petr Pavlu, Daniel Gomez, Sami Tolvanen,
linux-modules, linux-kernel
Cc: Jinchao Wang
In-Reply-To: <20250829084927.156676-1-wangjinchao600@gmail.com>
Move try_to_force_load() call from check_version() to
check_modstruct_version() to handle "no versions" case only once before
other version checks.
Suggested-by: Petr Pavlu <petr.pavlu@suse.com>
Signed-off-by: Jinchao Wang <wangjinchao600@gmail.com>
---
kernel/module/version.c | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/kernel/module/version.c b/kernel/module/version.c
index 2beefeba82d9..7a458c681049 100644
--- a/kernel/module/version.c
+++ b/kernel/module/version.c
@@ -41,9 +41,9 @@ int check_version(const struct load_info *info,
return 1;
}
- /* No versions at all? modprobe --force does this. */
+ /* No versions? Ok, already tainted in check_modstruct_version(). */
if (versindex == 0)
- return try_to_force_load(mod, symname) == 0;
+ return 1;
versions = (void *)sechdrs[versindex].sh_addr;
num_versions = sechdrs[versindex].sh_size
@@ -80,6 +80,7 @@ int check_modstruct_version(const struct load_info *info,
.gplok = true,
};
bool have_symbol;
+ char *reason;
/*
* Since this should be found in kernel (which can't be removed), no
@@ -90,6 +91,11 @@ int check_modstruct_version(const struct load_info *info,
have_symbol = find_symbol(&fsa);
BUG_ON(!have_symbol);
+ /* No versions at all? modprobe --force does this. */
+ if (!info->index.vers && !info->index.vers_ext_crc) {
+ reason = "no versions for imported symbols";
+ return try_to_force_load(mod, reason) == 0;
+ }
return check_version(info, "module_layout", mod, fsa.crc);
}
--
2.43.0
^ permalink raw reply related
* [PATCH v3 2/4] module: show why force load fails
From: Jinchao Wang @ 2025-08-29 8:49 UTC (permalink / raw)
To: Luis Chamberlain, Petr Pavlu, Daniel Gomez, Sami Tolvanen,
linux-modules, linux-kernel
Cc: Jinchao Wang
In-Reply-To: <20250829084927.156676-1-wangjinchao600@gmail.com>
Include reason in error message when force loading is disabled.
Signed-off-by: Jinchao Wang <wangjinchao600@gmail.com>
---
kernel/module/main.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/kernel/module/main.c b/kernel/module/main.c
index c66b26184936..e7484c6ce6e3 100644
--- a/kernel/module/main.c
+++ b/kernel/module/main.c
@@ -1083,6 +1083,7 @@ int try_to_force_load(struct module *mod, const char *reason)
add_taint_module(mod, TAINT_FORCED_MODULE, LOCKDEP_NOW_UNRELIABLE);
return 0;
#else
+ pr_err("%s: %s, force load is not supported\n", mod->name, reason);
return -ENOEXEC;
#endif
}
--
2.43.0
^ permalink raw reply related
* [PATCH v3 1/4] module: signing: Use pr_err for signature rejection
From: Jinchao Wang @ 2025-08-29 8:49 UTC (permalink / raw)
To: Luis Chamberlain, Petr Pavlu, Daniel Gomez, Sami Tolvanen,
linux-modules, linux-kernel
Cc: Jinchao Wang
In-Reply-To: <20250829084927.156676-1-wangjinchao600@gmail.com>
Make module signature rejection messages more visible by using pr_err
instead of pr_notice.
Signed-off-by: Jinchao Wang <wangjinchao600@gmail.com>
---
kernel/module/signing.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/kernel/module/signing.c b/kernel/module/signing.c
index a2ff4242e623..557cb795fa31 100644
--- a/kernel/module/signing.c
+++ b/kernel/module/signing.c
@@ -117,7 +117,7 @@ int module_sig_check(struct load_info *info, int flags)
}
if (is_module_sig_enforced()) {
- pr_notice("Loading of %s is rejected\n", reason);
+ pr_err("Loading of %s is rejected\n", reason);
return -EKEYREJECTED;
}
--
2.43.0
^ permalink raw reply related
* [PATCH v3 0/4] module: logging and code improvements
From: Jinchao Wang @ 2025-08-29 8:49 UTC (permalink / raw)
To: Luis Chamberlain, Petr Pavlu, Daniel Gomez, Sami Tolvanen,
linux-modules, linux-kernel
Cc: Jinchao Wang
This series of patches cleans up and refactors the kernel's module loading
code. The goal is to make the subsystem's logging clearer and its internal
logic more straightforward for developers to understand.
The patches in this series:
- module: signing: Use pr_err for signature rejection
Makes module signature rejection messages more visible.
- module: show why force load fails
Adds a reason to the error message when force loading is disabled.
- module: centralize no-versions force load check
Refactors the code to centralize the "no versions" force load check.
- module: separate vermagic and livepatch checks
Improves code organization by separating vermagic and livepatch checks.
---
Changes from v2:
- show mod->name in try_to_force_load
- fix a introduced bug in patch 3
Changes from v1:
- A patch was dropped because it was based on a misunderstanding
of the ignore versioning flag's original intent.
v2:
https://lore.kernel.org/all/20250825091545.18607-1-wangjinchao600@gmail.com/
v1 :
https://lore.kernel.org/all/20250822125454.1287066-1-wangjinchao600@gmail.com
Jinchao Wang (4):
module: signing: Use pr_err for signature rejection
module: show why force load fails
module: centralize no-versions force load check
module: separate vermagic and livepatch checks
kernel/module/main.c | 14 ++++++++------
kernel/module/signing.c | 2 +-
kernel/module/version.c | 10 ++++++++--
3 files changed, 17 insertions(+), 9 deletions(-)
--
2.43.0
^ permalink raw reply
* [Question] Non-usage of PKEY_ID_PGP and PKEY_ID_X509 in module signing
From: Yunseong Kim @ 2025-08-26 18:58 UTC (permalink / raw)
To: Luis Chamberlain, Petr Pavlu, Daniel Gomez,
Sami Tolvanen <samitolvanen@google.com> David Howells,
David Woodhouse
Cc: linux-modules, keyrings, linux-kernel
I would like to inquire about the purpose of the PKEY_ID_PGP and
PKEY_ID_X509 identifiers defined in include/linux/module_signature.h.
The enum pkey_id_type is defined as follows:
enum pkey_id_type {
PKEY_ID_PGP, /* OpenPGP generated key ID */
PKEY_ID_X509, /* X.509 arbitrary subjectKeyIdentifier */
PKEY_ID_PKCS7, /* Signature in PKCS#7 message */
};
While examining the module signing and verification process, it appears
that the current implementation strictly assumes the use of PKCS#7, making
PKEY_ID_PGP and PKEY_ID_X509 seem unused in this context.
I observed the following:
1. In scripts/sign-file.c, the module_signature structure is explicitly
initialized assuming PKCS#7:
/* Key identifier type [PKEY_ID_PKCS7] */
struct module_signature sig_info = { .id_type = PKEY_ID_PKCS7 };
2. In kernel/module_signature.c, the verification function mod_check_sig()
strictly enforces this type and rejects others:
int mod_check_sig(const struct module_signature *ms, size_t file_len,
const char *name)
{
if (be32_to_cpu(ms->sig_len) >= file_len - sizeof(*ms))
return -EBADMSG;
if (ms->id_type != PKEY_ID_PKCS7) {
pr_err("%s: not signed with expected PKCS#7 message\n",
name);
return -ENOPKG;
}
// ...
}
3. Furthermore, I noticed that certs/extract-cert.c only defines
PKEY_ID_PKCS7 locally, seemingly without utilizing the definitions from
the header for the other types:
#define PKEY_ID_PKCS7 2
Given that the module signature infrastructure seems hardcoded to use
PKCS#7, could anyone clarify if PKEY_ID_PGP and PKEY_ID_X509 are used
elsewhere in the kernel? Are they perhaps placeholders for future
implementations or remnants of past ones?
If they are indeed unused and there are no plans to support them, would
a patch to clean up these unused enum values be welcome? Or is there
another reason for keeping them?
Thank you for your time and clarification.
Best regards,
Yunseong Kim
^ permalink raw reply
* Re: [PATCH v2 2/4] module: show why force load fails
From: Jinchao Wang @ 2025-08-26 9:51 UTC (permalink / raw)
To: Petr Pavlu
Cc: Luis Chamberlain, Daniel Gomez, Sami Tolvanen, linux-modules,
linux-kernel
In-Reply-To: <52288605-a16c-4a93-9e80-66d32de88847@suse.com>
On 8/26/25 17:33, Petr Pavlu wrote:
> On 8/25/25 11:15 AM, Jinchao Wang wrote:
>> Include reason in error message when force loading is disabled.
>>
>> Signed-off-by: Jinchao Wang <wangjinchao600@gmail.com>
>> ---
>> kernel/module/main.c | 1 +
>> 1 file changed, 1 insertion(+)
>>
>> diff --git a/kernel/module/main.c b/kernel/module/main.c
>> index c66b26184936..a426bd8a18b5 100644
>> --- a/kernel/module/main.c
>> +++ b/kernel/module/main.c
>> @@ -1083,6 +1083,7 @@ int try_to_force_load(struct module *mod, const char *reason)
>> add_taint_module(mod, TAINT_FORCED_MODULE, LOCKDEP_NOW_UNRELIABLE);
>> return 0;
>> #else
>> + pr_err("%s force load is not supported\n", reason);
>> return -ENOEXEC;
>> #endif
>> }
>
> The module name is already available at all points where
> try_to_force_load() is called, so the new error message should include
> it.
>
> Additionally, we should be careful about the message. In the case of the
> init_module syscall, the missing modversions and vermagic could mean
> that the data was deliberately stripped by kmod because the module was
> inserted with --force, or it could mean that the module lacks this data
> in the first place. In other words, it is not always the case that that
> we're reaching this logic because of a force load.
>
> My suggestion would be to use the following:
>
> pr_err("%s: %s, force load is not supported\n", mod->name, reason);
>
Good suggestion. Thanks.
--
Best regards,
Jinchao
^ permalink raw reply
* Re: [PATCH] module: pr_debug when there is no version info
From: Jinchao Wang @ 2025-08-26 9:45 UTC (permalink / raw)
To: Petr Pavlu
Cc: Luis Chamberlain, Daniel Gomez, Sami Tolvanen, linux-kernel,
linux-modules
In-Reply-To: <80d7313a-0f0c-494d-b2ad-2662d1992b2b@suse.com>
On 8/26/25 15:20, Petr Pavlu wrote:
> On 7/22/25 10:25 AM, Petr Pavlu wrote:
>> On 7/22/25 5:08 AM, Wang Jinchao wrote:
>>> On 7/21/25 22:40, Petr Pavlu wrote:
>>>> On 7/21/25 6:52 AM, Wang Jinchao wrote:
>>>>> When there is no version information, modprobe and insmod only
>>>>> report "invalid format".
>>>>> Print the actual cause to make it easier to diagnose the issue.
>>>>> This helps developers quickly identify version-related module
>>>>> loading failures.
>>>>> Signed-off-by: Wang Jinchao <wangjinchao600@gmail.com>
>>>>> ---
>>>>> kernel/module/version.c | 4 +++-
>>>>> 1 file changed, 3 insertions(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/kernel/module/version.c b/kernel/module/version.c
>>>>> index 2beefeba82d9..bc28c697ff3a 100644
>>>>> --- a/kernel/module/version.c
>>>>> +++ b/kernel/module/version.c
>>>>> @@ -42,8 +42,10 @@ int check_version(const struct load_info *info,
>>>>> }
>>>>> /* No versions at all? modprobe --force does this. */
>>>>> - if (versindex == 0)
>>>>> + if (versindex == 0) {
>>>>> + pr_debug("No version info for module %s\n", info->name);
>>>>> return try_to_force_load(mod, symname) == 0;
>>>>> + }
>>>>> versions = (void *)sechdrs[versindex].sh_addr;
>>>>> num_versions = sechdrs[versindex].sh_size
>>>>
>>>> I think it would be better to instead improve the behavior of
>>>> try_to_force_load(). The function should print the error reason prior to
>>>> returning -ENOEXEC. This would also help its two other callers,
>>>> check_modinfo() and check_export_symbol_versions().
>>>>
>>>> Additionally, I suggest moving the check to ensure version information
>>>> is available for imported symbols earlier in the loading process.
>>>> A suitable place might be check_modstruct_version(). This way the check
>>>> is performed only once per module.
>>>>
>>>> The following is a prototype patch:
>>>>
>>>> diff --git a/kernel/module/main.c b/kernel/module/main.c
>>>> index c2c08007029d..c1ccd343e8c3 100644
>>>> --- a/kernel/module/main.c
>>>> +++ b/kernel/module/main.c
>>>> @@ -1053,6 +1053,7 @@ int try_to_force_load(struct module *mod, const char *reason)
>>>> add_taint_module(mod, TAINT_FORCED_MODULE, LOCKDEP_NOW_UNRELIABLE);
>>>> return 0;
>>>> #else
>>>> + pr_err("%s: %s\n", mod->name, reason);
>>>> return -ENOEXEC;
>>>> #endif
>>>> }
>>>> diff --git a/kernel/module/version.c b/kernel/module/version.c
>>>> index 2beefeba82d9..4d9ebf0834de 100644
>>>> --- a/kernel/module/version.c
>>>> +++ b/kernel/module/version.c
>>>> @@ -41,9 +41,9 @@ int check_version(const struct load_info *info,
>>>> return 1;
>>>> }
>>>> - /* No versions at all? modprobe --force does this. */
>>>> + /* No versions? Ok, already tainted in check_modstruct_version(). */
>>>> if (versindex == 0)
>>>> - return try_to_force_load(mod, symname) == 0;
>>>> + return 1;
>>>> versions = (void *)sechdrs[versindex].sh_addr;
>>>> num_versions = sechdrs[versindex].sh_size
>>>> @@ -90,6 +90,11 @@ int check_modstruct_version(const struct load_info *info,
>>>> have_symbol = find_symbol(&fsa);
>>>> BUG_ON(!have_symbol);
>>>> + /* No versions at all? modprobe --force does this. */
>>>> + if (!info->index.vers && !info->index.vers_ext_crc)
>>>> + return try_to_force_load(
>>>> + mod, "no versions for imported symbols") == 0;
>>>> +
>>>> return check_version(info, "module_layout", mod, fsa.crc);
>>>> }
>>>>
>>>> As a side note, I'm confused why with CONFIG_MODULE_FORCE_LOAD=y, the
>>>> code treats missing modversions for imported symbols as ok, even without
>>>> MODULE_INIT_IGNORE_MODVERSIONS. This is at least consistent with the
>>>> handling of missing vermagic, but it seems this behavior should be
>>>> stricter.
>>>>
>>> When debugging syzkaller, I noticed that the insmod command always reports errors. However, to get the exact information, I need to trace the kernel, so I casually submitted this patch.
>>>
>>> Based on your response, I also feel that the meaning of force_load here is somewhat unclear. It would be better to create a mask or a clear list to indicate which fields can be forced and which cannot. Once this is clear, we can create a function named may_force_check().
>>
>> I cannot find an explicit reason in the Git history why a missing
>> vermagic is treated as if the module was loaded with
>> MODULE_INIT_IGNORE_VERMAGIC, and similarly why missing modversions data
>> is treated as if the module was loaded with
>> MODULE_INIT_IGNORE_MODVERSIONS.
>>
>> I would argue that a more sensible behavior would be to consider
>> a missing vermagic as an error and allow loading the module only if
>> MODULE_INIT_IGNORE_VERMAGIC is explicitly specified. And analogously for
>> missing modversions and MODULE_INIT_IGNORE_MODVERSIONS.
>>
>> Nonetheless, if I understand correctly, this should be mostly separate
>> from your issue.
>
> To answer my own confusion, the thing that I missed is that the
> MODULE_INIT_IGNORE_* flags are available only for the finit_module
> syscall, not for init_module. In the case of init_module, the force
> logic is handled by kmod in userspace by stripping the relevant
> modversions and vermagic data. This means that when using init_module,
> the module loader cannot distinguish between a module that lacks this
> data and one that has it deliberately removed. When finit_module was
> introduced in 2012, commit 2f3238aebedb ("module: add flags arg to
> sys_finit_module()") added the MODULE_INIT_IGNORE_* flags, and they were
> simply implemented to mirror the kmod behavior.
>
> -- Petr
The composition of 'force' and 'ignore' is confusing.
I learn much from your feedback, thank you very much.
--
Best regards,
Jinchao
^ permalink raw reply
* Re: [PATCH v2 3/4] module: centralize no-versions force load check
From: Petr Pavlu @ 2025-08-26 9:40 UTC (permalink / raw)
To: Jinchao Wang
Cc: Luis Chamberlain, Daniel Gomez, Sami Tolvanen, linux-modules,
linux-kernel
In-Reply-To: <20250825091545.18607-4-wangjinchao600@gmail.com>
On 8/25/25 11:15 AM, Jinchao Wang wrote:
> Move try_to_force_load() call from check_version() to
> check_modstruct_version() to handle "no versions" case only once before
> other version checks.
>
> This prevents duplicate force load attempts and makes the error message
> show the proper reason.
>
> Signed-off-by: Jinchao Wang <wangjinchao600@gmail.com>
> ---
> kernel/module/version.c | 9 +++++----
> 1 file changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/kernel/module/version.c b/kernel/module/version.c
> index 2beefeba82d9..3f07fd03cb30 100644
> --- a/kernel/module/version.c
> +++ b/kernel/module/version.c
> @@ -41,10 +41,6 @@ int check_version(const struct load_info *info,
> return 1;
> }
>
> - /* No versions at all? modprobe --force does this. */
> - if (versindex == 0)
> - return try_to_force_load(mod, symname) == 0;
> -
> versions = (void *)sechdrs[versindex].sh_addr;
> num_versions = sechdrs[versindex].sh_size
> / sizeof(struct modversion_info);
Removing this return completely means that when versindex == 0, the
function incorrectly looks at the dummy section #0 and eventually
calls pr_warn_once("%s: no symbol version for %s\n", ...).
As I outlined in my prototype patch previously [1], I think this should
be changed to:
/* No versions? Ok, already tainted in check_modstruct_version(). */
if (versindex == 0)
return 1;
> @@ -81,6 +77,11 @@ int check_modstruct_version(const struct load_info *info,
> };
> bool have_symbol;
>
> + /* No versions at all? modprobe --force does this. */
> + if (info->index.vers == 0 &&
> + try_to_force_load(mod, "no versions module"))
> + return 1;
> +
> /*
> * Since this should be found in kernel (which can't be removed), no
> * locking is necessary. Regardless use a RCU read section to keep
I suggest that the reason message should say "no versions for imported
symbols" to indicate which version data is missing and to be consistent
with check_export_symbol_versions(), which uses "no versions for
exported symbols".
[1] https://lore.kernel.org/linux-modules/3992b57d-3d8b-4d60-bc4a-f227f712dcca@suse.com/
--
Thanks,
Petr
^ permalink raw reply
* Re: [PATCH v2 2/4] module: show why force load fails
From: Petr Pavlu @ 2025-08-26 9:33 UTC (permalink / raw)
To: Jinchao Wang
Cc: Luis Chamberlain, Daniel Gomez, Sami Tolvanen, linux-modules,
linux-kernel
In-Reply-To: <20250825091545.18607-3-wangjinchao600@gmail.com>
On 8/25/25 11:15 AM, Jinchao Wang wrote:
> Include reason in error message when force loading is disabled.
>
> Signed-off-by: Jinchao Wang <wangjinchao600@gmail.com>
> ---
> kernel/module/main.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/kernel/module/main.c b/kernel/module/main.c
> index c66b26184936..a426bd8a18b5 100644
> --- a/kernel/module/main.c
> +++ b/kernel/module/main.c
> @@ -1083,6 +1083,7 @@ int try_to_force_load(struct module *mod, const char *reason)
> add_taint_module(mod, TAINT_FORCED_MODULE, LOCKDEP_NOW_UNRELIABLE);
> return 0;
> #else
> + pr_err("%s force load is not supported\n", reason);
> return -ENOEXEC;
> #endif
> }
The module name is already available at all points where
try_to_force_load() is called, so the new error message should include
it.
Additionally, we should be careful about the message. In the case of the
init_module syscall, the missing modversions and vermagic could mean
that the data was deliberately stripped by kmod because the module was
inserted with --force, or it could mean that the module lacks this data
in the first place. In other words, it is not always the case that that
we're reaching this logic because of a force load.
My suggestion would be to use the following:
pr_err("%s: %s, force load is not supported\n", mod->name, reason);
--
Thanks,
Petr
^ 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