* [PATCH 0/2] module: Tweak return and warning
From: Lucas De Marchi @ 2025-10-13 16:26 UTC (permalink / raw)
To: linux-modules, linux-kernel; +Cc: Lucas De Marchi, Petr Pavlu
Do not let userspace tools and end users confused on the return code and
log messages.
Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
---
Lucas De Marchi (2):
module: Override -EEXISTS module return
module: Simplify warning on positive returns from module_init()
kernel/module/main.c | 15 ++++++++-------
1 file changed, 8 insertions(+), 7 deletions(-)
base-commit: e5f0a698b34ed76002dc5cff3804a61c80233a7a
change-id: 20251013-module-warn-ret-59f085298055
Lucas De Marchi
^ permalink raw reply
* [PATCH v2 10/10] module loader: enforce symbol import protection
From: Siddharth Nayyar @ 2025-10-13 15:39 UTC (permalink / raw)
To: petr.pavlu
Cc: arnd, linux-arch, linux-kbuild, linux-kernel, linux-modules,
mcgrof, nathan, nicolas.schier, samitolvanen, sidnayyar, maennich,
gprocida
In-Reply-To: <20251013153918.2206045-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>
Reviewed-by: Petr Pavlu <petr.pavlu@suse.com>
---
kernel/module/internal.h | 1 +
kernel/module/main.c | 10 +++++++++-
2 files changed, 10 insertions(+), 1 deletion(-)
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 f5f9872dc070..c27df62a68f5 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;
}
@@ -1267,6 +1268,13 @@ static const struct kernel_symbol *resolve_symbol(struct module *mod,
goto getname;
}
+ if (fsa.is_protected && !mod->sig_ok) {
+ pr_warn("%s: Cannot use protected symbol %s\n",
+ mod->name, name);
+ fsa.sym = ERR_PTR(-EACCES);
+ goto getname;
+ }
+
err = ref_module(mod, fsa.owner);
if (err) {
fsa.sym = ERR_PTR(err);
@@ -1550,7 +1558,7 @@ 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",
+ pr_warn("%s: Unresolved symbol %s (err %d)\n",
mod->name, name, ret);
break;
--
2.51.0.740.g6adb054d12-goog
^ permalink raw reply related
* [PATCH v2 09/10] modpost: add symbol import protection flag to kflagstab
From: Siddharth Nayyar @ 2025-10-13 15:39 UTC (permalink / raw)
To: petr.pavlu
Cc: arnd, linux-arch, linux-kbuild, linux-kernel, linux-modules,
mcgrof, nathan, nicolas.schier, samitolvanen, sidnayyar, maennich,
gprocida
In-Reply-To: <20251013153918.2206045-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>
Reviewed-by: Petr Pavlu <petr.pavlu@suse.com>
---
include/linux/module_symbol.h | 1 +
scripts/mod/modpost.c | 13 +++++++++++--
2 files changed, 12 insertions(+), 2 deletions(-)
diff --git a/include/linux/module_symbol.h b/include/linux/module_symbol.h
index 574609aced99..1d0414da4c7c 100644
--- a/include/linux/module_symbol.h
+++ b/include/linux/module_symbol.h
@@ -5,6 +5,7 @@
/* Kernel symbol flags bitset. */
enum ksym_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.740.g6adb054d12-goog
^ permalink raw reply related
* [PATCH v2 08/10] remove references to *_gpl sections in documentation
From: Siddharth Nayyar @ 2025-10-13 15:39 UTC (permalink / raw)
To: petr.pavlu
Cc: arnd, linux-arch, linux-kbuild, linux-kernel, linux-modules,
mcgrof, nathan, nicolas.schier, samitolvanen, sidnayyar, maennich,
gprocida
In-Reply-To: <20251013153918.2206045-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.0.740.g6adb054d12-goog
^ permalink raw reply related
* [PATCH v2 07/10] linker: remove *_gpl sections from vmlinux and modules
From: Siddharth Nayyar @ 2025-10-13 15:39 UTC (permalink / raw)
To: petr.pavlu
Cc: arnd, linux-arch, linux-kbuild, linux-kernel, linux-modules,
mcgrof, nathan, nicolas.schier, samitolvanen, sidnayyar, maennich,
gprocida
In-Reply-To: <20251013153918.2206045-1-sidnayyar@google.com>
These sections are not used anymore and can be removed from vmlinux and
modules.
Signed-off-by: Siddharth Nayyar <sidnayyar@google.com>
Reviewed-by: Petr Pavlu <petr.pavlu@suse.com>
---
include/asm-generic/vmlinux.lds.h | 18 ++----------------
scripts/module.lds.S | 2 --
2 files changed, 2 insertions(+), 18 deletions(-)
diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
index 310e2de56211..6490b93d23b1 100644
--- a/include/asm-generic/vmlinux.lds.h
+++ b/include/asm-generic/vmlinux.lds.h
@@ -490,34 +490,20 @@ defined(CONFIG_AUTOFDO_CLANG) || defined(CONFIG_PROPELLER_CLANG)
\
PRINTK_INDEX \
\
- /* Kernel symbol table: Normal symbols */ \
+ /* Kernel symbol table */ \
__ksymtab : AT(ADDR(__ksymtab) - LOAD_OFFSET) { \
__start___ksymtab = .; \
KEEP(*(SORT(___ksymtab+*))) \
__stop___ksymtab = .; \
} \
\
- /* Kernel symbol table: GPL-only symbols */ \
- __ksymtab_gpl : AT(ADDR(__ksymtab_gpl) - LOAD_OFFSET) { \
- __start___ksymtab_gpl = .; \
- KEEP(*(SORT(___ksymtab_gpl+*))) \
- __stop___ksymtab_gpl = .; \
- } \
- \
- /* Kernel symbol table: Normal symbols */ \
+ /* Kernel symbol CRC table */ \
__kcrctab : AT(ADDR(__kcrctab) - LOAD_OFFSET) { \
__start___kcrctab = .; \
KEEP(*(SORT(___kcrctab+*))) \
__stop___kcrctab = .; \
} \
\
- /* Kernel symbol table: GPL-only symbols */ \
- __kcrctab_gpl : AT(ADDR(__kcrctab_gpl) - LOAD_OFFSET) { \
- __start___kcrctab_gpl = .; \
- KEEP(*(SORT(___kcrctab_gpl+*))) \
- __stop___kcrctab_gpl = .; \
- } \
- \
/* Kernel symbol flags table */ \
__kflagstab : AT(ADDR(__kflagstab) - LOAD_OFFSET) { \
__start___kflagstab = .; \
diff --git a/scripts/module.lds.S b/scripts/module.lds.S
index 9a8a3b6d1569..1841a43d8771 100644
--- a/scripts/module.lds.S
+++ b/scripts/module.lds.S
@@ -20,9 +20,7 @@ SECTIONS {
}
__ksymtab 0 : ALIGN(8) { *(SORT(___ksymtab+*)) }
- __ksymtab_gpl 0 : ALIGN(8) { *(SORT(___ksymtab_gpl+*)) }
__kcrctab 0 : ALIGN(4) { *(SORT(___kcrctab+*)) }
- __kcrctab_gpl 0 : ALIGN(4) { *(SORT(___kcrctab_gpl+*)) }
__kflagstab 0 : ALIGN(1) { *(SORT(___kflagstab+*)) }
.ctors 0 : ALIGN(8) { *(SORT(.ctors.*)) *(.ctors) }
--
2.51.0.740.g6adb054d12-goog
^ permalink raw reply related
* [PATCH v2 06/10] module loader: remove references of *_gpl sections
From: Siddharth Nayyar @ 2025-10-13 15:39 UTC (permalink / raw)
To: petr.pavlu
Cc: arnd, linux-arch, linux-kbuild, linux-kernel, linux-modules,
mcgrof, nathan, nicolas.schier, samitolvanen, sidnayyar, maennich,
gprocida
In-Reply-To: <20251013153918.2206045-1-sidnayyar@google.com>
The *_gpl section are not being used populated by modpost anymore. Hence
the module loader doesn't need to find and process these sections in
modules.
Signed-off-by: Siddharth Nayyar <sidnayyar@google.com>
Reviewed-by: Petr Pavlu <petr.pavlu@suse.com>
---
include/linux/module.h | 3 ---
kernel/module/internal.h | 3 ---
kernel/module/main.c | 47 ++++++++++++++++------------------------
3 files changed, 19 insertions(+), 34 deletions(-)
diff --git a/include/linux/module.h b/include/linux/module.h
index 9ba6ce433ac3..1a9c41318e22 100644
--- a/include/linux/module.h
+++ b/include/linux/module.h
@@ -431,9 +431,6 @@ struct module {
unsigned int num_kp;
/* GPL-only exported symbols. */
- unsigned int num_gpl_syms;
- const struct kernel_symbol *gpl_syms;
- const u32 *gpl_crcs;
bool using_gplonly_symbols;
#ifdef CONFIG_MODULE_SIG
diff --git a/kernel/module/internal.h b/kernel/module/internal.h
index 69b84510e097..061161cc79d9 100644
--- a/kernel/module/internal.h
+++ b/kernel/module/internal.h
@@ -53,10 +53,7 @@ extern const size_t modinfo_attrs_count;
/* Provided by the linker */
extern const struct kernel_symbol __start___ksymtab[];
extern const struct kernel_symbol __stop___ksymtab[];
-extern const struct kernel_symbol __start___ksymtab_gpl[];
-extern const struct kernel_symbol __stop___ksymtab_gpl[];
extern const u32 __start___kcrctab[];
-extern const u32 __start___kcrctab_gpl[];
extern const u8 __start___kflagstab[];
#define KMOD_PATH_LEN 256
diff --git a/kernel/module/main.c b/kernel/module/main.c
index 4197af526087..f5f9872dc070 100644
--- a/kernel/module/main.c
+++ b/kernel/module/main.c
@@ -1464,29 +1464,18 @@ EXPORT_SYMBOL_GPL(__symbol_get);
*/
static int verify_exported_symbols(struct module *mod)
{
- unsigned int i;
const struct kernel_symbol *s;
- struct {
- const struct kernel_symbol *sym;
- unsigned int num;
- } arr[] = {
- { mod->syms, mod->num_syms },
- { mod->gpl_syms, mod->num_gpl_syms },
- };
-
- for (i = 0; i < ARRAY_SIZE(arr); i++) {
- for (s = arr[i].sym; s < arr[i].sym + arr[i].num; s++) {
- struct find_symbol_arg fsa = {
- .name = kernel_symbol_name(s),
- .gplok = true,
- };
- if (find_symbol(&fsa)) {
- pr_err("%s: exports duplicate symbol %s"
- " (owned by %s)\n",
- mod->name, kernel_symbol_name(s),
- module_name(fsa.owner));
- return -ENOEXEC;
- }
+ for (s = mod->syms; s < mod->syms + mod->num_syms; s++) {
+ struct find_symbol_arg fsa = {
+ .name = kernel_symbol_name(s),
+ .gplok = true,
+ };
+ if (find_symbol(&fsa)) {
+ pr_err("%s: exports duplicate symbol %s"
+ " (owned by %s)\n",
+ mod->name, kernel_symbol_name(s),
+ module_name(fsa.owner));
+ return -ENOEXEC;
}
}
return 0;
@@ -2601,12 +2590,15 @@ static int find_module_sections(struct module *mod, struct load_info *info)
mod->syms = section_objs(info, "__ksymtab",
sizeof(*mod->syms), &mod->num_syms);
mod->crcs = section_addr(info, "__kcrctab");
- mod->gpl_syms = section_objs(info, "__ksymtab_gpl",
- sizeof(*mod->gpl_syms),
- &mod->num_gpl_syms);
- mod->gpl_crcs = section_addr(info, "__kcrctab_gpl");
mod->flagstab = section_addr(info, "__kflagstab");
+ if (section_addr(info, "__ksymtab_gpl"))
+ pr_warn("%s: ignoring obsolete section __ksymtab_gpl\n",
+ mod->name);
+ if (section_addr(info, "__kcrctab_gpl"))
+ pr_warn("%s: ignoring obsolete section __kcrctab_gpl\n",
+ mod->name);
+
#ifdef CONFIG_CONSTRUCTORS
mod->ctors = section_objs(info, ".ctors",
sizeof(*mod->ctors), &mod->num_ctors);
@@ -2816,8 +2808,7 @@ static int check_export_symbol_sections(struct module *mod)
return -ENOEXEC;
}
#ifdef CONFIG_MODVERSIONS
- if ((mod->num_syms && !mod->crcs) ||
- (mod->num_gpl_syms && !mod->gpl_crcs)) {
+ if (mod->num_syms && !mod->crcs) {
return try_to_force_load(mod,
"no versions for exported symbols");
}
--
2.51.0.740.g6adb054d12-goog
^ permalink raw reply related
* [PATCH v2 05/10] modpost: put all exported symbols in ksymtab section
From: Siddharth Nayyar @ 2025-10-13 15:39 UTC (permalink / raw)
To: petr.pavlu
Cc: arnd, linux-arch, linux-kbuild, linux-kernel, linux-modules,
mcgrof, nathan, nicolas.schier, samitolvanen, sidnayyar, maennich,
gprocida
In-Reply-To: <20251013153918.2206045-1-sidnayyar@google.com>
Since the modules loader determines whether an exported symbol is GPL
only from the kflagstab section data, modpost can put all symbols in the
regular ksymtab and stop using the *_gpl versions of the ksymtab and
kcrctab.
Signed-off-by: Siddharth Nayyar <sidnayyar@google.com>
Reviewed-by: Petr Pavlu <petr.pavlu@suse.com>
---
include/linux/export-internal.h | 21 +++++++++++----------
scripts/mod/modpost.c | 8 ++++----
2 files changed, 15 insertions(+), 14 deletions(-)
diff --git a/include/linux/export-internal.h b/include/linux/export-internal.h
index 4123c7592404..726054614752 100644
--- a/include/linux/export-internal.h
+++ b/include/linux/export-internal.h
@@ -37,14 +37,14 @@
* section flag requires it. Use '%progbits' instead of '@progbits' since the
* former apparently works on all arches according to the binutils source.
*/
-#define __KSYMTAB(name, sym, sec, ns) \
+#define __KSYMTAB(name, sym, ns) \
asm(" .section \"__ksymtab_strings\",\"aMS\",%progbits,1" "\n" \
"__kstrtab_" #name ":" "\n" \
" .asciz \"" #name "\"" "\n" \
"__kstrtabns_" #name ":" "\n" \
" .asciz \"" ns "\"" "\n" \
" .previous" "\n" \
- " .section \"___ksymtab" sec "+" #name "\", \"a\"" "\n" \
+ " .section \"___ksymtab+" #name "\", \"a\"" "\n" \
__KSYM_ALIGN "\n" \
"__ksymtab_" #name ":" "\n" \
__KSYM_REF(sym) "\n" \
@@ -59,15 +59,16 @@
#define KSYM_FUNC(name) name
#endif
-#define KSYMTAB_FUNC(name, sec, ns) __KSYMTAB(name, KSYM_FUNC(name), sec, ns)
-#define KSYMTAB_DATA(name, sec, ns) __KSYMTAB(name, name, sec, ns)
+#define KSYMTAB_FUNC(name, ns) __KSYMTAB(name, KSYM_FUNC(name), ns)
+#define KSYMTAB_DATA(name, ns) __KSYMTAB(name, name, ns)
-#define SYMBOL_CRC(sym, crc, sec) \
- asm(".section \"___kcrctab" sec "+" #sym "\",\"a\"" "\n" \
- ".balign 4" "\n" \
- "__crc_" #sym ":" "\n" \
- ".long " #crc "\n" \
- ".previous" "\n")
+#define SYMBOL_CRC(sym, crc) \
+ asm(" .section \"___kcrctab+" #sym "\",\"a\"" "\n" \
+ " .balign 4" "\n" \
+ "__crc_" #sym ":" "\n" \
+ " .long " #crc "\n" \
+ " .previous" "\n" \
+ )
#define SYMBOL_FLAGS(sym, flags) \
asm(" .section \"___kflagstab+" #sym "\",\"a\"" "\n" \
diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
index f5ce7aeed52d..8936db84779b 100644
--- a/scripts/mod/modpost.c
+++ b/scripts/mod/modpost.c
@@ -1867,9 +1867,9 @@ static void add_exported_symbols(struct buffer *buf, struct module *mod)
if (trim_unused_exports && !sym->used)
continue;
- buf_printf(buf, "KSYMTAB_%s(%s, \"%s\", \"%s\");\n",
+ buf_printf(buf, "KSYMTAB_%s(%s, \"%s\");\n",
sym->is_func ? "FUNC" : "DATA", sym->name,
- sym->is_gpl_only ? "_gpl" : "", sym->namespace);
+ sym->namespace);
buf_printf(buf, "SYMBOL_FLAGS(%s, 0x%02x);\n",
sym->name, get_symbol_flags(sym));
@@ -1890,8 +1890,8 @@ static void add_exported_symbols(struct buffer *buf, struct module *mod)
sym->name, mod->name, mod->is_vmlinux ? "" : ".ko",
sym->name);
- buf_printf(buf, "SYMBOL_CRC(%s, 0x%08x, \"%s\");\n",
- sym->name, sym->crc, sym->is_gpl_only ? "_gpl" : "");
+ buf_printf(buf, "SYMBOL_CRC(%s, 0x%08x);\n",
+ sym->name, sym->crc);
}
}
--
2.51.0.740.g6adb054d12-goog
^ permalink raw reply related
* [PATCH v2 04/10] module loader: use kflagstab instead of *_gpl sections
From: Siddharth Nayyar @ 2025-10-13 15:39 UTC (permalink / raw)
To: petr.pavlu
Cc: arnd, linux-arch, linux-kbuild, linux-kernel, linux-modules,
mcgrof, nathan, nicolas.schier, samitolvanen, sidnayyar, maennich,
gprocida
In-Reply-To: <20251013153918.2206045-1-sidnayyar@google.com>
Read __kflagstab section for vmlinux and modules to determine whether
kernel symbols are GPL only.
Signed-off-by: Siddharth Nayyar <sidnayyar@google.com>
Reviewed-by: Petr Pavlu <petr.pavlu@suse.com>
---
include/linux/module.h | 1 +
kernel/module/internal.h | 1 +
kernel/module/main.c | 55 +++++++++++++++++++++-------------------
3 files changed, 31 insertions(+), 26 deletions(-)
diff --git a/include/linux/module.h b/include/linux/module.h
index 3319a5269d28..9ba6ce433ac3 100644
--- a/include/linux/module.h
+++ b/include/linux/module.h
@@ -415,6 +415,7 @@ struct module {
/* Exported symbols */
const struct kernel_symbol *syms;
const u32 *crcs;
+ const u8 *flagstab;
unsigned int num_syms;
#ifdef CONFIG_ARCH_USES_CFI_TRAPS
diff --git a/kernel/module/internal.h b/kernel/module/internal.h
index 618202578b42..69b84510e097 100644
--- a/kernel/module/internal.h
+++ b/kernel/module/internal.h
@@ -57,6 +57,7 @@ extern const struct kernel_symbol __start___ksymtab_gpl[];
extern const struct kernel_symbol __stop___ksymtab_gpl[];
extern const u32 __start___kcrctab[];
extern const u32 __start___kcrctab_gpl[];
+extern const u8 __start___kflagstab[];
#define KMOD_PATH_LEN 256
extern char modprobe_path[];
diff --git a/kernel/module/main.c b/kernel/module/main.c
index c66b26184936..4197af526087 100644
--- a/kernel/module/main.c
+++ b/kernel/module/main.c
@@ -11,6 +11,7 @@
#include <linux/extable.h>
#include <linux/moduleloader.h>
#include <linux/module_signature.h>
+#include <linux/module_symbol.h>
#include <linux/trace_events.h>
#include <linux/init.h>
#include <linux/kallsyms.h>
@@ -87,7 +88,7 @@ struct mod_tree_root mod_tree __cacheline_aligned = {
struct symsearch {
const struct kernel_symbol *start, *stop;
const u32 *crcs;
- enum mod_license license;
+ const u8 *flagstab;
};
/*
@@ -364,19 +365,21 @@ static bool find_exported_symbol_in_section(const struct symsearch *syms,
struct find_symbol_arg *fsa)
{
struct kernel_symbol *sym;
-
- if (!fsa->gplok && syms->license == GPL_ONLY)
- return false;
+ u8 sym_flags;
sym = bsearch(fsa->name, syms->start, syms->stop - syms->start,
sizeof(struct kernel_symbol), cmp_name);
if (!sym)
return false;
+ sym_flags = *(syms->flagstab + (sym - syms->start));
+ if (!fsa->gplok && (sym_flags & KSYM_FLAG_GPL_ONLY))
+ return false;
+
fsa->owner = owner;
fsa->crc = symversion(syms->crcs, sym - syms->start);
fsa->sym = sym;
- fsa->license = syms->license;
+ fsa->license = (sym_flags & KSYM_FLAG_GPL_ONLY) ? GPL_ONLY : NOT_GPL_ONLY;
return true;
}
@@ -387,36 +390,31 @@ static bool find_exported_symbol_in_section(const struct symsearch *syms,
*/
bool find_symbol(struct find_symbol_arg *fsa)
{
- static const struct symsearch arr[] = {
- { __start___ksymtab, __stop___ksymtab, __start___kcrctab,
- NOT_GPL_ONLY },
- { __start___ksymtab_gpl, __stop___ksymtab_gpl,
- __start___kcrctab_gpl,
- GPL_ONLY },
+ const struct symsearch syms = {
+ .start = __start___ksymtab,
+ .stop = __stop___ksymtab,
+ .crcs = __start___kcrctab,
+ .flagstab = __start___kflagstab,
};
struct module *mod;
- unsigned int i;
- for (i = 0; i < ARRAY_SIZE(arr); i++)
- if (find_exported_symbol_in_section(&arr[i], NULL, fsa))
- return true;
+ if (find_exported_symbol_in_section(&syms, NULL, fsa))
+ return true;
list_for_each_entry_rcu(mod, &modules, list,
lockdep_is_held(&module_mutex)) {
- struct symsearch arr[] = {
- { mod->syms, mod->syms + mod->num_syms, mod->crcs,
- NOT_GPL_ONLY },
- { mod->gpl_syms, mod->gpl_syms + mod->num_gpl_syms,
- mod->gpl_crcs,
- GPL_ONLY },
+ const struct symsearch syms = {
+ .start = mod->syms,
+ .stop = mod->syms + mod->num_syms,
+ .crcs = mod->crcs,
+ .flagstab = mod->flagstab,
};
if (mod->state == MODULE_STATE_UNFORMED)
continue;
- for (i = 0; i < ARRAY_SIZE(arr); i++)
- if (find_exported_symbol_in_section(&arr[i], mod, fsa))
- return true;
+ if (find_exported_symbol_in_section(&syms, mod, fsa))
+ return true;
}
pr_debug("Failed to find symbol %s\n", fsa->name);
@@ -2607,6 +2605,7 @@ static int find_module_sections(struct module *mod, struct load_info *info)
sizeof(*mod->gpl_syms),
&mod->num_gpl_syms);
mod->gpl_crcs = section_addr(info, "__kcrctab_gpl");
+ mod->flagstab = section_addr(info, "__kflagstab");
#ifdef CONFIG_CONSTRUCTORS
mod->ctors = section_objs(info, ".ctors",
@@ -2810,8 +2809,12 @@ static int move_module(struct module *mod, struct load_info *info)
return ret;
}
-static int check_export_symbol_versions(struct module *mod)
+static int check_export_symbol_sections(struct module *mod)
{
+ if (mod->num_syms && !mod->flagstab) {
+ pr_err("%s: no flags for exported symbols\n", mod->name);
+ return -ENOEXEC;
+ }
#ifdef CONFIG_MODVERSIONS
if ((mod->num_syms && !mod->crcs) ||
(mod->num_gpl_syms && !mod->gpl_crcs)) {
@@ -3427,7 +3430,7 @@ static int load_module(struct load_info *info, const char __user *uargs,
if (err)
goto free_unload;
- err = check_export_symbol_versions(mod);
+ err = check_export_symbol_sections(mod);
if (err)
goto free_unload;
--
2.51.0.740.g6adb054d12-goog
^ permalink raw reply related
* [PATCH v2 03/10] modpost: create entries for kflagstab
From: Siddharth Nayyar @ 2025-10-13 15:39 UTC (permalink / raw)
To: petr.pavlu
Cc: arnd, linux-arch, linux-kbuild, linux-kernel, linux-modules,
mcgrof, nathan, nicolas.schier, samitolvanen, sidnayyar, maennich,
gprocida
In-Reply-To: <20251013153918.2206045-1-sidnayyar@google.com>
Signed-off-by: Siddharth Nayyar <sidnayyar@google.com>
Reviewed-by: Petr Pavlu <petr.pavlu@suse.com>
---
include/linux/export-internal.h | 7 +++++++
scripts/mod/modpost.c | 8 ++++++++
2 files changed, 15 insertions(+)
diff --git a/include/linux/export-internal.h b/include/linux/export-internal.h
index d445705ac13c..4123c7592404 100644
--- a/include/linux/export-internal.h
+++ b/include/linux/export-internal.h
@@ -69,4 +69,11 @@
".long " #crc "\n" \
".previous" "\n")
+#define SYMBOL_FLAGS(sym, flags) \
+ asm(" .section \"___kflagstab+" #sym "\",\"a\"" "\n" \
+ "__flags_" #sym ":" "\n" \
+ " .byte " #flags "\n" \
+ " .previous" "\n" \
+ )
+
#endif /* __LINUX_EXPORT_INTERNAL_H__ */
diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
index 5ca7c268294e..f5ce7aeed52d 100644
--- a/scripts/mod/modpost.c
+++ b/scripts/mod/modpost.c
@@ -244,6 +244,11 @@ static struct symbol *alloc_symbol(const char *name)
return s;
}
+static uint8_t get_symbol_flags(const struct symbol *sym)
+{
+ return sym->is_gpl_only ? KSYM_FLAG_GPL_ONLY : 0;
+}
+
/* For the hash of exported symbols */
static void hash_add_symbol(struct symbol *sym)
{
@@ -1865,6 +1870,9 @@ static void add_exported_symbols(struct buffer *buf, struct module *mod)
buf_printf(buf, "KSYMTAB_%s(%s, \"%s\", \"%s\");\n",
sym->is_func ? "FUNC" : "DATA", sym->name,
sym->is_gpl_only ? "_gpl" : "", sym->namespace);
+
+ buf_printf(buf, "SYMBOL_FLAGS(%s, 0x%02x);\n",
+ sym->name, get_symbol_flags(sym));
}
if (!modversions)
--
2.51.0.740.g6adb054d12-goog
^ permalink raw reply related
* [PATCH v2 02/10] linker: add kflagstab section to vmlinux and modules
From: Siddharth Nayyar @ 2025-10-13 15:39 UTC (permalink / raw)
To: petr.pavlu
Cc: arnd, linux-arch, linux-kbuild, linux-kernel, linux-modules,
mcgrof, nathan, nicolas.schier, samitolvanen, sidnayyar, maennich,
gprocida
In-Reply-To: <20251013153918.2206045-1-sidnayyar@google.com>
This section will contain read-only kernel symbol flag values in the
form of a 8-bit bitset.
Signed-off-by: Siddharth Nayyar <sidnayyar@google.com>
Reviewed-by: Petr Pavlu <petr.pavlu@suse.com>
---
include/asm-generic/vmlinux.lds.h | 7 +++++++
scripts/module.lds.S | 1 +
2 files changed, 8 insertions(+)
diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
index ae2d2359b79e..310e2de56211 100644
--- a/include/asm-generic/vmlinux.lds.h
+++ b/include/asm-generic/vmlinux.lds.h
@@ -518,6 +518,13 @@ defined(CONFIG_AUTOFDO_CLANG) || defined(CONFIG_PROPELLER_CLANG)
__stop___kcrctab_gpl = .; \
} \
\
+ /* Kernel symbol flags table */ \
+ __kflagstab : AT(ADDR(__kflagstab) - LOAD_OFFSET) { \
+ __start___kflagstab = .; \
+ KEEP(*(SORT(___kflagstab+*))) \
+ __stop___kflagstab = .; \
+ } \
+ \
/* Kernel symbol table: strings */ \
__ksymtab_strings : AT(ADDR(__ksymtab_strings) - LOAD_OFFSET) { \
*(__ksymtab_strings) \
diff --git a/scripts/module.lds.S b/scripts/module.lds.S
index ee79c41059f3..9a8a3b6d1569 100644
--- a/scripts/module.lds.S
+++ b/scripts/module.lds.S
@@ -23,6 +23,7 @@ SECTIONS {
__ksymtab_gpl 0 : ALIGN(8) { *(SORT(___ksymtab_gpl+*)) }
__kcrctab 0 : ALIGN(4) { *(SORT(___kcrctab+*)) }
__kcrctab_gpl 0 : ALIGN(4) { *(SORT(___kcrctab_gpl+*)) }
+ __kflagstab 0 : ALIGN(1) { *(SORT(___kflagstab+*)) }
.ctors 0 : ALIGN(8) { *(SORT(.ctors.*)) *(.ctors) }
.init_array 0 : ALIGN(8) { *(SORT(.init_array.*)) *(.init_array) }
--
2.51.0.740.g6adb054d12-goog
^ permalink raw reply related
* [PATCH v2 01/10] define kernel symbol flags
From: Siddharth Nayyar @ 2025-10-13 15:39 UTC (permalink / raw)
To: petr.pavlu
Cc: arnd, linux-arch, linux-kbuild, linux-kernel, linux-modules,
mcgrof, nathan, nicolas.schier, samitolvanen, sidnayyar, maennich,
gprocida
In-Reply-To: <20251013153918.2206045-1-sidnayyar@google.com>
Symbol flags is an enumeration used to represent flags as a bitset, for
example a flag to tell if a symbols GPL only.
Signed-off-by: Siddharth Nayyar <sidnayyar@google.com>
Reviewed-by: Petr Pavlu <petr.pavlu@suse.com>
---
include/linux/module_symbol.h | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/include/linux/module_symbol.h b/include/linux/module_symbol.h
index 77c9895b9ddb..574609aced99 100644
--- a/include/linux/module_symbol.h
+++ b/include/linux/module_symbol.h
@@ -2,6 +2,11 @@
#ifndef _LINUX_MODULE_SYMBOL_H
#define _LINUX_MODULE_SYMBOL_H
+/* Kernel symbol flags bitset. */
+enum ksym_flags {
+ KSYM_FLAG_GPL_ONLY = 1 << 0,
+};
+
/* This ignores the intensely annoying "mapping symbols" found in ELF files. */
static inline bool is_mapping_symbol(const char *str)
{
--
2.51.0.740.g6adb054d12-goog
^ permalink raw reply related
* [PATCH v2 00/10] scalable symbol flags with __kflagstab
From: Siddharth Nayyar @ 2025-10-13 15:39 UTC (permalink / raw)
To: petr.pavlu
Cc: arnd, linux-arch, linux-kbuild, linux-kernel, linux-modules,
mcgrof, nathan, nicolas.schier, samitolvanen, sidnayyar, maennich,
gprocida
This patch series implements a mechanism for scalable exported symbol
flags using a separate section called __kflagstab. The series introduces
__kflagstab support, removes *_gpl sections in favor of a GPL flag,
simplifies symbol resolution during module loading, and adds symbol
import protection.
Thank you Petr Pavlu for their valuable feedback.
---
Changes from v1:
- added a check to ensure __kflagstab is present
- added warnings for the obsolete *_gpl sections
- moved protected symbol check before ref_module() call
- moved protected symbol check failure warning to issue detection point
v1:
https://lore.kernel.org/all/20250829105418.3053274-1-sidnayyar@google.com/
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 | 11 +--
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 | 112 +++++++++++++++---------------
scripts/mod/modpost.c | 27 +++++--
scripts/module.lds.S | 3 +-
9 files changed, 120 insertions(+), 97 deletions(-)
--
2.51.0.740.g6adb054d12-goog
^ permalink raw reply
* Re: [PATCH v17 35/47] i2c: rename wait_for_completion callback to wait_for_completion_cb
From: Byungchul Park @ 2025-10-13 5:27 UTC (permalink / raw)
To: Wolfram Sang
Cc: linux-kernel, kernel_team, torvalds, damien.lemoal, linux-ide,
adilger.kernel, linux-ext4, mingo, peterz, will, tglx, rostedt,
joel, sashal, daniel.vetter, duyuyang, johannes.berg, tj, tytso,
willy, david, amir73il, gregkh, kernel-team, linux-mm, akpm,
mhocko, minchan, hannes, vdavydov.dev, sj, jglisse, dennis, cl,
penberg, rientjes, vbabka, ngupta, linux-block, josef,
linux-fsdevel, jack, jlayton, dan.j.williams, hch, djwong,
dri-devel, rodrigosiqueiramelo, melissa.srw, hamohammed.sa,
harry.yoo, chris.p.wilson, gwan-gyeong.mun, max.byungchul.park,
boqun.feng, longman, yunseong.kim, ysk, yeoreum.yun, netdev,
matthew.brost, her0gyugyu, corbet, catalin.marinas, bp,
dave.hansen, x86, hpa, luto, sumit.semwal, gustavo,
christian.koenig, andi.shyti, arnd, lorenzo.stoakes, Liam.Howlett,
rppt, surenb, mcgrof, petr.pavlu, da.gomez, samitolvanen, paulmck,
frederic, neeraj.upadhyay, joelagnelf, josh, urezki,
mathieu.desnoyers, jiangshanlai, qiang.zhang, juri.lelli,
vincent.guittot, dietmar.eggemann, bsegall, mgorman, vschneid,
chuck.lever, neil, okorniev, Dai.Ngo, tom, trondmy, anna, kees,
bigeasy, clrkwllms, mark.rutland, ada.coupriediaz,
kristina.martsenko, wangkefeng.wang, broonie, kevin.brodsky, dwmw,
shakeel.butt, ast, ziy, yuzhao, baolin.wang, usamaarif642,
joel.granados, richard.weiyang, geert+renesas, tim.c.chen, linux,
alexander.shishkin, lillian, chenhuacai, francesco,
guoweikang.kernel, link, jpoimboe, masahiroy, brauner,
thomas.weissschuh, oleg, mjguzik, andrii, wangfushuai, linux-doc,
linux-arm-kernel, linux-media, linaro-mm-sig, linux-i2c,
linux-arch, linux-modules, rcu, linux-nfs, linux-rt-devel
In-Reply-To: <aOFNz2mKXCXUImwO@shikoro>
On Sat, Oct 04, 2025 at 06:39:43PM +0200, Wolfram Sang wrote:
> On Thu, Oct 02, 2025 at 05:12:35PM +0900, Byungchul Park wrote:
> > Functionally no change. This patch is a preparation for DEPT(DEPendency
> > Tracker) to track dependencies related to a scheduler API,
> > wait_for_completion().
> >
> > Unfortunately, struct i2c_algo_pca_data has a callback member named
> > wait_for_completion, that is the same as the scheduler API, which makes
> > it hard to change the scheduler API to a macro form because of the
> > ambiguity.
> >
> > Add a postfix _cb to the callback member to remove the ambiguity.
> >
> > Signed-off-by: Byungchul Park <byungchul@sk.com>
>
> This patch seems reasonable in any case. I'll pick it, so you have one
> dependency less. Good luck with the series!
Thanks, Wolfram Sang!
Byungchul
> Applied to for-next, thanks!
^ permalink raw reply
* Re: [PATCH v17 28/47] dept: add documentation for dept
From: Byungchul Park @ 2025-10-13 5:23 UTC (permalink / raw)
To: NeilBrown
Cc: linux-kernel, kernel_team, torvalds, damien.lemoal, linux-ide,
adilger.kernel, linux-ext4, mingo, peterz, will, tglx, rostedt,
joel, sashal, daniel.vetter, duyuyang, johannes.berg, tj, tytso,
willy, david, amir73il, gregkh, kernel-team, linux-mm, akpm,
mhocko, minchan, hannes, vdavydov.dev, sj, jglisse, dennis, cl,
penberg, rientjes, vbabka, ngupta, linux-block, josef,
linux-fsdevel, jack, jlayton, dan.j.williams, hch, djwong,
dri-devel, rodrigosiqueiramelo, melissa.srw, hamohammed.sa,
harry.yoo, chris.p.wilson, gwan-gyeong.mun, max.byungchul.park,
boqun.feng, longman, yunseong.kim, ysk, yeoreum.yun, netdev,
matthew.brost, her0gyugyu, corbet, catalin.marinas, bp,
dave.hansen, x86, hpa, luto, sumit.semwal, gustavo,
christian.koenig, andi.shyti, arnd, lorenzo.stoakes, Liam.Howlett,
rppt, surenb, mcgrof, petr.pavlu, da.gomez, samitolvanen, paulmck,
frederic, neeraj.upadhyay, joelagnelf, josh, urezki,
mathieu.desnoyers, jiangshanlai, qiang.zhang, juri.lelli,
vincent.guittot, dietmar.eggemann, bsegall, mgorman, vschneid,
chuck.lever, okorniev, Dai.Ngo, tom, trondmy, anna, kees, bigeasy,
clrkwllms, mark.rutland, ada.coupriediaz, kristina.martsenko,
wangkefeng.wang, broonie, kevin.brodsky, dwmw, shakeel.butt, ast,
ziy, yuzhao, baolin.wang, usamaarif642, joel.granados,
richard.weiyang, geert+renesas, tim.c.chen, linux,
alexander.shishkin, lillian, chenhuacai, francesco,
guoweikang.kernel, link, jpoimboe, masahiroy, brauner,
thomas.weissschuh, oleg, mjguzik, andrii, wangfushuai, linux-doc,
linux-arm-kernel, linux-media, linaro-mm-sig, linux-i2c,
linux-arch, linux-modules, rcu, linux-nfs, linux-rt-devel
In-Reply-To: <175947451487.247319.6809470356431942803@noble.neil.brown.name>
On Fri, Oct 03, 2025 at 04:55:14PM +1000, NeilBrown wrote:
> On Thu, 02 Oct 2025, Byungchul Park wrote:
> > This document describes the concept and APIs of dept.
> >
>
> Thanks for the documentation. I've been trying to understand it.
You're welcome. Feel free to ask me if you have any questions.
> > +How DEPT works
> > +--------------
> > +
> > +Let's take a look how DEPT works with the 1st example in the section
> > +'Limitation of lockdep'.
> > +
> > + context X context Y context Z
> > +
> > + mutex_lock A
> > + folio_lock B
> > + folio_lock B <- DEADLOCK
> > + mutex_lock A <- DEADLOCK
> > + folio_unlock B
> > + folio_unlock B
> > + mutex_unlock A
> > + mutex_unlock A
> > +
> > +Adding comments to describe DEPT's view in terms of wait and event:
> > +
> > + context X context Y context Z
> > +
> > + mutex_lock A
> > + /* wait for A */
> > + folio_lock B
> > + /* wait for A */
> > + /* start event A context */
> > +
> > + folio_lock B
> > + /* wait for B */ <- DEADLOCK
> > + /* start event B context */
> > +
> > + mutex_lock A
> > + /* wait for A */ <- DEADLOCK
> > + /* start event A context */
> > +
> > + folio_unlock B
> > + /* event B */
> > + folio_unlock B
> > + /* event B */
> > +
> > + mutex_unlock A
> > + /* event A */
> > + mutex_unlock A
> > + /* event A */
> > +
>
> I can't see the value of the above section.
> The first section with no comments is useful as it is easy to see the
> deadlock being investigate. The section below is useful as it add
> comments to explain how DEPT sees the situation. But the above section,
> with some but not all of the comments, does seem (to me) to add anything
> useful.
I just wanted to convert 'locking terms' to 'wait and event terms' by
one step. However, I can remove the section you pointed out that you
thought was useless.
> > +Adding more supplementary comments to describe DEPT's view in detail:
> > +
> > + context X context Y context Z
> > +
> > + mutex_lock A
> > + /* might wait for A */
> > + /* start to take into account event A's context */
>
> What do you mean precisely by "context".
That means one of task context, irq context, wq worker context (even
though it can also be considered as task context), or something.
Of course, in the example above, it must be task context since it showed
a case involving only sleepible ones. However, I wanted to use general
terms in the document to cover all the types of context e.g. irq, task,
and so on.
> You use the word in the heading "context X context Y context Z"
> so it seems like "context" means "task" or "process". But then as I
> read on, I think - maybe it means something else. If it does, then you
> should use different words. Maybe "task X ..." in the heading.
It should cover all the types of context. What word would you recommend
for that purpose?
> If the examples that follow It seems that the "context" for event A
> starts at "mutex lock A" when it (possibly) waits for a mutex and ends
> at "mutex unlock A" - which are both in the same process. Clearly
> various other events that happen between these two points in the same
> process could be seen as the "context" for event A.
>
> However event B starts in "context X" with "folio_lock B" and ends in
> "context Z" or "context Y" with "folio_unlock B". Is that right?
Right.
> My question then is: how do you decide which, of all the event in all
> the processes in all the system, between the start[S] and the end[E] are
> considered to be part of the "context" of event A.
DEPT can identify the "context" of event A only *once* the event A is
actually executed, and builds dependencies between the event and the
recorded waits in the "context" of event A since [S].
> I think it would help me if you defined what a "context" is earlier.
Sorry if my description was not clear enough.
Byungchul
> What sorts of things appear in a context?
>
> Thanks,
> NeilBrown
>
>
> > + /* 1 */
> > + folio_lock B
> > + /* might wait for B */
> > + /* start to take into account event B's context */
> > + /* 2 */
> > +
> > + folio_lock B
> > + /* might wait for B */ <- DEADLOCK
> > + /* start to take into account event B's context */
> > + /* 3 */
> > +
> > + mutex_lock A
> > + /* might wait for A */ <- DEADLOCK
> > + /* start to take into account
> > + event A's context */
> > + /* 4 */
> > +
> > + folio_unlock B
> > + /* event B that's been valid since 2 */
> > + folio_unlock B
> > + /* event B that's been valid since 3 */
> > +
> > + mutex_unlock A
> > + /* event A that's been valid since 1 */
> > +
> > + mutex_unlock A
> > + /* event A that's been valid since 4 */
> > +
> > +Let's build up dependency graph with this example. Firstly, context X:
> > +
> > + context X
> > +
> > + folio_lock B
> > + /* might wait for B */
> > + /* start to take into account event B's context */
> > + /* 2 */
> > +
> > +There are no events to create dependency. Next, context Y:
> > +
> > + context Y
> > +
> > + mutex_lock A
> > + /* might wait for A */
> > + /* start to take into account event A's context */
> > + /* 1 */
> > +
> > + folio_lock B
> > + /* might wait for B */
> > + /* start to take into account event B's context */
> > + /* 3 */
> > +
> > + folio_unlock B
> > + /* event B that's been valid since 3 */
> > +
> > + mutex_unlock A
> > + /* event A that's been valid since 1 */
> > +
> > +There are two events. For event B, folio_unlock B, since there are no
> > +waits between 3 and the event, event B does not create dependency. For
> > +event A, there is a wait, folio_lock B, between 1 and the event. Which
> > +means event A cannot be triggered if event B does not wake up the wait.
> > +Therefore, we can say event A depends on event B, say, 'A -> B'. The
> > +graph will look like after adding the dependency:
> > +
> > + A -> B
> > +
> > + where 'A -> B' means that event A depends on event B.
> > +
> > +Lastly, context Z:
> > +
> > + context Z
> > +
> > + mutex_lock A
> > + /* might wait for A */
> > + /* start to take into account event A's context */
> > + /* 4 */
> > +
> > + folio_unlock B
> > + /* event B that's been valid since 2 */
> > +
> > + mutex_unlock A
> > + /* event A that's been valid since 4 */
> > +
> > +There are also two events. For event B, folio_unlock B, there is a
> > +wait, mutex_lock A, between 2 and the event - remind 2 is at a very
> > +start and before the wait in timeline. Which means event B cannot be
> > +triggered if event A does not wake up the wait. Therefore, we can say
> > +event B depends on event A, say, 'B -> A'. The graph will look like
> > +after adding the dependency:
> > +
> > + -> A -> B -
> > + / \
> > + \ /
> > + -----------
> > +
> > + where 'A -> B' means that event A depends on event B.
> > +
> > +A new loop has been created. So DEPT can report it as a deadlock. For
> > +event A, mutex_unlock A, since there are no waits between 4 and the
> > +event, event A does not create dependency. That's it.
> > +
> > +CONCLUSION
> > +
> > +DEPT works well with any general synchronization mechanisms by focusing
> > +on wait, event and its context.
> > +
^ permalink raw reply
* Re: [PATCH v17 09/47] arm64, dept: add support CONFIG_ARCH_HAS_DEPT_SUPPORT to arm64
From: Byungchul Park @ 2025-10-13 4:28 UTC (permalink / raw)
To: Mark Rutland
Cc: linux-kernel, kernel_team, torvalds, damien.lemoal, linux-ide,
adilger.kernel, linux-ext4, mingo, peterz, will, tglx, rostedt,
joel, sashal, daniel.vetter, duyuyang, johannes.berg, tj, tytso,
willy, david, amir73il, gregkh, kernel-team, linux-mm, akpm,
mhocko, minchan, hannes, vdavydov.dev, sj, jglisse, dennis, cl,
penberg, rientjes, vbabka, ngupta, linux-block, josef,
linux-fsdevel, jack, jlayton, dan.j.williams, hch, djwong,
dri-devel, rodrigosiqueiramelo, melissa.srw, hamohammed.sa,
harry.yoo, chris.p.wilson, gwan-gyeong.mun, max.byungchul.park,
boqun.feng, longman, yunseong.kim, ysk, yeoreum.yun, netdev,
matthew.brost, her0gyugyu, corbet, catalin.marinas, bp,
dave.hansen, x86, hpa, luto, sumit.semwal, gustavo,
christian.koenig, andi.shyti, arnd, lorenzo.stoakes, Liam.Howlett,
rppt, surenb, mcgrof, petr.pavlu, da.gomez, samitolvanen, paulmck,
frederic, neeraj.upadhyay, joelagnelf, josh, urezki,
mathieu.desnoyers, jiangshanlai, qiang.zhang, juri.lelli,
vincent.guittot, dietmar.eggemann, bsegall, mgorman, vschneid,
chuck.lever, neil, okorniev, Dai.Ngo, tom, trondmy, anna, kees,
bigeasy, clrkwllms, ada.coupriediaz, kristina.martsenko,
wangkefeng.wang, broonie, kevin.brodsky, dwmw, shakeel.butt, ast,
ziy, yuzhao, baolin.wang, usamaarif642, joel.granados,
richard.weiyang, geert+renesas, tim.c.chen, linux,
alexander.shishkin, lillian, chenhuacai, francesco,
guoweikang.kernel, link, jpoimboe, masahiroy, brauner,
thomas.weissschuh, oleg, mjguzik, andrii, wangfushuai, linux-doc,
linux-arm-kernel, linux-media, linaro-mm-sig, linux-i2c,
linux-arch, linux-modules, rcu, linux-nfs, linux-rt-devel
In-Reply-To: <aN_fel4Rpqz6TPsD@J2N7QTR9R3>
On Fri, Oct 03, 2025 at 03:36:42PM +0100, Mark Rutland wrote:
> On Thu, Oct 02, 2025 at 05:12:09PM +0900, Byungchul Park wrote:
> > dept needs to notice every entrance from user to kernel mode to treat
> > every kernel context independently when tracking wait-event dependencies.
> > Roughly, system call and user oriented fault are the cases.
> >
> > Make dept aware of the entrances of arm64 and add support
> > CONFIG_ARCH_HAS_DEPT_SUPPORT to arm64.
> >
> > Signed-off-by: Byungchul Park <byungchul@sk.com>
> > ---
> > arch/arm64/Kconfig | 1 +
> > arch/arm64/kernel/syscall.c | 7 +++++++
> > arch/arm64/mm/fault.c | 7 +++++++
> > 3 files changed, 15 insertions(+)
> >
> > diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> > index e9bbfacc35a6..a8fab2c052dc 100644
> > --- a/arch/arm64/Kconfig
> > +++ b/arch/arm64/Kconfig
> > @@ -281,6 +281,7 @@ config ARM64
> > select USER_STACKTRACE_SUPPORT
> > select VDSO_GETRANDOM
> > select VMAP_STACK
> > + select ARCH_HAS_DEPT_SUPPORT
> > help
> > ARM 64-bit (AArch64) Linux support.
> >
> > diff --git a/arch/arm64/kernel/syscall.c b/arch/arm64/kernel/syscall.c
> > index c442fcec6b9e..bbd306335179 100644
> > --- a/arch/arm64/kernel/syscall.c
> > +++ b/arch/arm64/kernel/syscall.c
> > @@ -7,6 +7,7 @@
> > #include <linux/ptrace.h>
> > #include <linux/randomize_kstack.h>
> > #include <linux/syscalls.h>
> > +#include <linux/dept.h>
> >
> > #include <asm/debug-monitors.h>
> > #include <asm/exception.h>
> > @@ -96,6 +97,12 @@ static void el0_svc_common(struct pt_regs *regs, int scno, int sc_nr,
> > * (Similarly for HVC and SMC elsewhere.)
> > */
> >
> > + /*
> > + * This is a system call from user mode. Make dept work with a
> > + * new kernel mode context.
> > + */
> > + dept_update_cxt();
>
> As Mark Brown pointed out in his replies, this patch is missing a whole
> bunch of cases and does not work correctly as-is.
>
> As Dave Hansen pointed out on the x86 patch, you shouldn't do this
> piecemeal in architecture code, and should instead work with the
> existing context tracking, e.g. by adding logic to
> enter_from_user_mode() and exit_to_user_mode(), or by reusing some
I will consider it. However, I need to check if there are not any waits
and events before enter_from_user_mode(), or after exit_to_user_mode()
since those functions aren't the outmost functions for kernel mode C
code anyway.
Byungchul
> existing context tracking logic that's called there.
>
> Mark.
^ permalink raw reply
* Re: [PATCH v17 09/47] arm64, dept: add support CONFIG_ARCH_HAS_DEPT_SUPPORT to arm64
From: Byungchul Park @ 2025-10-13 1:51 UTC (permalink / raw)
To: Mark Brown
Cc: linux-kernel, kernel_team, torvalds, damien.lemoal, linux-ide,
adilger.kernel, linux-ext4, mingo, peterz, will, tglx, rostedt,
joel, sashal, daniel.vetter, duyuyang, johannes.berg, tj, tytso,
willy, david, amir73il, gregkh, kernel-team, linux-mm, akpm,
mhocko, minchan, hannes, vdavydov.dev, sj, jglisse, dennis, cl,
penberg, rientjes, vbabka, ngupta, linux-block, josef,
linux-fsdevel, jack, jlayton, dan.j.williams, hch, djwong,
dri-devel, rodrigosiqueiramelo, melissa.srw, hamohammed.sa,
harry.yoo, chris.p.wilson, gwan-gyeong.mun, max.byungchul.park,
boqun.feng, longman, yunseong.kim, ysk, yeoreum.yun, netdev,
matthew.brost, her0gyugyu, corbet, catalin.marinas, bp,
dave.hansen, x86, hpa, luto, sumit.semwal, gustavo,
christian.koenig, andi.shyti, arnd, lorenzo.stoakes, Liam.Howlett,
rppt, surenb, mcgrof, petr.pavlu, da.gomez, samitolvanen, paulmck,
frederic, neeraj.upadhyay, joelagnelf, josh, urezki,
mathieu.desnoyers, jiangshanlai, qiang.zhang, juri.lelli,
vincent.guittot, dietmar.eggemann, bsegall, mgorman, vschneid,
chuck.lever, neil, okorniev, Dai.Ngo, tom, trondmy, anna, kees,
bigeasy, clrkwllms, mark.rutland, ada.coupriediaz,
kristina.martsenko, wangkefeng.wang, kevin.brodsky, dwmw,
shakeel.butt, ast, ziy, yuzhao, baolin.wang, usamaarif642,
joel.granados, richard.weiyang, geert+renesas, tim.c.chen, linux,
alexander.shishkin, lillian, chenhuacai, francesco,
guoweikang.kernel, link, jpoimboe, masahiroy, brauner,
thomas.weissschuh, oleg, mjguzik, andrii, wangfushuai, linux-doc,
linux-arm-kernel, linux-media, linaro-mm-sig, linux-i2c,
linux-arch, linux-modules, rcu, linux-nfs, linux-rt-devel
In-Reply-To: <b69ab7d0-ba5e-4d22-88ef-53e0ebf07869@sirena.org.uk>
On Fri, Oct 03, 2025 at 12:33:03PM +0100, Mark Brown wrote:
> On Fri, Oct 03, 2025 at 10:46:41AM +0900, Byungchul Park wrote:
> > On Thu, Oct 02, 2025 at 12:39:31PM +0100, Mark Brown wrote:
> > > On Thu, Oct 02, 2025 at 05:12:09PM +0900, Byungchul Park wrote:
> > > > dept needs to notice every entrance from user to kernel mode to treat
> > > > every kernel context independently when tracking wait-event dependencies.
> > > > Roughly, system call and user oriented fault are the cases.
>
> > > > Make dept aware of the entrances of arm64 and add support
> > > > CONFIG_ARCH_HAS_DEPT_SUPPORT to arm64.
>
> > > The description of what needs to be tracked probably needs some
> > > tightening up here, it's not clear to me for example why exceptions for
> > > mops or the vector extensions aren't included here, or what the
> > > distinction is with error faults like BTI or GCS not being tracked?
>
> > Thanks for the feedback but I'm afraid I don't get you. Can you explain
> > in more detail with example?
>
> Your commit log says we need to track every entrance from user mode to
> kernel mode but the code only adds tracking to syscalls and some memory
> faults. The exception types listed above (and some others) also result
> in entries to the kernel from userspace.
You're right. Each kernel mode context needs to be tracked
independently. Just for your information, context ID is used for making
DEPT only track waits and events within each context, preventing
tracking those across independent contexts to avoid false positives.
Currently, irq, fault, and system calls are covered. It should be taken
into account if any other exceptions can include waits and events anyway.
> > JFYI, pairs of wait and its event need to be tracked to see if each
> > event can be prevented from being reachable by other waits like:
>
> > context X context Y
> >
> > lock L
> > ...
> > initiate event A context start toward event A
> > ... ...
> > wait A // wait for event A and lock L // wait for unlock L and
> > // prevent unlock L // prevent event A
> > ... ...
> > unlock L unlock L
> > ...
> > event A
>
> > I meant things like this need to be tracked.
>
> I don't think that's at all clear from the above context, and the
> handling for some of the above exception types (eg, the vector
> extensions) includes taking locks.
A trivial thing to mention, each typical lock pair, lock and unlock, can
only happen within each independent context, not across different
contexts. So the context ID is not necessary for typical locks.
exception X
lock A;
...
unlock A; // already guarateed to unlock A in the context that lock A
// has been acqured in.
...
finish exception X and return back to user mode;
But yes, as you concern, we should take into account all the exceptions
if any can include general waits and events in it, to avoid unnecessary
false positives.
Thank you!
Byungchul
^ permalink raw reply
* Re: [PATCH v17 28/47] dept: add documentation for dept
From: Byungchul Park @ 2025-10-13 1:28 UTC (permalink / raw)
To: Bagas Sanjaya
Cc: linux-kernel, kernel_team, torvalds, damien.lemoal, linux-ide,
adilger.kernel, linux-ext4, mingo, peterz, will, tglx, rostedt,
joel, sashal, daniel.vetter, duyuyang, johannes.berg, tj, tytso,
willy, david, amir73il, gregkh, kernel-team, linux-mm, akpm,
mhocko, minchan, hannes, vdavydov.dev, sj, jglisse, dennis, cl,
penberg, rientjes, vbabka, ngupta, linux-block, josef,
linux-fsdevel, jack, jlayton, dan.j.williams, hch, djwong,
dri-devel, rodrigosiqueiramelo, melissa.srw, hamohammed.sa,
harry.yoo, chris.p.wilson, gwan-gyeong.mun, max.byungchul.park,
boqun.feng, longman, yunseong.kim, ysk, yeoreum.yun, netdev,
matthew.brost, her0gyugyu, corbet, catalin.marinas, bp,
dave.hansen, x86, hpa, luto, sumit.semwal, gustavo,
christian.koenig, andi.shyti, arnd, lorenzo.stoakes, Liam.Howlett,
rppt, surenb, mcgrof, petr.pavlu, da.gomez, samitolvanen, paulmck,
frederic, neeraj.upadhyay, joelagnelf, josh, urezki,
mathieu.desnoyers, jiangshanlai, qiang.zhang, juri.lelli,
vincent.guittot, dietmar.eggemann, bsegall, mgorman, vschneid,
chuck.lever, neil, okorniev, Dai.Ngo, tom, trondmy, anna, kees,
bigeasy, clrkwllms, mark.rutland, ada.coupriediaz,
kristina.martsenko, wangkefeng.wang, broonie, kevin.brodsky, dwmw,
shakeel.butt, ast, ziy, yuzhao, baolin.wang, usamaarif642,
joel.granados, richard.weiyang, geert+renesas, tim.c.chen, linux,
alexander.shishkin, lillian, chenhuacai, francesco,
guoweikang.kernel, link, jpoimboe, masahiroy, brauner,
thomas.weissschuh, oleg, mjguzik, andrii, wangfushuai, linux-doc,
linux-arm-kernel, linux-media, linaro-mm-sig, linux-i2c,
linux-arch, linux-modules, rcu, linux-nfs, linux-rt-devel
In-Reply-To: <aN84jKyrE1BumpLj@archie.me>
On Fri, Oct 03, 2025 at 09:44:28AM +0700, Bagas Sanjaya wrote:
> On Thu, Oct 02, 2025 at 05:12:28PM +0900, Byungchul Park wrote:
> > This document describes the concept and APIs of dept.
> >
> > Signed-off-by: Byungchul Park <byungchul@sk.com>
> > ---
> > Documentation/dependency/dept.txt | 735 ++++++++++++++++++++++++++
> > Documentation/dependency/dept_api.txt | 117 ++++
> > 2 files changed, 852 insertions(+)
> > create mode 100644 Documentation/dependency/dept.txt
> > create mode 100644 Documentation/dependency/dept_api.txt
>
> What about writing dept docs in reST (like the rest of kernel documentation)?
Sorry for late reply, but sure. I should and will. Thank you!
> ---- >8 ----
> diff --git a/Documentation/dependency/dept.txt b/Documentation/locking/dept.rst
> similarity index 92%
> rename from Documentation/dependency/dept.txt
> rename to Documentation/locking/dept.rst
> index 5dd358b96734e6..7b90a0d95f0876 100644
> --- a/Documentation/dependency/dept.txt
> +++ b/Documentation/locking/dept.rst
However, I'm not sure if dept is about locking. dept is about general
waits e.g. wait_for_completion(), wait_event(), and so on, rather than
just waits involved in typical locking mechanisms e.g. spin lock, mutex,
and so on.
[snip]
> > +
> > + context X context Y
> > +
/*
* Acquired A.
*/
> > + mutex_lock A
/*
* Request something that will be handled
* through e.g. wq, deamon or any its own
* way, and then do 'complete B'.
*/
request_xxx_and_complete_B();
/*
* The request from
* context Y has been
* done. So running
* toward 'complete B'.
*/
/*
* Wait for A to be
* released, but will
* never happen.
*/
> > + mutex_lock A <- DEADLOCK
/*
* Wait for 'complete B' to happen, but
* will never happen.
*/
> > + wait_for_complete B <- DEADLOCK
/*
* Never reachable.
*/
> > + complete B
/*
* Never reachable.
*/
> > + mutex_unlock A
> > + mutex_unlock A
>
> Can you explain how DEPT detects deadlock on the second example above (like
> the first one being described in "How DEPT works" section)?
Sure. I added the explanation inline above. Don't hesitate if you have
any questions. Thanks.
Byungchul
>
> Confused...
>
> --
> An old man doll... just what I always wanted! - Clara
^ permalink raw reply
* Re: [PATCH v17 28/47] dept: add documentation for dept
From: Byungchul Park @ 2025-10-13 1:03 UTC (permalink / raw)
To: Jonathan Corbet
Cc: linux-kernel, kernel_team, torvalds, damien.lemoal, linux-ide,
adilger.kernel, linux-ext4, mingo, peterz, will, tglx, rostedt,
joel, sashal, daniel.vetter, duyuyang, johannes.berg, tj, tytso,
willy, david, amir73il, gregkh, kernel-team, linux-mm, akpm,
mhocko, minchan, hannes, vdavydov.dev, sj, jglisse, dennis, cl,
penberg, rientjes, vbabka, ngupta, linux-block, josef,
linux-fsdevel, jack, jlayton, dan.j.williams, hch, djwong,
dri-devel, rodrigosiqueiramelo, melissa.srw, hamohammed.sa,
harry.yoo, chris.p.wilson, gwan-gyeong.mun, max.byungchul.park,
boqun.feng, longman, yunseong.kim, ysk, yeoreum.yun, netdev,
matthew.brost, her0gyugyu, catalin.marinas, bp, dave.hansen, x86,
hpa, luto, sumit.semwal, gustavo, christian.koenig, andi.shyti,
arnd, lorenzo.stoakes, Liam.Howlett, rppt, surenb, mcgrof,
petr.pavlu, da.gomez, samitolvanen, paulmck, frederic,
neeraj.upadhyay, joelagnelf, josh, urezki, mathieu.desnoyers,
jiangshanlai, qiang.zhang, juri.lelli, vincent.guittot,
dietmar.eggemann, bsegall, mgorman, vschneid, chuck.lever, neil,
okorniev, Dai.Ngo, tom, trondmy, anna, kees, bigeasy, clrkwllms,
mark.rutland, ada.coupriediaz, kristina.martsenko,
wangkefeng.wang, broonie, kevin.brodsky, dwmw, shakeel.butt, ast,
ziy, yuzhao, baolin.wang, usamaarif642, joel.granados,
richard.weiyang, geert+renesas, tim.c.chen, linux,
alexander.shishkin, lillian, chenhuacai, francesco,
guoweikang.kernel, link, jpoimboe, masahiroy, brauner,
thomas.weissschuh, oleg, mjguzik, andrii, wangfushuai, linux-doc,
linux-arm-kernel, linux-media, linaro-mm-sig, linux-i2c,
linux-arch, linux-modules, rcu, linux-nfs, linux-rt-devel
In-Reply-To: <87ldlssg1b.fsf@trenco.lwn.net>
On Thu, Oct 02, 2025 at 11:36:16PM -0600, Jonathan Corbet wrote:
> Byungchul Park <byungchul@sk.com> writes:
>
> > This document describes the concept and APIs of dept.
> >
> > Signed-off-by: Byungchul Park <byungchul@sk.com>
> > ---
> > Documentation/dependency/dept.txt | 735 ++++++++++++++++++++++++++
> > Documentation/dependency/dept_api.txt | 117 ++++
> > 2 files changed, 852 insertions(+)
> > create mode 100644 Documentation/dependency/dept.txt
> > create mode 100644 Documentation/dependency/dept_api.txt
>
> As already suggested, this should be in RST; you're already 95% of the
> way there. Also, please put it under Documentation/dev-tools; we don't
> need another top-level directory for this.
Sure. I will. Thanks!
Byungchul
>
> Thanks,
>
> jon
^ permalink raw reply
* Re: [PATCH v8 7/8] modpost: Create modalias for builtin modules
From: Charles Mirabile @ 2025-10-10 13:24 UTC (permalink / raw)
To: Nathan Chancellor
Cc: Nicolas Schier, Alexey Gladkov, da.gomez, linux-kbuild,
linux-kernel, linux-modules, masahiroy, mcgrof, petr.pavlu,
samitolvanen, sfr
In-Reply-To: <20251010053736.GA447238@ax162>
On Fri, Oct 10, 2025 at 1:37 AM Nathan Chancellor <nathan@kernel.org> wrote:
>
> On Thu, Oct 09, 2025 at 09:52:08PM +0200, Nicolas Schier wrote:
> > On Tue, Oct 07, 2025 at 12:15:21PM +0200, Alexey Gladkov wrote:
> > > Hm. Indeed. I haven't found a good solution yet, but you can use the
> > > following patch to unlock compilation. It won't solve the problem, it will
> > > only hide it.
> > >
> > > --- a/scripts/Makefile.vmlinux
> > > +++ b/scripts/Makefile.vmlinux
> > > @@ -84,7 +84,7 @@ endif
> > > remove-section-y := .modinfo
> > > remove-section-$(CONFIG_ARCH_VMLINUX_NEEDS_RELOCS) += '.rel*'
> > >
> > > -remove-symbols := -w --strip-symbol='__mod_device_table__*'
> > > +remove-symbols := -w --strip-unneeded-symbol='__mod_device_table__*'
> > >
> > > # To avoid warnings: "empty loadable segment detected at ..." from GNU objcopy,
> > > # it is necessary to remove the PT_LOAD flag from the segment.
> > >
> >
> > Is it problematic to hide that? Otherwise we'd have to revert the
> > patch, right?
>
> Yeah, I would much prefer to ending up with pointless
> __mod_device_table__ symbols in the final binary than erroring out
> during the build... Does this happen with other architectures? I have
> not seen any reports yet but I have not tested anything yet. Why is
> RISC-V special here?
>
> It seems like the relocation comes from the .LASANLOC4 symbol in
> .data.rel.local?
>
> $ llvm-objdump -Dr drivers/irqchip/irq-riscv-aplic-main.o
> ...
> Disassembly of section .data.rel.local:
> ...
> 0000000000000130 <.LASANLOC4>:
> ...
> 1c0: 0000 unimp
> 00000000000001c0: R_RISCV_64 __mod_device_table__kmod_irq_riscv_aplic_main__acpi__aplic_acpi_match
> ...
>
> I cannot find much information about this ASANLOC outside of its
> location within the GCC sources, do we even need it? I don't see a way
> to opt out of this section altogether or on a per-variable basis, I
> wonder if there is some way to strip it out...
It seems from reading the gcc source that these are emitted to contain
information about where global declarations came from, presumably to
allow nicer ASAN error messages. (i.e. memory corruption affected this
symbol defined here). I don't think that KASAN uses these, but I am
not sure. I also don't know how they could be removed. I found a hit
using grep.app https://grep.app/search?case=true&q=ASANLOC in the
apache/nuttx repository where they seem to be doing something with
these symbols in a linker script, but I am not familiar enough with
linker scripts, or that project to fully understand what is going on.
>
> I plan to send the initial 6.18 Kbuild fixes pull request on Saturday.
> If we cannot figure out a real solution before then, maybe we can just
> switch to '--strip-unneeded-symbol' with a comment to upgrade that to
> '--strip-symbol' when possible?
>
> Cheers,
> Nathan
>
^ permalink raw reply
* Re: [PATCH v8 7/8] modpost: Create modalias for builtin modules
From: Alexey Gladkov @ 2025-10-10 10:58 UTC (permalink / raw)
To: Nathan Chancellor
Cc: Nicolas Schier, Charles Mirabile, da.gomez, linux-kbuild,
linux-kernel, linux-modules, masahiroy, mcgrof, petr.pavlu,
samitolvanen, sfr
In-Reply-To: <aOi9hqyvMg4bmXAw@example.org>
On Fri, Oct 10, 2025 at 10:02:20AM +0200, Alexey Gladkov wrote:
> On Thu, Oct 09, 2025 at 10:37:36PM -0700, Nathan Chancellor wrote:
> > On Thu, Oct 09, 2025 at 09:52:08PM +0200, Nicolas Schier wrote:
> > > On Tue, Oct 07, 2025 at 12:15:21PM +0200, Alexey Gladkov wrote:
> > > > Hm. Indeed. I haven't found a good solution yet, but you can use the
> > > > following patch to unlock compilation. It won't solve the problem, it will
> > > > only hide it.
> > > >
> > > > --- a/scripts/Makefile.vmlinux
> > > > +++ b/scripts/Makefile.vmlinux
> > > > @@ -84,7 +84,7 @@ endif
> > > > remove-section-y := .modinfo
> > > > remove-section-$(CONFIG_ARCH_VMLINUX_NEEDS_RELOCS) += '.rel*'
> > > >
> > > > -remove-symbols := -w --strip-symbol='__mod_device_table__*'
> > > > +remove-symbols := -w --strip-unneeded-symbol='__mod_device_table__*'
> > > >
> > > > # To avoid warnings: "empty loadable segment detected at ..." from GNU objcopy,
> > > > # it is necessary to remove the PT_LOAD flag from the segment.
> > > >
> > >
> > > Is it problematic to hide that? Otherwise we'd have to revert the
> > > patch, right?
> >
> > Yeah, I would much prefer to ending up with pointless
> > __mod_device_table__ symbols in the final binary than erroring out
> > during the build...
>
> This is a very unpleasant problem, but it does not seem fatal. There will
> not be many such characters in the final vmlinux. In the configuration
> from the bug report, there are only:
>
> $ nm vmlinux.unstripped.riscv |grep -c __mod_device_table__
> 17
>
> Of course, this does not mean that the problem does not need to be solved.
>
> > Does this happen with other architectures? I have
> > not seen any reports yet but I have not tested anything yet.
>
> LDFLAGS_vmlinux for riscv was taken from arm64. I suspect that there may
> be the same problem there. But I haven't checked yet whether the problem
> actually exists on arm64.
I tried to compile the kernel for arm64 with CONFIG_RELOCATABLE=y.
It works without errors and the symbols are removed.
vmlinux.unstripped: ELF 64-bit LSB pie executable, ARM aarch64, version 1 (SYSV), statically linked, not stripped
--
Rgrds, legion
^ permalink raw reply
* Re: [PATCH v8 7/8] modpost: Create modalias for builtin modules
From: Alexey Gladkov @ 2025-10-10 8:02 UTC (permalink / raw)
To: Nathan Chancellor
Cc: Nicolas Schier, Charles Mirabile, da.gomez, linux-kbuild,
linux-kernel, linux-modules, masahiroy, mcgrof, petr.pavlu,
samitolvanen, sfr
In-Reply-To: <20251010053736.GA447238@ax162>
On Thu, Oct 09, 2025 at 10:37:36PM -0700, Nathan Chancellor wrote:
> On Thu, Oct 09, 2025 at 09:52:08PM +0200, Nicolas Schier wrote:
> > On Tue, Oct 07, 2025 at 12:15:21PM +0200, Alexey Gladkov wrote:
> > > Hm. Indeed. I haven't found a good solution yet, but you can use the
> > > following patch to unlock compilation. It won't solve the problem, it will
> > > only hide it.
> > >
> > > --- a/scripts/Makefile.vmlinux
> > > +++ b/scripts/Makefile.vmlinux
> > > @@ -84,7 +84,7 @@ endif
> > > remove-section-y := .modinfo
> > > remove-section-$(CONFIG_ARCH_VMLINUX_NEEDS_RELOCS) += '.rel*'
> > >
> > > -remove-symbols := -w --strip-symbol='__mod_device_table__*'
> > > +remove-symbols := -w --strip-unneeded-symbol='__mod_device_table__*'
> > >
> > > # To avoid warnings: "empty loadable segment detected at ..." from GNU objcopy,
> > > # it is necessary to remove the PT_LOAD flag from the segment.
> > >
> >
> > Is it problematic to hide that? Otherwise we'd have to revert the
> > patch, right?
>
> Yeah, I would much prefer to ending up with pointless
> __mod_device_table__ symbols in the final binary than erroring out
> during the build...
This is a very unpleasant problem, but it does not seem fatal. There will
not be many such characters in the final vmlinux. In the configuration
from the bug report, there are only:
$ nm vmlinux.unstripped.riscv |grep -c __mod_device_table__
17
Of course, this does not mean that the problem does not need to be solved.
> Does this happen with other architectures? I have
> not seen any reports yet but I have not tested anything yet.
LDFLAGS_vmlinux for riscv was taken from arm64. I suspect that there may
be the same problem there. But I haven't checked yet whether the problem
actually exists on arm64.
> Why is RISC-V special here?
This problem on riscv only occurs when CONFIG_RELOCATABLE=y is specified.
Without this parameter, everything will compile as expected.
> It seems like the relocation comes from the .LASANLOC4 symbol in
> .data.rel.local?
>
> $ llvm-objdump -Dr drivers/irqchip/irq-riscv-aplic-main.o
> ...
> Disassembly of section .data.rel.local:
> ...
> 0000000000000130 <.LASANLOC4>:
> ...
> 1c0: 0000 unimp
> 00000000000001c0: R_RISCV_64 __mod_device_table__kmod_irq_riscv_aplic_main__acpi__aplic_acpi_match
> ...
>
> I cannot find much information about this ASANLOC outside of its
> location within the GCC sources, do we even need it? I don't see a way
> to opt out of this section altogether or on a per-variable basis, I
> wonder if there is some way to strip it out...
The aplic_acpi_match structure is indeed used, but they are used
themselves, not their alias, which is generated by the MODULE_DEVICE_TABLE
macro.
I also asked the guys from binutils for help:
https://sourceware.org/pipermail/binutils/2025-October/144782.html
> I plan to send the initial 6.18 Kbuild fixes pull request on Saturday.
> If we cannot figure out a real solution before then, maybe we can just
> switch to '--strip-unneeded-symbol' with a comment to upgrade that to
> '--strip-symbol' when possible?
Yes, that would be great.
Maybe I'm looking in the wrong direction, but still.
On riscv:
* with CONFIG_RELOCATABLE=y (where the error appears):
vmlinux.unstripped: ELF 64-bit LSB shared object, UCB RISC-V, RVC, soft-float ABI, version 1 (SYSV), dynamically linked, not stripped
* without CONFIG_RELOCATABLE:
vmlinux.unstripped: ELF 64-bit LSB executable, UCB RISC-V, RVC, soft-float ABI, version 1 (SYSV), statically linked, not stripped
On x64_64:
* with and without CONFIG_RELOCATABLE=y:
vmlinux.unstripped: ELF 64-bit LSB executable, x86-64, version 1 (SYSV), statically linked, not stripped
--
Rgrds, legion
^ permalink raw reply
* Re: [PATCH v2 2/3] media: radio: si470x: Fix DRIVER_AUTHOR macro definition
From: Mauro Carvalho Chehab @ 2025-10-10 7:10 UTC (permalink / raw)
To: Kees Cook
Cc: Luis Chamberlain, Hans Verkuil, Hans Verkuil,
Mauro Carvalho Chehab, Uwe Kleine-König, linux-media,
Malcolm Priestley, Rusty Russell, Petr Pavlu, Daniel Gomez,
Sami Tolvanen, linux-kernel, linux-modules, linux-hardening
In-Reply-To: <20251010030610.3032147-2-kees@kernel.org>
Em Thu, 9 Oct 2025 20:06:08 -0700
Kees Cook <kees@kernel.org> escreveu:
> The DRIVER_AUTHOR macro incorrectly included a semicolon in its
> string literal definition. Right now, this wasn't causing any real
> problem, but coming changes to the MODULE_INFO() macro make this more
> sensitive. Specifically, when used with MODULE_AUTHOR(), this created
> syntax errors during macro expansion:
>
> MODULE_AUTHOR(DRIVER_AUTHOR);
>
> expands to:
>
> MODULE_INFO(author, "Joonyoung Shim <jy0922.shim@samsung.com>";)
> ^
> syntax error
>
> Remove the trailing semicolon from the DRIVER_AUTHOR definition.
> Semicolons should only appear at the point of use, not in the macro
> definition.
>
> Reviewed-by: Hans Verkuil <hverkuil+cisco@kernel.org>
> Signed-off-by: Kees Cook <kees@kernel.org>
LGTM.
Reviewed-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
> ---
> Cc: Hans Verkuil <hverkuil@kernel.org>
> Cc: Mauro Carvalho Chehab <mchehab@kernel.org>
> Cc: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> Cc: <linux-media@vger.kernel.org>
> ---
> drivers/media/radio/si470x/radio-si470x-i2c.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/media/radio/si470x/radio-si470x-i2c.c b/drivers/media/radio/si470x/radio-si470x-i2c.c
> index cdd2ac198f2c..3932a449a1b1 100644
> --- a/drivers/media/radio/si470x/radio-si470x-i2c.c
> +++ b/drivers/media/radio/si470x/radio-si470x-i2c.c
> @@ -10,7 +10,7 @@
>
>
> /* driver definitions */
> -#define DRIVER_AUTHOR "Joonyoung Shim <jy0922.shim@samsung.com>";
> +#define DRIVER_AUTHOR "Joonyoung Shim <jy0922.shim@samsung.com>"
> #define DRIVER_CARD "Silicon Labs Si470x FM Radio"
> #define DRIVER_DESC "I2C radio driver for Si470x FM Radio Receivers"
> #define DRIVER_VERSION "1.0.2"
^ permalink raw reply
* Re: [PATCH v2 1/3] media: dvb-usb-v2: lmedm04: Fix firmware macro definitions
From: Mauro Carvalho Chehab @ 2025-10-10 7:10 UTC (permalink / raw)
To: Kees Cook
Cc: Luis Chamberlain, Hans Verkuil, Malcolm Priestley,
Mauro Carvalho Chehab, linux-media, Hans Verkuil,
Uwe Kleine-König, Rusty Russell, Petr Pavlu, Daniel Gomez,
Sami Tolvanen, linux-kernel, linux-modules, linux-hardening
In-Reply-To: <20251010030610.3032147-1-kees@kernel.org>
Em Thu, 9 Oct 2025 20:06:07 -0700
Kees Cook <kees@kernel.org> escreveu:
> The firmware filename macros incorrectly included semicolons in their
> string literal definitions. Right now, this wasn't causing any real
> problem, but coming changes to the MODULE_INFO() macro make this more
> sensitive. Specifically, when used with MODULE_FIRMWARE(), this
> created syntax errors during macro expansion:
>
> MODULE_FIRMWARE(LME2510_C_S7395);
>
> expands to:
>
> MODULE_INFO(firmware, "dvb-usb-lme2510c-s7395.fw";)
> ^
> syntax error
>
> Remove the trailing semicolons from all six firmware filename macro
> definitions. Semicolons should only appear at the point of use, not in
> the macro definition.
>
> Reviewed-by: Hans Verkuil <hverkuil+cisco@kernel.org>
> Signed-off-by: Kees Cook <kees@kernel.org>
LGTM.
Reviewed-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
> ---
> Cc: Malcolm Priestley <tvboxspy@gmail.com>
> Cc: Mauro Carvalho Chehab <mchehab@kernel.org>
> Cc: <linux-media@vger.kernel.org>
> ---
> drivers/media/usb/dvb-usb-v2/lmedm04.c | 12 ++++++------
> 1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/media/usb/dvb-usb-v2/lmedm04.c b/drivers/media/usb/dvb-usb-v2/lmedm04.c
> index 0c510035805b..05c18b6de5c6 100644
> --- a/drivers/media/usb/dvb-usb-v2/lmedm04.c
> +++ b/drivers/media/usb/dvb-usb-v2/lmedm04.c
> @@ -70,12 +70,12 @@
> #include "ts2020.h"
>
>
> -#define LME2510_C_S7395 "dvb-usb-lme2510c-s7395.fw";
> -#define LME2510_C_LG "dvb-usb-lme2510c-lg.fw";
> -#define LME2510_C_S0194 "dvb-usb-lme2510c-s0194.fw";
> -#define LME2510_C_RS2000 "dvb-usb-lme2510c-rs2000.fw";
> -#define LME2510_LG "dvb-usb-lme2510-lg.fw";
> -#define LME2510_S0194 "dvb-usb-lme2510-s0194.fw";
> +#define LME2510_C_S7395 "dvb-usb-lme2510c-s7395.fw"
> +#define LME2510_C_LG "dvb-usb-lme2510c-lg.fw"
> +#define LME2510_C_S0194 "dvb-usb-lme2510c-s0194.fw"
> +#define LME2510_C_RS2000 "dvb-usb-lme2510c-rs2000.fw"
> +#define LME2510_LG "dvb-usb-lme2510-lg.fw"
> +#define LME2510_S0194 "dvb-usb-lme2510-s0194.fw"
>
> /* debug */
> static int dvb_usb_lme2510_debug;
^ permalink raw reply
* Re: [PATCH v8 7/8] modpost: Create modalias for builtin modules
From: Nathan Chancellor @ 2025-10-10 5:37 UTC (permalink / raw)
To: Nicolas Schier
Cc: Alexey Gladkov, Charles Mirabile, da.gomez, linux-kbuild,
linux-kernel, linux-modules, masahiroy, mcgrof, petr.pavlu,
samitolvanen, sfr
In-Reply-To: <aOgSaNejdcBWKXx8@levanger>
On Thu, Oct 09, 2025 at 09:52:08PM +0200, Nicolas Schier wrote:
> On Tue, Oct 07, 2025 at 12:15:21PM +0200, Alexey Gladkov wrote:
> > Hm. Indeed. I haven't found a good solution yet, but you can use the
> > following patch to unlock compilation. It won't solve the problem, it will
> > only hide it.
> >
> > --- a/scripts/Makefile.vmlinux
> > +++ b/scripts/Makefile.vmlinux
> > @@ -84,7 +84,7 @@ endif
> > remove-section-y := .modinfo
> > remove-section-$(CONFIG_ARCH_VMLINUX_NEEDS_RELOCS) += '.rel*'
> >
> > -remove-symbols := -w --strip-symbol='__mod_device_table__*'
> > +remove-symbols := -w --strip-unneeded-symbol='__mod_device_table__*'
> >
> > # To avoid warnings: "empty loadable segment detected at ..." from GNU objcopy,
> > # it is necessary to remove the PT_LOAD flag from the segment.
> >
>
> Is it problematic to hide that? Otherwise we'd have to revert the
> patch, right?
Yeah, I would much prefer to ending up with pointless
__mod_device_table__ symbols in the final binary than erroring out
during the build... Does this happen with other architectures? I have
not seen any reports yet but I have not tested anything yet. Why is
RISC-V special here?
It seems like the relocation comes from the .LASANLOC4 symbol in
.data.rel.local?
$ llvm-objdump -Dr drivers/irqchip/irq-riscv-aplic-main.o
...
Disassembly of section .data.rel.local:
...
0000000000000130 <.LASANLOC4>:
...
1c0: 0000 unimp
00000000000001c0: R_RISCV_64 __mod_device_table__kmod_irq_riscv_aplic_main__acpi__aplic_acpi_match
...
I cannot find much information about this ASANLOC outside of its
location within the GCC sources, do we even need it? I don't see a way
to opt out of this section altogether or on a per-variable basis, I
wonder if there is some way to strip it out...
I plan to send the initial 6.18 Kbuild fixes pull request on Saturday.
If we cannot figure out a real solution before then, maybe we can just
switch to '--strip-unneeded-symbol' with a comment to upgrade that to
'--strip-symbol' when possible?
Cheers,
Nathan
^ permalink raw reply
* Re: [PATCH v2 3/3] module: Add compile-time check for embedded NUL characters
From: Petr Pavlu @ 2025-10-10 4:19 UTC (permalink / raw)
To: Kees Cook
Cc: Luis Chamberlain, Rusty Russell, Daniel Gomez, Sami Tolvanen,
linux-modules, Hans Verkuil, Malcolm Priestley,
Mauro Carvalho Chehab, Hans Verkuil, Uwe Kleine-König,
linux-kernel, linux-media, linux-hardening
In-Reply-To: <20251010030610.3032147-3-kees@kernel.org>
On 10/10/25 5:06 AM, Kees Cook wrote:
> Long ago, the kernel module license checks were bypassed by embedding a
> NUL character in the MODULE_LICENSE() string[1]. By using a string like
> "GPL\0proprietary text", the kernel would only read "GPL" due to C string
> termination at the NUL byte, allowing proprietary modules to avoid kernel
> tainting and access GPL-only symbols.
>
> The MODULE_INFO() macro stores these strings in the .modinfo ELF
> section, and get_next_modinfo() uses strcmp()-family functions
> which stop at the first NUL. This split the embedded string into two
> separate .modinfo entries, with only the first part being processed by
> license_is_gpl_compatible().
>
> Add a compile-time check using static_assert that compares the full
> string length (sizeof - 1) against __builtin_strlen(), which stops at
> the first NUL. If they differ, compilation fails with a clear error
> message.
>
> While this check can still be circumvented by modifying the ELF binary
> post-compilation, it prevents accidental embedded NULs and forces
> intentional abuse to require deliberate binary manipulation rather than
> simple source-level tricks.
>
> Build tested with test modules containing both valid and invalid license
> strings. The check correctly rejects:
>
> MODULE_LICENSE("GPL\0proprietary")
>
> while accepting normal declarations:
>
> MODULE_LICENSE("GPL")
>
> Link: https://lwn.net/Articles/82305/ [1]
> Suggested-by: Rusty Russell <rusty@rustcorp.com.au>
> Signed-off-by: Kees Cook <kees@kernel.org>
Reviewed-by: Petr Pavlu <petr.pavlu@suse.com>
--
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