* [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
* [PATCH v2 1/3] media: dvb-usb-v2: lmedm04: Fix firmware macro definitions
From: Kees Cook @ 2025-10-10 3:06 UTC (permalink / raw)
To: Luis Chamberlain
Cc: Kees Cook, 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: <20251010030348.it.784-kees@kernel.org>
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>
---
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;
--
2.34.1
^ permalink raw reply related
* [PATCH v2 3/3] module: Add compile-time check for embedded NUL characters
From: Kees Cook @ 2025-10-10 3:06 UTC (permalink / raw)
To: Luis Chamberlain
Cc: Kees Cook, Rusty Russell, Petr Pavlu, 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: <20251010030348.it.784-kees@kernel.org>
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>
---
Cc: Luis Chamberlain <mcgrof@kernel.org>
Cc: Petr Pavlu <petr.pavlu@suse.com>
Cc: Daniel Gomez <da.gomez@kernel.org>
Cc: Sami Tolvanen <samitolvanen@google.com>
Cc: <linux-modules@vger.kernel.org>
---
include/linux/moduleparam.h | 3 +++
1 file changed, 3 insertions(+)
diff --git a/include/linux/moduleparam.h b/include/linux/moduleparam.h
index 6907aedc4f74..915f32f7d888 100644
--- a/include/linux/moduleparam.h
+++ b/include/linux/moduleparam.h
@@ -26,6 +26,9 @@
/* Generic info of form tag = "info" */
#define MODULE_INFO(tag, info) \
+ static_assert( \
+ sizeof(info) - 1 == __builtin_strlen(info), \
+ "MODULE_INFO(" #tag ", ...) contains embedded NUL byte"); \
static const char __UNIQUE_ID(modinfo)[] \
__used __section(".modinfo") __aligned(1) \
= __MODULE_INFO_PREFIX __stringify(tag) "=" info
--
2.34.1
^ permalink raw reply related
* [PATCH v2 2/3] media: radio: si470x: Fix DRIVER_AUTHOR macro definition
From: Kees Cook @ 2025-10-10 3:06 UTC (permalink / raw)
To: Luis Chamberlain
Cc: Kees Cook, 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: <20251010030348.it.784-kees@kernel.org>
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>
---
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"
--
2.34.1
^ permalink raw reply related
* [PATCH v2 0/3] module: Add compile-time check for embedded NUL characters
From: Kees Cook @ 2025-10-10 3:06 UTC (permalink / raw)
To: Luis Chamberlain
Cc: Kees Cook, Hans Verkuil, Malcolm Priestley, Mauro Carvalho Chehab,
Hans Verkuil, Uwe Kleine-König, Rusty Russell, Petr Pavlu,
Daniel Gomez, Sami Tolvanen, linux-kernel, linux-media,
linux-modules, linux-hardening
v2:
- use static_assert instead of _Static_assert
- add Hans's Reviewed-by's
v1: https://lore.kernel.org/lkml/20251008033844.work.801-kees@kernel.org/
Hi!
A long time ago we had an issue with embedded NUL bytes in MODULE_INFO
strings[1]. While this stands out pretty strongly when you look at the
code, and we can't do anything about a binary module that just plain lies,
we never actually implemented the trivial compile-time check needed to
detect it.
Add this check (and fix 2 instances of needless trailing semicolons that
this change exposed).
Note that these patches were produced as part of another LLM exercise.
This time I wanted to try "what happens if I ask an LLM to go read
a specific LWN article and write a patch based on a discussion?" It
pretty effortlessly chose and implemented a suggested solution, tested
the change, and fixed new build warnings in the process.
Since this was a relatively short session, here's an overview of the
prompts involved as I guided it through a clean change and tried to see
how it would reason about static_assert vs _Static_assert. (It wanted
to use what was most common, not what was the current style -- we may
want to update the comment above the static_assert macro to suggest
using _Static_assert directly these days...)
I want to fix a weakness in the module info strings. Read about it
here: https://lwn.net/Articles/82305/
Since it's only "info" that we need to check, can you reduce the checks
to just that instead of all the other stuff?
I think the change to the comment is redundent, and that should be
in a commit log instead. Let's just keep the change to the static assert.
Is "static_assert" the idiomatic way to use a static assert in this
code base? I've seen _Static_assert used sometimes.
What's the difference between the two?
Does Linux use C11 by default now?
Then let's not use the wrapper any more.
Do an "allmodconfig all -s" build to verify this works for all modules
in the kernel.
Thanks!
-Kees
[1] https://lwn.net/Articles/82305/
Kees Cook (3):
media: dvb-usb-v2: lmedm04: Fix firmware macro definitions
media: radio: si470x: Fix DRIVER_AUTHOR macro definition
module: Add compile-time check for embedded NUL characters
include/linux/moduleparam.h | 3 +++
drivers/media/radio/si470x/radio-si470x-i2c.c | 2 +-
drivers/media/usb/dvb-usb-v2/lmedm04.c | 12 ++++++------
3 files changed, 10 insertions(+), 7 deletions(-)
--
2.34.1
^ permalink raw reply
* Re: [PATCH v8 7/8] modpost: Create modalias for builtin modules
From: Nicolas Schier @ 2025-10-09 19:52 UTC (permalink / raw)
To: Alexey Gladkov
Cc: Charles Mirabile, da.gomez, linux-kbuild, linux-kernel,
linux-modules, masahiroy, mcgrof, nathan, petr.pavlu,
samitolvanen, sfr
In-Reply-To: <aOToOeNGiaFVM0Ds@example.org>
On Tue, Oct 07, 2025 at 12:15:21PM +0200, Alexey Gladkov wrote:
> On Mon, Oct 06, 2025 at 09:16:37PM -0400, Charles Mirabile wrote:
> > On Thu, Sep 18, 2025 at 10:05:51AM +0200, Alexey Gladkov wrote:
> > > For some modules, modalias is generated using the modpost utility and
> > > the section is added to the module file.
> > >
> > > When a module is added inside vmlinux, modpost does not generate
> > > modalias for such modules and the information is lost.
> > >
> > > As a result kmod (which uses modules.builtin.modinfo in userspace)
> > > cannot determine that modalias is handled by a builtin kernel module.
> > >
> > > $ cat /sys/devices/pci0000:00/0000:00:14.0/modalias
> > > pci:v00008086d0000A36Dsv00001043sd00008694bc0Csc03i30
> > >
> > > $ modinfo xhci_pci
> > > name: xhci_pci
> > > filename: (builtin)
> > > license: GPL
> > > file: drivers/usb/host/xhci-pci
> > > description: xHCI PCI Host Controller Driver
> > >
> > > Missing modalias "pci:v*d*sv*sd*bc0Csc03i30*" which will be generated by
> > > modpost if the module is built separately.
> > >
> > > To fix this it is necessary to generate the same modalias for vmlinux as
> > > for the individual modules. Fortunately '.vmlinux.export.o' is already
> > > generated from which '.modinfo' can be extracted in the same way as for
> > > vmlinux.o.
> >
> > Hi -
> >
> > This patch broke RISC-V builds for me. During the final objcopy where the new
> > symbols are supposed to be stripped, an error occurs producing lots of error
> > messages similar to this one:
> >
> > riscv64-linux-gnu-objcopy: not stripping symbol `__mod_device_table__...'
> > because it is named in a relocation
> >
> > It does not occur using defconfig, but I was able to bisect my way to this
> > commit and then reduce my config delta w.r.t defconfig until I landed on:
> >
> > cat > .config <<'EOF'
> > CONFIG_RELOCATABLE=y
> > CONFIG_KASAN=y
> > EOF
> > ARCH=riscv CROSS_COMPILE=riscv64-linux-gnu- make olddefconfig
> > ARCH=riscv CROSS_COMPILE=riscv64-linux-gnu- make -j $(nproc)
> > ...
> > LD vmlinux.unstripped
> > NM System.map
> > SORTTAB vmlinux.unstripped
> > CHKREL vmlinux.unstripped
> > OBJCOPY vmlinux
> > OBJCOPY modules.builtin.modinfo
> > GEN modules.builtin
> > riscv64-linux-gnu-objcopy: not stripping symbol `<long symbol name>'
> > because it is named in a relocation
> > <repeats with different symbol names about a dozen times>
> > make[3]: *** [scripts/Makefile.vmlinux:97: vmlinux] Error 1
> > make[3]: *** Deleting file 'vmlinux'
> > make[2]: *** [Makefile:1242: vmlinux] Error 2
> > make[1]: *** [/tmp/linux/Makefile:369: __build_one_by_one] Error 2
> > make: *** [Makefile:248: __sub-make] Error 2
> >
> > I confirmed that reverting this commit fixes the issue.
Thanks for the report!
>
> 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?
Kind regards,
Nicolas
^ permalink raw reply
* Re: modprobe returns 0 upon -EEXIST from insmod
From: Lucas De Marchi @ 2025-10-09 14:13 UTC (permalink / raw)
To: Petr Pavlu; +Cc: Phil Sutter, Christophe Leroy, linux-modules, Yi Chen
In-Reply-To: <ce7f293c-d9f9-4137-bcad-8cc492d34773@suse.com>
On Thu, Oct 09, 2025 at 03:47:42PM +0200, Petr Pavlu wrote:
>On 10/8/25 8:41 AM, Lucas De Marchi wrote:
>> On Tue, Aug 19, 2025 at 09:17:50AM -0500, Lucas De Marchi wrote:
>>> On Tue, Aug 19, 2025 at 10:52:16AM +0200, Petr Pavlu wrote:
>>>> On 8/18/25 11:34 AM, Phil Sutter wrote:
>>>>> On Sun, Aug 17, 2025 at 05:54:27PM +0200, Christophe Leroy wrote:
>>>>>> Le 17/08/2025 à 01:33, Phil Sutter a écrit :
>>>>>>> [Vous ne recevez pas souvent de courriers de phil@nwl.cc. D?couvrez pourquoi ceci est important ? https://aka.ms/LearnAboutSenderIdentification ]
>>>>>>>
>>>>>>> Hi,
>>>>>>>
>>>>>>> I admittedly didn't fully analyze the cause, but on my system a call to:
>>>>>>>
>>>>>>> # insmod /lib/module/$(uname -r)/kernel/net/netfilter/nf_conntrack_ftp.ko
>>>>>>>
>>>>>>> fails with -EEXIST (due to a previous call to 'nfct add helper ftp inet
>>>>>>> tcp'). A call to:
>>>>>>>
>>>>>>> # modprobe nf_conntrack_ftp
>>>>>>>
>>>>>>> though returns 0 even though module loading fails. Is there a bug in
>>>>>>> modprobe error status handling?
>>>>>>>
>>>>>>
>>>>>> Read the man page : https://linux.die.net/man/8/modprobe
>>>>>>
>>>>>> In the man page I see:
>>>>>>
>>>>>> Normally, modprobe will succeed (and do nothing) if told to
>>>>>> insert a module which is already present or to remove a module which
>>>>>> isn't present.
>>>>>
>>>>> This is not a case of already inserted module, it is not loaded before
>>>>> the call to modprobe. It is the module_init callback
>>>>> nf_conntrack_ftp_init() which returns -EEXIST it received from
>>>>> nf_conntrack_helpers_register().
>>>
>>> is this a real failure condition or something benign like "if it's
>>> already registered, there's nothing to do"?
>>>
>>>>>
>>>>> Can't user space distinguish the two causes of -EEXIST? Or in other
>>>>> words, is use of -EEXIST in module_init callbacks problematic?
>>>>
>>>> Unfortunately, error return codes from (f)init_module cannot be reliably
>>>> depended upon. For instance, cpufreq drivers have similar behavior of
>>>> returning -EEXIST when another cpufreq driver is already registered.
>>>> Returning this code unexpectedly can then confuse kmod, as it interprets
>>>> -EEXIST to mean "the module is already loaded" [1].
>>>
>>> well, it's not that it can't be relied on. There's 1 exit code that is
>>> treated specially, EEXISTS, because that error is used by the module
>>> loading part, before the module_init call, to signify the module is
>>> already loaded.
>>>
>>>>
>>>> I have thought about this problem before. We might fix the main
>>>> problematic occurrences, but we can't really audit all the code that
>>>> module init functions can invoke. I then wonder if it would make sense
>>>> for the module loader to warn about any -EEXIST returned by a module's
>>>> init function and translate it to -EBUSY.
>>>
>>> If it's a failure condition then yes, -EBUSY looks appropriate.
>>
>> something like this:
>>
>>
>> diff --git a/kernel/module/main.c b/kernel/module/main.c
>> index c66b261849362..e5fb1a4ef3441 100644
>> --- a/kernel/module/main.c
>> +++ b/kernel/module/main.c
>> @@ -3038,6 +3038,11 @@ static noinline int do_init_module(struct module *mod)
>> if (mod->init != NULL)
>> ret = do_one_initcall(mod->init);
>> if (ret < 0) {
>> + if (ret == -EEXIST) {
>> + pr_warn("%s: '%s'->init suspiciously returned %d: Overriding with %d\n",
>> + __func__, mod->name, -EEXIST, -EBUSY);
>> + ret = -EBUSY;
>> + }
>> goto fail_free_freeinit;
>> }
>> if (ret > 0) {
>
>Yes, that's what I had in mind. Could you please send this as a proper
>patch to the list?
>
>I only think we should include a hint to explain why this is a problem
>and simplify the message somewhat, something like:
>
>pr_warn("%s: init suspiciously returned -EEXIST (reserved for loaded modules), overriding with -EBUSY\n", mod->name);
>
>I realize you based the message on the later warning about the init
>function returning a >0 value but I think we should rather update that
>message as well. It should follow the usual style of
>"<mod-name>: <error-description>". I suggest simplifying it to:
>
>pr_warn("%s: init suspiciously returned %d, it should follow 0/-E convention\n", mod->name, ret);
will do and actually run some tests to make sure it's not only
build-tested.
Thanks,
Lucas De Marchi
>
>--
>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