* [PATCH v3 08/24] dyndbg: factor ddebug_match_desc out from ddebug_change
From: Jim Cromie @ 2026-06-01 12:04 UTC (permalink / raw)
To: Jonathan Corbet, Shuah Khan, Arnd Bergmann, Jason Baron,
Luis Chamberlain, Petr Pavlu, Daniel Gomez, Sami Tolvanen,
Aaron Tomlin, Andrew Morton, Shuah Khan, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter
Cc: linux-doc, linux-kernel, linux-arch, linux-modules,
linux-kselftest, dri-devel, Jim Cromie, Louis Chauvet
In-Reply-To: <20260601-dd-maint-2-v3-0-4a15b241bd3c@gmail.com>
ddebug_change() is a big (~100 lines) function with a nested for loop.
The outer loop walks the per-module ddebug_tables list, and does
module stuff: it filters on a query's "module FOO*" and "class BAR",
failures here skip the entire inner loop.
The inner loop (60 lines) scans a module's descriptors. It starts
with a long block of filters on function, line, format, and the
validated "BAR" class (or the legacy/_DPRINTK_CLASS_DFLT).
These filters "continue" past pr_debugs that don't match the query
criteria, before it falls through the code below that counts matches,
then adjusts the flags and static-keys. This is unnecessarily hard to
think about.
So move the per-descriptor filter-block into a boolean function:
ddebug_match_desc(desc), and change each "continue" to "return false".
This puts a clear interface in place, so any future changes are either
inside, outside, or across this interface.
also fix checkpatch complaints about spaces and braces.
Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
Reviewed-by: Louis Chauvet <louis.chauvet@bootlin.com>
---
lib/dynamic_debug.c | 83 ++++++++++++++++++++++++++++++-----------------------
1 file changed, 47 insertions(+), 36 deletions(-)
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index 18a71a9108d3..6b1e983cfedc 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -172,6 +172,52 @@ static struct ddebug_class_map *ddebug_find_valid_class(struct ddebug_table cons
* callsites, normally the same as number of changes. If verbose,
* logs the changes. Takes ddebug_lock.
*/
+static bool ddebug_match_desc(const struct ddebug_query *query,
+ struct _ddebug *dp,
+ int valid_class)
+{
+ /* match site against query-class */
+ if (dp->class_id != valid_class)
+ return false;
+
+ /* match against the source filename */
+ if (query->filename &&
+ !match_wildcard(query->filename, dp->filename) &&
+ !match_wildcard(query->filename,
+ kbasename(dp->filename)) &&
+ !match_wildcard(query->filename,
+ trim_prefix(dp->filename)))
+ return false;
+
+ /* match against the function */
+ if (query->function &&
+ !match_wildcard(query->function, dp->function))
+ return false;
+
+ /* match against the format */
+ if (query->format) {
+ if (*query->format == '^') {
+ char *p;
+ /* anchored search. match must be at beginning */
+ p = strstr(dp->format, query->format + 1);
+ if (p != dp->format)
+ return false;
+ } else if (!strstr(dp->format, query->format)) {
+ return false;
+ }
+ }
+
+ /* match against the line number range */
+ if (query->first_lineno &&
+ dp->lineno < query->first_lineno)
+ return false;
+ if (query->last_lineno &&
+ dp->lineno > query->last_lineno)
+ return false;
+
+ return true;
+}
+
static int ddebug_change(const struct ddebug_query *query,
struct flag_settings *modifiers)
{
@@ -204,42 +250,7 @@ static int ddebug_change(const struct ddebug_query *query,
for (i = 0; i < dt->num_ddebugs; i++) {
struct _ddebug *dp = &dt->ddebugs[i];
- /* match site against query-class */
- if (dp->class_id != valid_class)
- continue;
-
- /* match against the source filename */
- if (query->filename &&
- !match_wildcard(query->filename, dp->filename) &&
- !match_wildcard(query->filename,
- kbasename(dp->filename)) &&
- !match_wildcard(query->filename,
- trim_prefix(dp->filename)))
- continue;
-
- /* match against the function */
- if (query->function &&
- !match_wildcard(query->function, dp->function))
- continue;
-
- /* match against the format */
- if (query->format) {
- if (*query->format == '^') {
- char *p;
- /* anchored search. match must be at beginning */
- p = strstr(dp->format, query->format+1);
- if (p != dp->format)
- continue;
- } else if (!strstr(dp->format, query->format))
- continue;
- }
-
- /* match against the line number range */
- if (query->first_lineno &&
- dp->lineno < query->first_lineno)
- continue;
- if (query->last_lineno &&
- dp->lineno > query->last_lineno)
+ if (!ddebug_match_desc(query, dp, valid_class))
continue;
nfound++;
--
2.54.0
^ permalink raw reply related
* [PATCH v3 07/24] dyndbg.lds.S: fix lost dyndbg sections in modules
From: Jim Cromie @ 2026-06-01 12:04 UTC (permalink / raw)
To: Jonathan Corbet, Shuah Khan, Arnd Bergmann, Jason Baron,
Luis Chamberlain, Petr Pavlu, Daniel Gomez, Sami Tolvanen,
Aaron Tomlin, Andrew Morton, Shuah Khan, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter
Cc: linux-doc, linux-kernel, linux-arch, linux-modules,
linux-kselftest, dri-devel, Jim Cromie
In-Reply-To: <20260601-dd-maint-2-v3-0-4a15b241bd3c@gmail.com>
With CONFIG_DRM_USE_DYNAMIC_DEBUG=y, several build configs had
problems with __dyndbg* sections getting lost in drm drivers. Fix
this by following the model demonstrated in codetag.lds.h.
Introduce include/asm-generic/dyndbg.lds.h, to bundle dynamic-debug's
multiple sections together, into 2 macros:
vmlinux.lds.h DATA_DATA: move the 2 BOUNDED_SECTION_BY(__dyndbg*)
calls into dyndbg.lds.h DYNDBG_SECTIONS(). vmlinux.lds.h now includes
the new file and calls the new macro.
MOD_DYNDBG_SECTIONS keeps the 2 sections by name, aligns them and sets
the output address to 0 when the sections are empty.
dyndbg.lds.h includes (reuses) bounded-section.lds.h
scripts/module.lds.S: now calls MOD_DYNDBG_SECTIONS right before the
CODETAG macro (consistent with their placements in vmlinux.lds.h), and
also includes dyndbg.lds.h
This isolates vmlinux.lds.h from further __dyndbg section additions.
CC: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
Reviewed-by: Petr Pavlu <petr.pavlu@suse.com>
---
v3: move #includes to top, drop extra ALIGN(8) in DYNDBG_SECTIONS, add RvBy
v2: Address linker script review feedback for relocatable modules.
MOD_DYNDBG_SECTIONS() used the BOUNDED_SECTION_BY() macro, which
proved problematic for kernel modules for two reasons:
1. Unwanted Empty Sections:
BOUNDED_SECTION_BY() automatically generates `__start` and `__stop`
symbols. When applied to `MOD_DYNDBG_SECTIONS()`, the linker assumes
the sections are populated due to the symbol definitions, forcing an
empty `__dyndbg` and `__dyndbg_classes` output section in every
compiled module, even those without dynamic debug configuration.
Since the module loader uses `section_objs()` to locate data via
ELF headers instead of relying on `__start`/`__stop` symbols, these
assignments are completely unnecessary.
2. Non-zero Output Addresses:
During relocatable linking (e.g., `ld.bfd -r`), omitting an explicit
base address causes the section to inherit the current location
counter. This results in non-zero sh_addr values in `.ko` files,
which is confusing, degrades compressibility, and can cause issues
with external tools parsing the ELF.
Fix both issues by dropping `BOUNDED_SECTION_BY()` in favor of a simple
`KEEP(*(...))` constraint and explicitly defining the sections with a `0`
base address: `__dyndbg 0 : ALIGN(8) { ... }`.
fixup-inc-vml
---
MAINTAINERS | 1 +
include/asm-generic/dyndbg.lds.h | 18 ++++++++++++++++++
include/asm-generic/vmlinux.lds.h | 6 ++----
scripts/module.lds.S | 2 ++
4 files changed, 23 insertions(+), 4 deletions(-)
diff --git a/MAINTAINERS b/MAINTAINERS
index 9ec290e38b44..6cf80e7ac039 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -9083,6 +9083,7 @@ DYNAMIC DEBUG
M: Jason Baron <jbaron@akamai.com>
M: Jim Cromie <jim.cromie@gmail.com>
S: Maintained
+F: include/asm-generic/dyndbg.lds.h
F: include/linux/dynamic_debug.h
F: lib/dynamic_debug.c
F: lib/test_dynamic_debug.c
diff --git a/include/asm-generic/dyndbg.lds.h b/include/asm-generic/dyndbg.lds.h
new file mode 100644
index 000000000000..9d8951bef688
--- /dev/null
+++ b/include/asm-generic/dyndbg.lds.h
@@ -0,0 +1,18 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+#ifndef __ASM_GENERIC_DYNDBG_LDS_H
+#define __ASM_GENERIC_DYNDBG_LDS_H
+
+#include <asm-generic/bounded_sections.lds.h>
+#define DYNDBG_SECTIONS() \
+ BOUNDED_SECTION_BY(__dyndbg, ___dyndbg) \
+ BOUNDED_SECTION_BY(__dyndbg_classes, ___dyndbg_classes)
+
+#define MOD_DYNDBG_SECTIONS() \
+ __dyndbg 0 : ALIGN(8) { \
+ KEEP(*(__dyndbg)) \
+ } \
+ __dyndbg_classes 0 : ALIGN(8) { \
+ KEEP(*(__dyndbg_classes)) \
+ }
+
+#endif /* __ASM_GENERIC_DYNDBG_LDS_H */
diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
index 2b1becd809be..0a1994bb8793 100644
--- a/include/asm-generic/vmlinux.lds.h
+++ b/include/asm-generic/vmlinux.lds.h
@@ -52,6 +52,7 @@
#include <asm-generic/bounded_sections.lds.h>
#include <asm-generic/codetag.lds.h>
+#include <asm-generic/dyndbg.lds.h>
#ifndef LOAD_OFFSET
#define LOAD_OFFSET 0
@@ -344,10 +345,7 @@
*(.data..do_once) \
STRUCT_ALIGN(); \
*(__tracepoints) \
- /* implement dynamic printk debug */ \
- . = ALIGN(8); \
- BOUNDED_SECTION_BY(__dyndbg_classes, ___dyndbg_classes) \
- BOUNDED_SECTION_BY(__dyndbg, ___dyndbg) \
+ DYNDBG_SECTIONS() \
CODETAG_SECTIONS() \
LIKELY_PROFILE() \
BRANCH_PROFILE() \
diff --git a/scripts/module.lds.S b/scripts/module.lds.S
index b62683061d79..2e62dc5bd5d4 100644
--- a/scripts/module.lds.S
+++ b/scripts/module.lds.S
@@ -10,6 +10,7 @@
#endif
#include <asm-generic/codetag.lds.h>
+#include <asm-generic/dyndbg.lds.h>
SECTIONS {
/DISCARD/ : {
@@ -61,6 +62,7 @@ SECTIONS {
*(.rodata..L*)
}
+ MOD_DYNDBG_SECTIONS()
MOD_SEPARATE_CODETAG_SECTIONS()
}
--
2.54.0
^ permalink raw reply related
* [PATCH v3 06/24] vmlinux.lds.h: remove redundant ALIGN(8) directives
From: Jim Cromie @ 2026-06-01 12:04 UTC (permalink / raw)
To: Jonathan Corbet, Shuah Khan, Arnd Bergmann, Jason Baron,
Luis Chamberlain, Petr Pavlu, Daniel Gomez, Sami Tolvanen,
Aaron Tomlin, Andrew Morton, Shuah Khan, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter
Cc: linux-doc, linux-kernel, linux-arch, linux-modules,
linux-kselftest, dri-devel, Jim Cromie
In-Reply-To: <20260601-dd-maint-2-v3-0-4a15b241bd3c@gmail.com>
The BOUNDED_SECTION_PRE_LABEL and BOUNDED_SECTION_POST_LABEL macros
were recently updated to inherently enforce an 8-byte alignment. This
makes the explicit '. = ALIGN(8);' statements preceding 'naked' macro
calls in vmlinux.lds.h redundant.
Remove these redundant alignment directives to clean up the file and
clarify that the macros handle their own alignment padding.
Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
---
include/asm-generic/vmlinux.lds.h | 13 -------------
1 file changed, 13 deletions(-)
diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
index 145beb14b94b..2b1becd809be 100644
--- a/include/asm-generic/vmlinux.lds.h
+++ b/include/asm-generic/vmlinux.lds.h
@@ -228,7 +228,6 @@
#ifdef CONFIG_KPROBES
#define KPROBE_BLACKLIST() \
- . = ALIGN(8); \
BOUNDED_SECTION(_kprobe_blacklist)
#else
#define KPROBE_BLACKLIST()
@@ -244,7 +243,6 @@
#ifdef CONFIG_EVENT_TRACING
#define FTRACE_EVENTS() \
- . = ALIGN(8); \
BOUNDED_SECTION(_ftrace_events) \
BOUNDED_SECTION_BY(_ftrace_eval_map, _ftrace_eval_maps)
#else
@@ -261,7 +259,6 @@
#ifdef CONFIG_FTRACE_SYSCALLS
#define TRACE_SYSCALLS() \
- . = ALIGN(8); \
BOUNDED_SECTION_BY(__syscalls_metadata, _syscalls_metadata)
#else
#define TRACE_SYSCALLS()
@@ -276,7 +273,6 @@
#ifdef CONFIG_SERIAL_EARLYCON
#define EARLYCON_TABLE() \
- . = ALIGN(8); \
BOUNDED_SECTION_POST_LABEL(__earlycon_table, __earlycon_table, , _end)
#else
#define EARLYCON_TABLE()
@@ -284,11 +280,9 @@
#ifdef CONFIG_SECURITY
#define LSM_TABLE() \
- . = ALIGN(8); \
BOUNDED_SECTION_PRE_LABEL(.lsm_info.init, _lsm_info, __start, __end)
#define EARLY_LSM_TABLE() \
- . = ALIGN(8); \
BOUNDED_SECTION_PRE_LABEL(.early_lsm_info.init, _early_lsm_info, __start, __end)
#else
#define LSM_TABLE()
@@ -314,7 +308,6 @@
#ifdef CONFIG_ACPI
#define ACPI_PROBE_TABLE(name) \
- . = ALIGN(8); \
BOUNDED_SECTION_POST_LABEL(__##name##_acpi_probe_table, \
__##name##_acpi_probe_table,, _end)
#else
@@ -323,7 +316,6 @@
#ifdef CONFIG_THERMAL
#define THERMAL_TABLE(name) \
- . = ALIGN(8); \
BOUNDED_SECTION_POST_LABEL(__##name##_thermal_table, \
__##name##_thermal_table,, _end)
#else
@@ -403,12 +395,10 @@
__end_init_stack = .;
#define JUMP_TABLE_DATA \
- . = ALIGN(8); \
BOUNDED_SECTION_BY(__jump_table, ___jump_table)
#ifdef CONFIG_HAVE_STATIC_CALL_INLINE
#define STATIC_CALL_DATA \
- . = ALIGN(8); \
BOUNDED_SECTION_BY(.static_call_sites, _static_call_sites) \
BOUNDED_SECTION_BY(.static_call_tramp_key, _static_call_tramp_key)
#else
@@ -453,7 +443,6 @@
*(.rodata) *(.rodata.*) *(.data.rel.ro*) \
SCHED_DATA \
RO_AFTER_INIT_DATA /* Read only after init */ \
- . = ALIGN(8); \
BOUNDED_SECTION_BY(__tracepoints_ptrs, ___tracepoints_ptrs) \
*(__tracepoints_strings)/* Tracepoints: strings */ \
} \
@@ -947,12 +936,10 @@
/* Alignment must be consistent with (kunit_suite *) in include/kunit/test.h */
#define KUNIT_TABLE() \
- . = ALIGN(8); \
BOUNDED_SECTION_POST_LABEL(.kunit_test_suites, __kunit_suites, _start, _end)
/* Alignment must be consistent with (kunit_suite *) in include/kunit/test.h */
#define KUNIT_INIT_TABLE() \
- . = ALIGN(8); \
BOUNDED_SECTION_POST_LABEL(.kunit_init_test_suites, \
__kunit_init_suites, _start, _end)
--
2.54.0
^ permalink raw reply related
* [PATCH v3 05/24] vmlinux.lds.h: Fix ALIGN(8) omission causing NULL ptr on i386
From: Jim Cromie @ 2026-06-01 12:04 UTC (permalink / raw)
To: Jonathan Corbet, Shuah Khan, Arnd Bergmann, Jason Baron,
Luis Chamberlain, Petr Pavlu, Daniel Gomez, Sami Tolvanen,
Aaron Tomlin, Andrew Morton, Shuah Khan, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter
Cc: linux-doc, linux-kernel, linux-arch, linux-modules,
linux-kselftest, dri-devel, Jim Cromie, Louis Chauvet
In-Reply-To: <20260601-dd-maint-2-v3-0-4a15b241bd3c@gmail.com>
Almost all uses of the BOUNDED_SECTION macros are ALIGN(8), either
explicitly, or by being below an aligned section containing x*8 byte
objects. The noteworthy exception is BOUNDED_SECTION(__dyndbg), which
immediately follows BOUNDED_SECTION(__dyndbg_classes).
On i386, struct _ddebug_classmap is 28 bytes, so without an explicit
ALIGN(8) in the macro, the following __dyndbg section gets misaligned,
causing a NULL ptr deref in dynamic_debug_init().
So fix this with an explicit ALIGN(8) in the existing BOUNDED_SECTION
macros, and introduce _ALIGNED variants to handle the cases with an
explicit . = ALIGN(x)
Also add explicit alignments for: EXCEPTION_TABLE, ORC_UNWIND_TABLE,
TRACEDATA, INIT_SETUP, and NOTES.
update BOUNDED_SECTION uses inside . = ALIGN(x) stanzas to use
_ALIGNED variants, but keep the outer ALIGNs so the symbols between
them are not "re-aligned".
In particular, scripts/sorttable.c does not tolerate sloppy padding.
At the top of ORC_UNWIND_TABLE, add . = ALIGN(4) to match the struct
orc_header __align() call in the code:
commit b9f174c811e3 ("x86/unwind/orc: Add ELF section with ORC version identifier")
Suggested-by: Louis Chauvet <louis.chauvet@bootlin.com> # _ALIGNED variants.
Link: https://lore.kernel.org/lkml/177402491426.6181.12855763650074831089.b4-review@b4/
Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
---
v3:
sashiko complained about NOTES and .BTF_ids.
gemini asserts that NOTES are natively 4-byte aligned, add comment repeating it.
.BTF_ids doesnt use BOUNDED_BY, since start/end isnt needed;
sashiko evidently got confused by immediately preceding usage.
v2:
sashiko picked up 2 cases, added to the explicit list above
https://sashiko.dev/#/patchset/20260515-asm-generic-1-v3-0-680b273666d4%40gmail.com
---
include/asm-generic/bounded_sections.lds.h | 17 ++++++++++++++---
include/asm-generic/vmlinux.lds.h | 18 ++++++++++--------
2 files changed, 24 insertions(+), 11 deletions(-)
diff --git a/include/asm-generic/bounded_sections.lds.h b/include/asm-generic/bounded_sections.lds.h
index 268cdc34389b..8ff3e3420f60 100644
--- a/include/asm-generic/bounded_sections.lds.h
+++ b/include/asm-generic/bounded_sections.lds.h
@@ -3,19 +3,30 @@
#ifndef _ASM_GENERIC_BOUNDED_SECTIONS_H
#define _ASM_GENERIC_BOUNDED_SECTIONS_H
-#define BOUNDED_SECTION_PRE_LABEL(_sec_, _label_, _BEGIN_, _END_) \
+#define BOUNDED_SECTION_PRE_LABEL_ALIGNED(_sec_, _label_, _BEGIN_, _END_, _ALIGNED_) \
+ . = ALIGN(_ALIGNED_); \
_BEGIN_##_label_ = .; \
KEEP(*(_sec_)) \
_END_##_label_ = .;
-#define BOUNDED_SECTION_POST_LABEL(_sec_, _label_, _BEGIN_, _END_) \
+#define BOUNDED_SECTION_PRE_LABEL(_sec_, _label_, _BEGIN_, _END_) \
+ BOUNDED_SECTION_PRE_LABEL_ALIGNED(_sec_, _label_, _BEGIN_, _END_, 8)
+
+#define BOUNDED_SECTION_POST_LABEL_ALIGNED(_sec_, _label_, _BEGIN_, _END_, _ALIGNED_) \
+ . = ALIGN(_ALIGNED_); \
_label_##_BEGIN_ = .; \
KEEP(*(_sec_)) \
_label_##_END_ = .;
+#define BOUNDED_SECTION_POST_LABEL(_sec_, _label_, _BEGIN_, _END_) \
+ BOUNDED_SECTION_POST_LABEL_ALIGNED(_sec_, _label_, _BEGIN_, _END_, 8)
+
#define BOUNDED_SECTION_BY(_sec_, _label_) \
BOUNDED_SECTION_PRE_LABEL(_sec_, _label_, __start, __stop)
-#define BOUNDED_SECTION(_sec) BOUNDED_SECTION_BY(_sec, _sec)
+#define BOUNDED_SECTION_BY_ALIGNED(_sec_, _label_, _ALIGNED_) \
+ BOUNDED_SECTION_PRE_LABEL_ALIGNED(_sec_, _label_, __start, __stop, _ALIGNED_)
+
+#define BOUNDED_SECTION(_sec) BOUNDED_SECTION_BY(_sec, _sec)
#endif /* _ASM_GENERIC_BOUNDED_SECTIONS_H */
diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
index 9c61dd083f26..145beb14b94b 100644
--- a/include/asm-generic/vmlinux.lds.h
+++ b/include/asm-generic/vmlinux.lds.h
@@ -640,7 +640,7 @@
#define EXCEPTION_TABLE(align) \
. = ALIGN(align); \
__ex_table : AT(ADDR(__ex_table) - LOAD_OFFSET) { \
- BOUNDED_SECTION_BY(__ex_table, ___ex_table) \
+ BOUNDED_SECTION_BY_ALIGNED(__ex_table, ___ex_table, align) \
}
/*
@@ -650,7 +650,7 @@
#define BTF \
. = ALIGN(PAGE_SIZE); \
.BTF : AT(ADDR(.BTF) - LOAD_OFFSET) { \
- BOUNDED_SECTION_BY(.BTF, _BTF) \
+ BOUNDED_SECTION_BY_ALIGNED(.BTF, _BTF, PAGE_SIZE) \
} \
. = ALIGN(PAGE_SIZE); \
.BTF_ids : AT(ADDR(.BTF_ids) - LOAD_OFFSET) { \
@@ -832,16 +832,17 @@
#ifdef CONFIG_UNWINDER_ORC
#define ORC_UNWIND_TABLE \
+ . = ALIGN(4); \
.orc_header : AT(ADDR(.orc_header) - LOAD_OFFSET) { \
- BOUNDED_SECTION_BY(.orc_header, _orc_header) \
+ BOUNDED_SECTION_BY_ALIGNED(.orc_header, _orc_header, 4) \
} \
. = ALIGN(4); \
.orc_unwind_ip : AT(ADDR(.orc_unwind_ip) - LOAD_OFFSET) { \
- BOUNDED_SECTION_BY(.orc_unwind_ip, _orc_unwind_ip) \
+ BOUNDED_SECTION_BY_ALIGNED(.orc_unwind_ip, _orc_unwind_ip, 4)\
} \
. = ALIGN(2); \
.orc_unwind : AT(ADDR(.orc_unwind) - LOAD_OFFSET) { \
- BOUNDED_SECTION_BY(.orc_unwind, _orc_unwind) \
+ BOUNDED_SECTION_BY_ALIGNED(.orc_unwind, _orc_unwind, 2) \
} \
text_size = _etext - _stext; \
. = ALIGN(4); \
@@ -869,7 +870,7 @@
#define TRACEDATA \
. = ALIGN(4); \
.tracedata : AT(ADDR(.tracedata) - LOAD_OFFSET) { \
- BOUNDED_SECTION_POST_LABEL(.tracedata, __tracedata, _start, _end) \
+ BOUNDED_SECTION_POST_LABEL_ALIGNED(.tracedata, __tracedata, _start, _end, 4) \
}
#else
#define TRACEDATA
@@ -898,13 +899,14 @@
*(.note.gnu.property) \
} \
.notes : AT(ADDR(.notes) - LOAD_OFFSET) { \
- BOUNDED_SECTION_BY(.note.*, _notes) \
+ /* *(.note.*) are natively 4-byte aligned */ \
+ BOUNDED_SECTION_BY_ALIGNED(.note.*, _notes, 4) \
} NOTES_HEADERS \
NOTES_HEADERS_RESTORE
#define INIT_SETUP(initsetup_align) \
. = ALIGN(initsetup_align); \
- BOUNDED_SECTION_POST_LABEL(.init.setup, __setup, _start, _end)
+ BOUNDED_SECTION_POST_LABEL_ALIGNED(.init.setup, __setup, _start, _end, initsetup_align)
#define INIT_CALLS_LEVEL(level) \
__initcall##level##_start = .; \
--
2.54.0
^ permalink raw reply related
* [PATCH v3 04/24] vmlinux.lds.h: drop unused HEADERED_SECTION* macros
From: Jim Cromie @ 2026-06-01 12:04 UTC (permalink / raw)
To: Jonathan Corbet, Shuah Khan, Arnd Bergmann, Jason Baron,
Luis Chamberlain, Petr Pavlu, Daniel Gomez, Sami Tolvanen,
Aaron Tomlin, Andrew Morton, Shuah Khan, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter
Cc: linux-doc, linux-kernel, linux-arch, linux-modules,
linux-kselftest, dri-devel, Jim Cromie
In-Reply-To: <20260601-dd-maint-2-v3-0-4a15b241bd3c@gmail.com>
These macros are unused, no point in carrying them any more.
NB: these macros were just moved to bounded_sections.lds.h, from
vmlinux.lds.h, which is the known entity, and therefore more
meaningful in the 1-line summary, so thats what I used as the topic.
Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
---
include/asm-generic/bounded_sections.lds.h | 15 ---------------
1 file changed, 15 deletions(-)
diff --git a/include/asm-generic/bounded_sections.lds.h b/include/asm-generic/bounded_sections.lds.h
index 8c29293ca7fb..268cdc34389b 100644
--- a/include/asm-generic/bounded_sections.lds.h
+++ b/include/asm-generic/bounded_sections.lds.h
@@ -18,19 +18,4 @@
#define BOUNDED_SECTION(_sec) BOUNDED_SECTION_BY(_sec, _sec)
-#define HEADERED_SECTION_PRE_LABEL(_sec_, _label_, _BEGIN_, _END_, _HDR_) \
- _HDR_##_label_ = .; \
- KEEP(*(.gnu.linkonce.##_sec_)) \
- BOUNDED_SECTION_PRE_LABEL(_sec_, _label_, _BEGIN_, _END_)
-
-#define HEADERED_SECTION_POST_LABEL(_sec_, _label_, _BEGIN_, _END_, _HDR_) \
- _label_##_HDR_ = .; \
- KEEP(*(.gnu.linkonce.##_sec_)) \
- BOUNDED_SECTION_POST_LABEL(_sec_, _label_, _BEGIN_, _END_)
-
-#define HEADERED_SECTION_BY(_sec_, _label_) \
- HEADERED_SECTION_PRE_LABEL(_sec_, _label_, __start, __stop)
-
-#define HEADERED_SECTION(_sec) HEADERED_SECTION_BY(_sec, _sec)
-
#endif /* _ASM_GENERIC_BOUNDED_SECTIONS_H */
--
2.54.0
^ permalink raw reply related
* [PATCH v3 03/24] vmlinux.lds.h: refactor BOUNDED_SECTION_* macros into bounded_sections.lds.h
From: Jim Cromie @ 2026-06-01 12:04 UTC (permalink / raw)
To: Jonathan Corbet, Shuah Khan, Arnd Bergmann, Jason Baron,
Luis Chamberlain, Petr Pavlu, Daniel Gomez, Sami Tolvanen,
Aaron Tomlin, Andrew Morton, Shuah Khan, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter
Cc: linux-doc, linux-kernel, linux-arch, linux-modules,
linux-kselftest, dri-devel, Jim Cromie
In-Reply-To: <20260601-dd-maint-2-v3-0-4a15b241bd3c@gmail.com>
Move BOUNDED_SECTION_* macros to a new helper file:
include/asm-generic/bounded_sections.lds.h and include it back into
vmlinux.lds.h. This allows its reuse later to fix a failure to keep
dyndbg sections in some circumstances.
NOTES:
These macros are only for use in vmlinux.lds.h, where the _start &
_end symbols are needed. Modules keep sections separate in ELF
sections, with their boundaries known, so the _start and _end are not
useful, and may confuse tools not expecting them.
This patch ignores a checkpatch warning, because new file is covered
by "GENERIC INCLUDE/ASM HEADER FILES" in MAINTAINERS
CC: Arnd Bergmann <arnd@arndb.de>
CC: linux-arch@vger.kernel.org
Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
---
v3: move include to top
---
include/asm-generic/bounded_sections.lds.h | 36 ++++++++++++++++++++++++++++++
include/asm-generic/vmlinux.lds.h | 31 +------------------------
2 files changed, 37 insertions(+), 30 deletions(-)
diff --git a/include/asm-generic/bounded_sections.lds.h b/include/asm-generic/bounded_sections.lds.h
new file mode 100644
index 000000000000..8c29293ca7fb
--- /dev/null
+++ b/include/asm-generic/bounded_sections.lds.h
@@ -0,0 +1,36 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+
+#ifndef _ASM_GENERIC_BOUNDED_SECTIONS_H
+#define _ASM_GENERIC_BOUNDED_SECTIONS_H
+
+#define BOUNDED_SECTION_PRE_LABEL(_sec_, _label_, _BEGIN_, _END_) \
+ _BEGIN_##_label_ = .; \
+ KEEP(*(_sec_)) \
+ _END_##_label_ = .;
+
+#define BOUNDED_SECTION_POST_LABEL(_sec_, _label_, _BEGIN_, _END_) \
+ _label_##_BEGIN_ = .; \
+ KEEP(*(_sec_)) \
+ _label_##_END_ = .;
+
+#define BOUNDED_SECTION_BY(_sec_, _label_) \
+ BOUNDED_SECTION_PRE_LABEL(_sec_, _label_, __start, __stop)
+
+#define BOUNDED_SECTION(_sec) BOUNDED_SECTION_BY(_sec, _sec)
+
+#define HEADERED_SECTION_PRE_LABEL(_sec_, _label_, _BEGIN_, _END_, _HDR_) \
+ _HDR_##_label_ = .; \
+ KEEP(*(.gnu.linkonce.##_sec_)) \
+ BOUNDED_SECTION_PRE_LABEL(_sec_, _label_, _BEGIN_, _END_)
+
+#define HEADERED_SECTION_POST_LABEL(_sec_, _label_, _BEGIN_, _END_, _HDR_) \
+ _label_##_HDR_ = .; \
+ KEEP(*(.gnu.linkonce.##_sec_)) \
+ BOUNDED_SECTION_POST_LABEL(_sec_, _label_, _BEGIN_, _END_)
+
+#define HEADERED_SECTION_BY(_sec_, _label_) \
+ HEADERED_SECTION_PRE_LABEL(_sec_, _label_, __start, __stop)
+
+#define HEADERED_SECTION(_sec) HEADERED_SECTION_BY(_sec, _sec)
+
+#endif /* _ASM_GENERIC_BOUNDED_SECTIONS_H */
diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
index 60c8c22fd3e4..9c61dd083f26 100644
--- a/include/asm-generic/vmlinux.lds.h
+++ b/include/asm-generic/vmlinux.lds.h
@@ -50,6 +50,7 @@
* [__nosave_begin, __nosave_end] for the nosave data
*/
+#include <asm-generic/bounded_sections.lds.h>
#include <asm-generic/codetag.lds.h>
#ifndef LOAD_OFFSET
@@ -211,36 +212,6 @@
# endif
#endif
-#define BOUNDED_SECTION_PRE_LABEL(_sec_, _label_, _BEGIN_, _END_) \
- _BEGIN_##_label_ = .; \
- KEEP(*(_sec_)) \
- _END_##_label_ = .;
-
-#define BOUNDED_SECTION_POST_LABEL(_sec_, _label_, _BEGIN_, _END_) \
- _label_##_BEGIN_ = .; \
- KEEP(*(_sec_)) \
- _label_##_END_ = .;
-
-#define BOUNDED_SECTION_BY(_sec_, _label_) \
- BOUNDED_SECTION_PRE_LABEL(_sec_, _label_, __start, __stop)
-
-#define BOUNDED_SECTION(_sec) BOUNDED_SECTION_BY(_sec, _sec)
-
-#define HEADERED_SECTION_PRE_LABEL(_sec_, _label_, _BEGIN_, _END_, _HDR_) \
- _HDR_##_label_ = .; \
- KEEP(*(.gnu.linkonce.##_sec_)) \
- BOUNDED_SECTION_PRE_LABEL(_sec_, _label_, _BEGIN_, _END_)
-
-#define HEADERED_SECTION_POST_LABEL(_sec_, _label_, _BEGIN_, _END_, _HDR_) \
- _label_##_HDR_ = .; \
- KEEP(*(.gnu.linkonce.##_sec_)) \
- BOUNDED_SECTION_POST_LABEL(_sec_, _label_, _BEGIN_, _END_)
-
-#define HEADERED_SECTION_BY(_sec_, _label_) \
- HEADERED_SECTION_PRE_LABEL(_sec_, _label_, __start, __stop)
-
-#define HEADERED_SECTION(_sec) HEADERED_SECTION_BY(_sec, _sec)
-
#ifdef CONFIG_TRACE_BRANCH_PROFILING
#define LIKELY_PROFILE() \
BOUNDED_SECTION_BY(_ftrace_annotated_branch, _annotated_branch_profile)
--
2.54.0
^ permalink raw reply related
* [PATCH v3 02/24] docs/dyndbg: explain flags parse 1st
From: Jim Cromie @ 2026-06-01 12:04 UTC (permalink / raw)
To: Jonathan Corbet, Shuah Khan, Arnd Bergmann, Jason Baron,
Luis Chamberlain, Petr Pavlu, Daniel Gomez, Sami Tolvanen,
Aaron Tomlin, Andrew Morton, Shuah Khan, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter
Cc: linux-doc, linux-kernel, linux-arch, linux-modules,
linux-kselftest, dri-devel, Jim Cromie, Louis Chauvet
In-Reply-To: <20260601-dd-maint-2-v3-0-4a15b241bd3c@gmail.com>
When writing queries to >control, flags are parsed 1st, since they are
the only required field, and they require specific compositions. So
if the flags draw an error (on those specifics), then keyword errors
aren't reported. This can be mildly confusing/annoying, so explain it
instead.
cc: linux-doc@vger.kernel.org
Reviewed-by: Louis Chauvet <louis.chauvet@bootlin.com>
Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
---
Documentation/admin-guide/dynamic-debug-howto.rst | 17 +++++++++++++----
1 file changed, 13 insertions(+), 4 deletions(-)
diff --git a/Documentation/admin-guide/dynamic-debug-howto.rst b/Documentation/admin-guide/dynamic-debug-howto.rst
index 4b14d9fd0300..9c2f096ed1d8 100644
--- a/Documentation/admin-guide/dynamic-debug-howto.rst
+++ b/Documentation/admin-guide/dynamic-debug-howto.rst
@@ -109,10 +109,19 @@ The match-spec's select *prdbgs* from the catalog, upon which to apply
the flags-spec, all constraints are ANDed together. An absent keyword
is the same as keyword "*".
-
-A match specification is a keyword, which selects the attribute of
-the callsite to be compared, and a value to compare against. Possible
-keywords are:::
+Note that since the match-spec can be empty, the flags are checked 1st,
+then the pairs of keyword and value. Flag errs will hide keyword errs::
+
+ bash-5.2# ddcmd mod bar +foo
+ dyndbg: read 13 bytes from userspace
+ dyndbg: query 0: "mod bar +foo" mod:*
+ dyndbg: unknown flag 'o'
+ dyndbg: flags parse failed
+ dyndbg: processed 1 queries, with 0 matches, 1 errs
+
+So a match-spec is a keyword, which selects the attribute of the
+callsite to be compared, and a value to compare against. Possible
+keywords are::
match-spec ::= 'func' string |
'file' string |
--
2.54.0
^ permalink raw reply related
* [PATCH v3 01/24] docs/dyndbg: update examples \012 to \n
From: Jim Cromie @ 2026-06-01 12:04 UTC (permalink / raw)
To: Jonathan Corbet, Shuah Khan, Arnd Bergmann, Jason Baron,
Luis Chamberlain, Petr Pavlu, Daniel Gomez, Sami Tolvanen,
Aaron Tomlin, Andrew Morton, Shuah Khan, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter
Cc: linux-doc, linux-kernel, linux-arch, linux-modules,
linux-kselftest, dri-devel, Jim Cromie, Louis Chauvet
In-Reply-To: <20260601-dd-maint-2-v3-0-4a15b241bd3c@gmail.com>
commit 47ea6f99d06e ("dyndbg: use ESCAPE_SPACE for cat control")
changed the control-file to display format strings with "\n" rather
than "\012". Update the docs to match the new reality.
Reviewed-by: Louis Chauvet <louis.chauvet@bootlin.com>
Tested-by: Louis Chauvet <louis.chauvet@bootlin.com>
Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
---
Documentation/admin-guide/dynamic-debug-howto.rst | 20 ++++++++++----------
1 file changed, 10 insertions(+), 10 deletions(-)
diff --git a/Documentation/admin-guide/dynamic-debug-howto.rst b/Documentation/admin-guide/dynamic-debug-howto.rst
index 095a63892257..4b14d9fd0300 100644
--- a/Documentation/admin-guide/dynamic-debug-howto.rst
+++ b/Documentation/admin-guide/dynamic-debug-howto.rst
@@ -38,12 +38,12 @@ You can view the currently configured behaviour in the *prdbg* catalog::
:#> head -n7 /proc/dynamic_debug/control
# filename:lineno [module]function flags format
- init/main.c:1179 [main]initcall_blacklist =_ "blacklisting initcall %s\012
- init/main.c:1218 [main]initcall_blacklisted =_ "initcall %s blacklisted\012"
- init/main.c:1424 [main]run_init_process =_ " with arguments:\012"
- init/main.c:1426 [main]run_init_process =_ " %s\012"
- init/main.c:1427 [main]run_init_process =_ " with environment:\012"
- init/main.c:1429 [main]run_init_process =_ " %s\012"
+ init/main.c:1179 [main]initcall_blacklist =_ "blacklisting initcall %s\n"
+ init/main.c:1218 [main]initcall_blacklisted =_ "initcall %s blacklisted\n"
+ init/main.c:1424 [main]run_init_process =_ " with arguments:\n"
+ init/main.c:1426 [main]run_init_process =_ " %s\n"
+ init/main.c:1427 [main]run_init_process =_ " with environment:\n"
+ init/main.c:1429 [main]run_init_process =_ " %s\n"
The 3rd space-delimited column shows the current flags, preceded by
a ``=`` for easy use with grep/cut. ``=p`` shows enabled callsites.
@@ -59,10 +59,10 @@ query/commands to the control file. Example::
:#> ddcmd '-p; module main func run* +p'
:#> grep =p /proc/dynamic_debug/control
- init/main.c:1424 [main]run_init_process =p " with arguments:\012"
- init/main.c:1426 [main]run_init_process =p " %s\012"
- init/main.c:1427 [main]run_init_process =p " with environment:\012"
- init/main.c:1429 [main]run_init_process =p " %s\012"
+ init/main.c:1424 [main]run_init_process =p " with arguments:\n"
+ init/main.c:1426 [main]run_init_process =p " %s\n"
+ init/main.c:1427 [main]run_init_process =p " with environment:\n"
+ init/main.c:1429 [main]run_init_process =p " %s\n"
Error messages go to console/syslog::
--
2.54.0
^ permalink raw reply related
* [PATCH v3 00/24] dynamic-debug cleanups refactors maintenance + alignment fix
From: Jim Cromie @ 2026-06-01 12:04 UTC (permalink / raw)
To: Jonathan Corbet, Shuah Khan, Arnd Bergmann, Jason Baron,
Luis Chamberlain, Petr Pavlu, Daniel Gomez, Sami Tolvanen,
Aaron Tomlin, Andrew Morton, Shuah Khan, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter
Cc: linux-doc, linux-kernel, linux-arch, linux-modules,
linux-kselftest, dri-devel, Jim Cromie, Louis Chauvet,
Łukasz Bartosik
This series starts with 2 doc-only patches updating the current state
of dynamic-debug, they were posted separately here, without pushback.
https://lore.kernel.org/linux-doc/20260502-dyndbg-doc-v1-0-67cc4a93a77e@gmail.com/
Next 5 are a fix to a linker-script alignment problem in 32bit arches
causing a null-ptr scanning dyndbg-descriptor section on i386. These
were reviewed by Petr Pavlu.
The remaining patches are cleanups, refactors in preparation for an
API change needed to fix a regression in DRM when it uses classmaps.
I split these out for easier review, I will follow up with the API
change afterwards.
The biggest revision vs V2 is the new patch: 25. It addresses a flaw
detected by sashiko which is best described by example.
Dyndbg uses KBUILD_MODNAME to provide module-name, this works well for
loadable modules (module loader requires unique module names), but for
builtin modules, is effectively kbasename, and not unique.
So we get 4 modules named "main": init/main, kernel/power/main,
kernel/base/poser/main. This ambiguity is visible in user-space since
the beginning of dyndbg.
Now suppose kernel/{,base}/power/main want to define classmaps to
categorize the various pr-debugs they have. The current code finds a
module's classmaps by strcmp on modname, so init/main will match
against classmaps defined by both kernel/{,base}/power/main.
The current code will also map "main" classes to kernel/*/power/main,
so they will probably work at first, but 2 independent classmaps can
both use class-ids 0-N, but will conflict if they're both used by a
module. Then we have classmap overlaps and unpredictable results.
patch-25 eliminates the ambiguity by using KBUILD_MODFILE to provide a
unique module-name, then adds matching against kbasename(modname) to
restore the legacy query behavior. It *does* change the modname
exposed in /proc/dynamic_debug/control, but not the result of a query
like "module main +p".
OLDER VERSIONS:
V2 primarily revises:
https://lore.kernel.org/lkml/20260504-dd-cleanups-2-v1-0-6fdd24040642@gmail.com/
V2 addressed most of sashiko's feedback on V1:
https://sashiko.dev/#/patchset/20260504-dd-cleanups-2-v1-0-6fdd24040642%40gmail.com
It dropped the pr-fmt patch, as not reproducible,
advanced the drop-NAMES patch to reduce subsequent churn,
and fixed the classmaps PARAMs to u64 to avoid 32bit flags on 32bit arches
For easy one-stop-shopping, V2 also included 2 smaller series:
1st fixes a section alignment problem, with Reviewed-by from Petr Pavlu
https://lore.kernel.org/lkml/20260515-asm-generic-1-v3-0-680b273666d4@gmail.com/
Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
---
Changes in v3:
- EDITME: describe what is new in this series revision.
- EDITME: use bulletpoints and terse descriptions.
- Link to v2: https://lore.kernel.org/r/20260523-dd-maint-2-v2-0-b937312aa083@gmail.com
---
Jim Cromie (24):
docs/dyndbg: update examples \012 to \n
docs/dyndbg: explain flags parse 1st
vmlinux.lds.h: refactor BOUNDED_SECTION_* macros into bounded_sections.lds.h
vmlinux.lds.h: drop unused HEADERED_SECTION* macros
vmlinux.lds.h: Fix ALIGN(8) omission causing NULL ptr on i386
vmlinux.lds.h: remove redundant ALIGN(8) directives
dyndbg.lds.S: fix lost dyndbg sections in modules
dyndbg: factor ddebug_match_desc out from ddebug_change
dyndbg: add stub macro for DECLARE_DYNDBG_CLASSMAP
dyndbg: reword "class unknown," to "class:_UNKNOWN_"
dyndbg-API: remove DD_CLASS_TYPE_(DISJOINT|LEVEL)_NAMES and code
dyndbg: drop NUM_TYPE_ARGS
dyndbg: reduce verbose/debug clutter
dyndbg: refactor param_set_dyndbg_classes and below
dyndbg: tighten fn-sig of ddebug_apply_class_bitmap
dyndbg: replace classmap list with an array-slice
dyndbg: macrofy a 2-index for-loop pattern
dyndbg: Upgrade class param storage to u64 for 64-bit classmaps
dyndbg,module: make proper substructs in _ddebug_info
dyndbg: move mod_name down from struct ddebug_table to _ddebug_info
dyndbg: hoist classmap-filter-by-modname up to ddebug_add_module
selftests-dyndbg: add a dynamic_debug run_tests target
dyndbg: change __dynamic_func_call_cls* macros into expressions
dynamic_debug: use KBUILD_MODFILE for unique builtin module names
Documentation/admin-guide/dynamic-debug-howto.rst | 55 ++-
MAINTAINERS | 2 +
drivers/gpu/drm/drm_print.c | 6 +-
include/asm-generic/bounded_sections.lds.h | 32 ++
include/asm-generic/dyndbg.lds.h | 18 +
include/asm-generic/vmlinux.lds.h | 68 +--
include/drm/drm_print.h | 2 +-
include/linux/dynamic_debug.h | 133 +++---
kernel/module/main.c | 12 +-
lib/dynamic_debug.c | 501 ++++++++++-----------
lib/test_dynamic_debug.c | 30 +-
scripts/module.lds.S | 2 +
tools/testing/selftests/Makefile | 1 +
tools/testing/selftests/dynamic_debug/Makefile | 9 +
tools/testing/selftests/dynamic_debug/config | 8 +
.../selftests/dynamic_debug/dyndbg_selftest.sh | 294 ++++++++++++
16 files changed, 736 insertions(+), 437 deletions(-)
---
base-commit: e43ffb69e0438cddd72aaa30898b4dc446f664f8
change-id: 20260521-dd-maint-2-76c542079420
Best regards,
--
Jim Cromie <jim.cromie@gmail.com>
^ permalink raw reply
* [PATCH v6] kernel: param: initialize module_kset in a pure_initcall
From: Shashank Balaji @ 2026-06-01 10:19 UTC (permalink / raw)
To: Gary Guo, Danilo Krummrich, Petr Pavlu
Cc: Shashank Balaji, Rahul Bukte, linux-kernel, coresight,
linux-arm-kernel, driver-core, rust-for-linux, linux-doc,
Daniel Palmer, Tim Bird, linux-modules, linux-tegra, Sumit Gupta,
Suzuki K Poulose, James Clark, Alexander Shishkin,
Greg Kroah-Hartman, Rafael J. Wysocki, Miguel Ojeda, Boqun Feng,
Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
Trevor Gross, Jonathan Corbet, Shuah Khan, Luis Chamberlain,
Daniel Gomez, Sami Tolvanen, Aaron Tomlin, Mike Leach, Leo Yan,
Thierry Reding, Jonathan Hunter
In-Reply-To: <ahEd4iC-2hqUbMy3@JPC00244420>
Commit "driver core: platform: set mod_name in driver registration" will set
struct device_driver's mod_name member for platform driver registration. For a
driver to be registered with its mod_name set, module_kset needs to be
initialized, which currently happens in a subsys_initcall in param_sysfs_init().
The tegra cbb drivers register themselves before module_kset init, in a
core_initcall. This works currently because lookup_or_create_module_kobject(),
which dereferences module_kset via kset_find_obj(), is not called if mod_name
is not set, which is the case now.
So in preparation for the commit "driver core: platform: set mod_name in driver registration",
move module_kset init to pure_initcall level, ensuring it happens before tegra
cbb driver registration.
Suggested-by: Gary Guo <gary@garyguo.net>
Reviewed-by: Gary Guo <gary@garyguo.net>
Co-developed-by: Rahul Bukte <rahul.bukte@sony.com>
Signed-off-by: Rahul Bukte <rahul.bukte@sony.com>
Signed-off-by: Shashank Balaji <shashank.mahadasyam@sony.com>
---
I'm sending v6 of just this patch to add the comment suggested by Petr and pick
up Gary's Reviewed-by. The rest of the patches are the same as v5.
Danilo, I'm assuming this series goes through driver-core. Could you please pick
up this version of this patch and the v5 of the others?
---
kernel/params.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/kernel/params.c b/kernel/params.c
index 74d620bc2521..a668863a4bb6 100644
--- a/kernel/params.c
+++ b/kernel/params.c
@@ -942,9 +942,9 @@ const struct kobj_type module_ktype = {
/*
* param_sysfs_init - create "module" kset
*
- * This must be done before the initramfs is unpacked and
- * request_module() thus becomes possible, because otherwise the
- * module load would fail in mod_sysfs_init.
+ * This must be done before any driver registration so that when a driver comes
+ * from a built-in module, the driver core can add the module under /sys/module
+ * and create the associated driver symlinks.
*/
static int __init param_sysfs_init(void)
{
@@ -957,7 +957,7 @@ static int __init param_sysfs_init(void)
return 0;
}
-subsys_initcall(param_sysfs_init);
+pure_initcall(param_sysfs_init);
/*
* param_sysfs_builtin_init - add sysfs version and parameter
--
2.43.0
^ permalink raw reply related
* Re: [PATCH v2] rust: module_param: return value by copy from `value`
From: sashiko-bot @ 2026-06-01 10:28 UTC (permalink / raw)
To: Andreas Hindborg; +Cc: linux-modules, ojeda
In-Reply-To: <20260601-modules-value-ref-v2-1-12ebbf0510c9@kernel.org>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [High] Casting a pointer of `ModuleParamAccess<T>` to `SetOnce<T>` invokes undefined behavior due to missing `#[repr(C)]` on `ModuleParamAccess`.
--
commit 9dc35dfddf4037b5b314ed7586fa775219811371
Author: Andreas Hindborg <a.hindborg@kernel.org>
rust: module_param: return value by copy from value
This commit updates parameter access to be more ergonomic for Copy types
by returning the parameter value by copy rather than through a shared
reference. It changes ModuleParamAccess::value to return T by copy when
T: Copy, and renames the previous reference-returning accessor to
value_ref. The in-tree caller in rust_minimal is updated to reflect this
change.
> diff --git a/rust/kernel/module_param.rs b/rust/kernel/module_param.rs
> index 6a8a7a875643a..a33302d1b8a36 100644
> --- a/rust/kernel/module_param.rs
> +++ b/rust/kernel/module_param.rs
[ ... ]
[Severity: High]
This isn't a bug introduced by this patch, but casting the pointer to
SetOnce<T> appears to rely on an unspecified memory layout.
In rust/kernel/module_param.rs:ModuleParamAccess:
pub struct ModuleParamAccess<T> {
value: SetOnce<T>,
default: T,
}
Since ModuleParamAccess lacks a #[repr(C)] annotation, the compiler is
free to reorder its fields. If the default field is placed before value
to minimize padding, would the cast in set_param() overwrite the default
field instead?
rust/kernel/module_param.rs:set_param() {
...
// SAFETY: By function safety requirements, this access is safe.
let container = unsafe {
&*((*param).__bindgen_anon_1.arg.cast::<SetOnce<T>>())
};
container
.populate(new_value)
...
}
Should ModuleParamAccess be marked with #[repr(C)], or should the pointer
be cast back to ModuleParamAccess<T> before accessing the value field?
> @@ -131,10 +131,26 @@ pub const fn new(default: T) -> Self {
> }
> }
>
> + /// Get a copy of the parameter value.
> + ///
[ ... ]
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260601-modules-value-ref-v2-1-12ebbf0510c9@kernel.org?part=1
^ permalink raw reply
* [PATCH v2] rust: module_param: return value by copy from `value`
From: Andreas Hindborg @ 2026-06-01 10:17 UTC (permalink / raw)
To: Luis Chamberlain, Petr Pavlu, Daniel Gomez, Sami Tolvanen,
Aaron Tomlin, Miguel Ojeda, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Alice Ryhl, Trevor Gross,
Danilo Krummrich
Cc: linux-modules, linux-kernel, rust-for-linux, Andreas Hindborg
For `Copy` parameter types it is more ergonomic to retrieve the
parameter value by copy than through a shared reference. Change
`ModuleParamAccess::value` to return `T` by copy when `T: Copy`,
and rename the previous reference-returning accessor to
`value_ref`. Update the in-tree caller in `rust_minimal`.
Suggested-by: Alice Ryhl <aliceryhl@google.com>
Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
---
This change was suggested during review of the rust null block series [1].
[1] https://lore.kernel.org/r/abfe2LbhLzXiGCkA@google.com
---
Changes in v2:
- Move `value` into existing impl block (Gary).
- Add inline attributes (Sashiko).
- Link to v1: https://msgid.link/20260529-modules-value-ref-v1-1-a0df7e9e6f2d@kernel.org
---
rust/kernel/module_param.rs | 18 +++++++++++++++++-
samples/rust/rust_minimal.rs | 2 +-
2 files changed, 18 insertions(+), 2 deletions(-)
diff --git a/rust/kernel/module_param.rs b/rust/kernel/module_param.rs
index 6a8a7a875643..a33302d1b8a3 100644
--- a/rust/kernel/module_param.rs
+++ b/rust/kernel/module_param.rs
@@ -131,10 +131,26 @@ pub const fn new(default: T) -> Self {
}
}
+ /// Get a copy of the parameter value.
+ ///
+ /// Returns the value supplied at module load time, or the default value
+ /// if the parameter has not been set.
+ #[inline]
+ pub fn value(&self) -> T
+ where
+ T: Copy,
+ {
+ self.value.copy().unwrap_or(self.default)
+ }
+
/// Get a shared reference to the parameter value.
+ ///
+ /// Returns a reference to the value supplied at module load time, or a
+ /// reference to the default value if the parameter has not been set.
// Note: When sysfs access to parameters are enabled, we have to pass in a
// held lock guard here.
- pub fn value(&self) -> &T {
+ #[inline]
+ pub fn value_ref(&self) -> &T {
self.value.as_ref().unwrap_or(&self.default)
}
diff --git a/samples/rust/rust_minimal.rs b/samples/rust/rust_minimal.rs
index 8eb9583571d7..60d03df6cd80 100644
--- a/samples/rust/rust_minimal.rs
+++ b/samples/rust/rust_minimal.rs
@@ -28,7 +28,7 @@ fn init(_module: &'static ThisModule) -> Result<Self> {
pr_info!("Am I built-in? {}\n", !cfg!(MODULE));
pr_info!(
"test_parameter: {}\n",
- *module_parameters::test_parameter.value()
+ module_parameters::test_parameter.value()
);
let mut numbers = KVec::new();
---
base-commit: 7fd2df204f342fc17d1a0bfcd474b24232fb0f32
change-id: 20260529-modules-value-ref-e95a7ab94fdb
Best regards,
--
Andreas Hindborg <a.hindborg@kernel.org>
^ permalink raw reply related
* Re: [PATCH] rust: module_param: return value by copy from `value`
From: Andreas Hindborg @ 2026-06-01 10:10 UTC (permalink / raw)
To: Gary Guo, Luis Chamberlain, Petr Pavlu, Daniel Gomez,
Sami Tolvanen, Aaron Tomlin, Miguel Ojeda, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Alice Ryhl, Trevor Gross,
Danilo Krummrich
Cc: linux-modules, linux-kernel, rust-for-linux
In-Reply-To: <DIV5NRUFAGLY.1RH2GL15CYQCG@garyguo.net>
"Gary Guo" <gary@garyguo.net> writes:
> On Fri May 29, 2026 at 9:41 AM BST, Andreas Hindborg wrote:
>> For `Copy` parameter types it is more ergonomic to retrieve the
>> parameter value by copy than through a shared reference. Change
>> `ModuleParamAccess::value` to return `T` by copy when `T: Copy`,
>> and rename the previous reference-returning accessor to
>> `value_ref`. Update the in-tree caller in `rust_minimal`.
>>
>> Suggested-by: Alice Ryhl <aliceryhl@google.com>
>> Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
>> ---
>> This change was suggested during review of the rust null block series [1].
>>
>> [1] https://lore.kernel.org/r/abfe2LbhLzXiGCkA@google.com
>> ---
>> rust/kernel/module_param.rs | 15 ++++++++++++++-
>> samples/rust/rust_minimal.rs | 2 +-
>> 2 files changed, 15 insertions(+), 2 deletions(-)
>>
>> diff --git a/rust/kernel/module_param.rs b/rust/kernel/module_param.rs
>> index 6a8a7a875643..701210206bba 100644
>> --- a/rust/kernel/module_param.rs
>> +++ b/rust/kernel/module_param.rs
>> @@ -132,9 +132,12 @@ pub const fn new(default: T) -> Self {
>> }
>>
>> /// Get a shared reference to the parameter value.
>> + ///
>> + /// Returns a reference to the value supplied at module load time, or a
>> + /// reference to the default value if the parameter has not been set.
>> // Note: When sysfs access to parameters are enabled, we have to pass in a
>> // held lock guard here.
>> - pub fn value(&self) -> &T {
>> + pub fn value_ref(&self) -> &T {
>> self.value.as_ref().unwrap_or(&self.default)
>> }
>>
>> @@ -146,6 +149,16 @@ pub const fn as_void_ptr(&self) -> *mut c_void {
>> }
>> }
>>
>> +impl<T: Copy> ModuleParamAccess<T> {
>
> Instead of adding a new impl block, this should just be moved to immediately
> after (or before) `value_ref` with a `where T: Copy` bound instead.
>
> This makes related code close together for readers of the code.
Will do.
Best regards,
Andreas Hindborg
^ permalink raw reply
* Re: [PATCH v3] ARM: module.lds: fix unwind metadata for merged .text sections
From: sashiko-bot @ 2026-05-30 15:56 UTC (permalink / raw)
To: Egg12138; +Cc: linux-modules
In-Reply-To: <tencent_08845B64E5F38EB7FA1779982A071AB4A607@qq.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [High] Missing NULL check on the return value of `unwind_table_add()` in `module_finalize()` causes a kernel panic on memory allocation failure.
--
commit 0098f68f3e89e8e569da9c5eaaa9486505d4ea3d
Author: Xiao Junzhe <egg12138@foxmail.com>
ARM: module.lds: fix unwind metadata for merged .text sections
Commit 1ba9f8979426 ("vmlinux.lds: Unify TEXT_MAIN, DATA_MAIN,
and related macros") made scripts/module.lds.S merge module input
.text.* sections into the output .text section. On ARM, the paired
unwind input sections keep their original names. This commit ensures
the ARM module unwind output names stay in sync with the text sections.
> diff --git a/arch/arm/include/asm/module.lds.h b/arch/arm/include/asm/module.lds.h
> index 0e7cb4e314b48..1d56c0375c18b 100644
> --- a/arch/arm/include/asm/module.lds.h
> +++ b/arch/arm/include/asm/module.lds.h
> @@ -1,4 +1,16 @@
> /* SPDX-License-Identifier: GPL-2.0 */
> +
> +#ifdef CONFIG_ARM_UNWIND
> +SECTIONS {
> + .ARM.extab 0 : {
> + *(.ARM.extab .ARM.extab.text .ARM.extab.text.[0-9a-zA-Z_]*)
> + }
> + .ARM.exidx 0 : {
> + *(.ARM.exidx .ARM.exidx.text .ARM.exidx.text.[0-9a-zA-Z_]*)
> + }
> +}
> +#endif
[Severity: High]
This is a pre-existing issue and was not introduced by the patch under review,
but while reviewing how ARM module unwind tables are registered, I noticed a
potential concern in arch/arm/kernel/module.c:
arch/arm/kernel/module.c:module_finalize() {
...
if (txt_sec) {
struct unwind_table *table =
unwind_table_add(s->sh_addr,
s->sh_size,
txt_sec->sh_addr,
txt_sec->sh_size);
list_add(&table->mod_list, unwind_list);
...
}
Does this code safely handle a null return from unwind_table_add?
If unwind_table_add fails to allocate memory, it returns a null pointer.
Passing that directly to list_add causes an offset-based dereference of null.
Could this trigger a kernel panic during module loading if the system is out
of memory?
> +
> #ifdef CONFIG_ARM_MODULE_PLTS
> SECTIONS {
> .plt : { BYTE(0) }
--
Sashiko AI review · https://sashiko.dev/#/patchset/tencent_08845B64E5F38EB7FA1779982A071AB4A607@qq.com?part=1
^ permalink raw reply
* [PATCH v3] ARM: module.lds: fix unwind metadata for merged .text sections
From: Egg12138 @ 2026-05-30 15:46 UTC (permalink / raw)
To: Russell King
Cc: Josh Poimboeuf, Petr Mladek, linux-arm-kernel, linux-modules,
linux-kernel, Xiao Junzhe
From: Xiao Junzhe <egg12138@foxmail.com>
Commit 1ba9f8979426 ("vmlinux.lds: Unify TEXT_MAIN, DATA_MAIN,
and related macros") made scripts/module.lds.S merge module input
.text.* sections into the output .text section.
On ARM, the paired unwind input sections keep their original names. A
module can therefore contain .ARM.exidx.text.unlikely with sh_link
pointing at .text, while .text.unlikely no longer exists.
This is a valid ELF relationship, but ARM module_finalize() does not use
sh_link when registering module unwind tables. It derives the target
text section from the exidx section name instead:
.ARM.exidx.text.unlikely -> .text.unlikely
The lookup fails and the unwind table is not registered for the actual
.text range. This can make module stack unwinding fail with:
unwind: Index not found
Keep the ARM module unwind output names in sync with the text sections
that scripts/module.lds.S now produces. Coalesce the .ARM.exidx/.ARM.extab
section associated with .text.*, into the
stable output names expected by the existing ARM module unwind code.
Fixes: 1ba9f8979426 ("vmlinux.lds: Unify TEXT_MAIN, DATA_MAIN, and related macros")
Signed-off-by: Xiao Junzhe <egg12138@foxmail.com>
---
Changes in v3:
- Apply the ARM unwind section coalescing under CONFIG_ARM_UNWIND.
- Remove quotes around linker-script wildcard patterns so GNU ld expands
them correctly.
- Use conventional linker-script wildcard syntax, i.e. *(...) without a
space after the wildcard.
arch/arm/include/asm/module.lds.h | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/arch/arm/include/asm/module.lds.h b/arch/arm/include/asm/module.lds.h
index 0e7cb4e314b4..1d56c0375c18 100644
--- a/arch/arm/include/asm/module.lds.h
+++ b/arch/arm/include/asm/module.lds.h
@@ -1,4 +1,16 @@
/* SPDX-License-Identifier: GPL-2.0 */
+
+#ifdef CONFIG_ARM_UNWIND
+SECTIONS {
+ .ARM.extab 0 : {
+ *(.ARM.extab .ARM.extab.text .ARM.extab.text.[0-9a-zA-Z_]*)
+ }
+ .ARM.exidx 0 : {
+ *(.ARM.exidx .ARM.exidx.text .ARM.exidx.text.[0-9a-zA-Z_]*)
+ }
+}
+#endif
+
#ifdef CONFIG_ARM_MODULE_PLTS
SECTIONS {
.plt : { BYTE(0) }
--
2.43.0
^ permalink raw reply related
* Re: [PATCH 11/11] moduleparam: Drop legacy kernel_param_ops .get_str field and dispatch logic
From: sashiko-bot @ 2026-05-30 0:46 UTC (permalink / raw)
To: Kees Cook
Cc: dmaengine, vkoul, kvm, linux-modules, linux-pci, Frank.Li,
linux-media
In-Reply-To: <20260521133326.2465264-11-kees@kernel.org>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [High] Unconditional call to `arr->ops->get()` without a NULL check can cause a kernel panic if the array parameter element lacks a `.get` callback.
--
commit a252ae87a187a5b644d160fded352c545493a611
Author: Kees Cook <kees@kernel.org>
moduleparam: Drop legacy kernel_param_ops .get_str field and dispatch logic
All struct kernel_param_ops .get callbacks have been migrated to using
struct seq_buf. Drop the migration scaffolding.
> diff --git a/kernel/params.c b/kernel/params.c
> index 25f0c8d5d19f3..6b410189297b1 100644
> --- a/kernel/params.c
> +++ b/kernel/params.c
[ ... ]
> @@ -470,23 +469,9 @@ static int param_array_get(struct seq_buf *s, const struct kernel_param *kp)
> p.arg = arr->elem + arr->elemsize * i;
> check_kparam_locked(p.mod);
>
> - if (arr->ops->get) {
> - ret = arr->ops->get(s, &p);
> - if (ret < 0)
> - goto out;
> - } else {
[ ... ]
> - }
> + ret = arr->ops->get(s, &p);
[Severity: High]
This isn't a bug introduced by this patch, but does this unconditional call to
arr->ops->get() need a NULL check to avoid a kernel panic?
If a module defines a write-only array parameter where the element's
kernel_param_ops struct lacks a .get callback, and assigns it write-only sysfs
permissions, root can still attempt to read the file. When this happens, sysfs
invokes param_attr_show().
For normal parameters, param_attr_show() has a safety check that rejects the
read if ops->get is NULL. However, for array parameters, ops resolves to
param_array_ops which has a valid .get callback, bypassing the safeguard.
Could this result in a NULL pointer dereference when arr->ops->get(s, &p) is
executed? Before this patch, the code fell back to calling .get_str(), which
would also be NULL for a write-only type, resulting in the same kernel panic.
> + if (ret < 0)
> + return ret;
>
> /* Nothing got written (e.g. overflow) — stop. */
> if (s->len == before)
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260521133315.work.845-kees@kernel.org?part=11
^ permalink raw reply
* Re: [PATCH 08/11] params: Convert generic kernel_param_ops .get helpers to seq_buf
From: sashiko-bot @ 2026-05-30 0:45 UTC (permalink / raw)
To: Kees Cook
Cc: vkoul, dmaengine, linux-modules, linux-media, kvm, Frank.Li,
linux-pci
In-Reply-To: <20260521133326.2465264-8-kees@kernel.org>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Information Leak / Uninitialized Memory Disclosure in param_array_get()
--
commit 4bb10449d07e3a1b177001d61d53053c25dd4767
Author: Kees Cook <kees@kernel.org>
params: Convert generic kernel_param_ops .get helpers to seq_buf
This commit converts the generic kernel parameter .get helpers in
kernel/params.c to use the seq_buf signature instead of char pointers. It
allows param_array_get to write element output directly into the parent
seq_buf when supported, updating various dependent callers to match the new
signature.
> diff --git a/kernel/params.c b/kernel/params.c
> index 4eda2d23ddf28..25f0c8d5d19f3 100644
> --- a/kernel/params.c
> +++ b/kernel/params.c
[ ... ]
> @@ -453,36 +457,46 @@ static int param_array_set(const char *val, const struct kernel_param *kp)
[ ... ]
> -static int param_array_get(char *buffer, const struct kernel_param *kp)
> +static int param_array_get(struct seq_buf *s, const struct kernel_param *kp)
> {
[ ... ]
> - for (i = off = 0; i < (arr->num ? *arr->num : arr->max); i++) {
> + for (i = 0; i < (arr->num ? *arr->num : arr->max); i++) {
> + size_t before = s->len;
>
> p.arg = arr->elem + arr->elemsize * i;
> check_kparam_locked(p.mod);
> - ret = arr->ops->get_str(elem_buf, &p);
> - if (ret < 0)
> - goto out;
> - ret = min(ret, (int)(PAGE_SIZE - 1 - off));
> - if (!ret)
> +
> + if (arr->ops->get) {
> + ret = arr->ops->get(s, &p);
> + if (ret < 0)
> + goto out;
> + } else {
[ ... ]
> + ret = arr->ops->get_str(elem_buf, &p);
> + if (ret < 0)
> + goto out;
> + seq_buf_putmem(s, elem_buf, ret);
> + }
> +
> + /* Nothing got written (e.g. overflow) — stop. */
> + if (s->len == before)
> break;
[Severity: High]
When a seq_buf overflows, it calls seq_buf_set_overflow() which sets
s->len = s->size. Because of this, does s->len actually equal before
when an overflow occurs?
If s->len becomes s->size, the condition s->len == before will evaluate
to false, causing the loop to continue for one more iteration rather than
breaking. On the next iteration, before will be s->size, the write will
fail again, and the loop will finally break.
Because the overflow mechanism sets the length to the buffer size without
actually writing data into the remaining space, could this leak uninitialized
kernel heap memory to userspace when sysfs returns the buffer?
> +
> + /* Replace the previous element's trailing newline with a comma. */
> - if (i)
> - buffer[off - 1] = ',';
> - memcpy(buffer + off, elem_buf, ret);
> - off += ret;
> - if (off == PAGE_SIZE - 1)
> - break;
> + if (i && s->buffer[before - 1] == '\n')
> + s->buffer[before - 1] = ',';
> }
> - buffer[off] = '\0';
> - ret = off;
> + ret = 0;
> out:
> kfree(elem_buf);
> return ret;
> }
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260521133315.work.845-kees@kernel.org?part=8
^ permalink raw reply
* Re: [PATCH 05/11] moduleparam: Rename .get field to .get_str
From: sashiko-bot @ 2026-05-30 0:45 UTC (permalink / raw)
To: Kees Cook
Cc: vkoul, dmaengine, linux-modules, Frank.Li, linux-pci, kvm,
linux-media
In-Reply-To: <20260521133326.2465264-5-kees@kernel.org>
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Low] The commit message claims that all `kernel_param_ops` initializers in the tree now use the `DEFINE_KERNEL_PARAM_OPS` macro, but several files (e.g., `arch/xtensa/platforms/iss/simdisk.c`, `drivers/video/fbdev/uvesafb.c`, `drivers/mtd/ubi/block.c`, `drivers/target/target_core_user.c`) still use direct struct initialization.
- [Low] Renaming the `.get` field to `.get_str` in `struct kernel_param_ops` breaks the kernel build when Rust is enabled (`CONFIG_RUST=y`).
--
commit 1952ed67c747da9c1d8185a27ee2651d4cdb0689
Author: Kees Cook <kees@kernel.org>
moduleparam: Rename .get field to .get_str
This commit renames the .get callback in struct kernel_param_ops to
.get_str. It frees up the .get name for a future commit that will re-add
it as a seq_buf-based callback.
> Now that every kernel_param_ops initializer in the tree goes through
> DEFINE_KERNEL_PARAM_OPS, no source file outside kernel/params.c and
[Severity: Low]
This isn't a bug, but is this description entirely accurate?
Several files like arch/xtensa/platforms/iss/simdisk.c,
drivers/video/fbdev/uvesafb.c, and drivers/mtd/ubi/block.c still use direct
struct initialization because they only define .set.
> include/linux/moduleparam.h references the .get field by name. Take
> advantage of that to rename the field to .get_str.
[ ... ]
> diff --git a/include/linux/moduleparam.h b/include/linux/moduleparam.h
> index 26bf45b36d023..f5f4148e25049 100644
> --- a/include/linux/moduleparam.h
> +++ b/include/linux/moduleparam.h
> @@ -63,7 +63,7 @@ struct kernel_param_ops {
> /* Returns 0, or -errno. arg is in kp->arg. */
> int (*set)(const char *val, const struct kernel_param *kp);
> /* Returns length written or -errno. Buffer is 4k (ie. be short!) */
> - int (*get)(char *buffer, const struct kernel_param *kp);
> + int (*get_str)(char *buffer, const struct kernel_param *kp);
[Severity: Low]
Does this break the kernel build when CONFIG_RUST=y?
In rust/kernel/module_param.rs, the make_param_ops! macro explicitly
initializes the generated C struct with get: None. When bindgen reflects
the renamed field, compilation will fail because the get field no longer
exists and the new get_str field is uninitialized.
Even though this is resolved later in the series by removing the get_str
field entirely, does this intermediate breakage cause issues for git bisect?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260521133315.work.845-kees@kernel.org?part=5
^ permalink raw reply
* Re: [PATCH 06/11] moduleparam: Add seq_buf-based .get callback alongside .get_str
From: sashiko-bot @ 2026-05-30 0:45 UTC (permalink / raw)
To: Kees Cook
Cc: vkoul, Frank.Li, dmaengine, linux-media, linux-modules, linux-pci,
kvm
In-Reply-To: <20260521133326.2465264-6-kees@kernel.org>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Silent data truncation and spurious kernel warning when the sysfs buffer is exactly filled.
--
commit 6802395b5f9f236d2cc29365e6d39e6de2eae4ca
Author: Kees Cook <kees@kernel.org>
moduleparam: Add seq_buf-based .get callback alongside .get_str
This commit adds a new seq_buf-based .get callback to struct kernel_param_ops
to format parameter values safely. It updates param_attr_show() to prefer
this new callback when available, using a page-sized buffer. The legacy
.get_str callback is retained as a fallback.
> diff --git a/kernel/params.c b/kernel/params.c
> index 6852caea17854..4eda2d23ddf28 100644
> --- a/kernel/params.c
> +++ b/kernel/params.c
[ ... ]
> @@ -553,12 +553,34 @@ static ssize_t param_attr_show(const struct module_attribute *mattr,
[ ... ]
> kernel_param_lock(mk->mod);
> - count = attribute->param->ops->get_str(buf, attribute->param);
> + if (ops->get) {
> + struct seq_buf s;
> +
> + seq_buf_init(&s, buf, PAGE_SIZE);
> + count = ops->get(&s, attribute->param);
> + if (count >= 0) {
> + WARN_ON_ONCE(count > 0);
> + count = seq_buf_used(&s);
> + /* Make sure string is terminated. */
> + seq_buf_str(&s);
> + /*
> + * If overflowed, reduce count by 1 for trailing
> + * NUL byte.
> + */
> + if (seq_buf_has_overflowed(&s))
> + count--;
[Severity: Medium]
Does this code silently truncate data and trigger a kernel warning when the
buffer is exactly filled?
If the .get() callback writes exactly PAGE_SIZE bytes without triggering an
overflow, seq_buf_has_overflowed(&s) will evaluate to false because s.len
exactly equals s.size. In this case, count remains PAGE_SIZE.
However, seq_buf_str(&s) unconditionally writes a NUL byte at the end of the
buffer, which will replace the last byte of valid data at PAGE_SIZE - 1.
Furthermore, returning PAGE_SIZE causes the sysfs core to print a bad count
warning to dmesg and truncate the count anyway. Should this code explicitly
decrement count when count == PAGE_SIZE to avoid overwriting data and causing
the warning?
> + }
> + } else {
> + count = ops->get_str(buf, attribute->param);
> + }
> kernel_param_unlock(mk->mod);
> return count;
> }
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260521133315.work.845-kees@kernel.org?part=6
^ permalink raw reply
* Re: [PATCH v2] ARM: module.lds: fix unwind metadata for merged .text sections
From: sashiko-bot @ 2026-05-29 17:12 UTC (permalink / raw)
To: Egg12138; +Cc: linux-modules
In-Reply-To: <tencent_D9CDCA221D0AF5EDF260559734464D39B708@qq.com>
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] The linker script fix is wrapped in an inverted configuration conditional (`#ifndef CONFIG_ARM_UNWIND`), applying the fix only when unwinding is disabled.
- [High] The wildcard patterns are enclosed in double quotes, which disables wildcard matching in GNU ld.
- [Low] Non-standard linker script syntax with a space between the wildcard asterisk and the section name list.
--
commit 165574ad9cf92e77eac788d811caa72c990e0b3d
Author: Xiao Junzhe <egg12138@foxmail.com>
Subject: ARM: module.lds: fix unwind metadata for merged .text sections
This commit attempts to fix a module stack unwinding failure on ARM by
coalescing .ARM.exidx and .ARM.extab input sections that belong to merged
.text.* sections. This is intended to keep the output names in sync with
what the ARM module unwind code expects.
> diff --git a/arch/arm/include/asm/module.lds.h b/arch/arm/include/asm/module.lds.h
> --- a/arch/arm/include/asm/module.lds.h
> +++ b/arch/arm/include/asm/module.lds.h
> @@ -1,4 +1,20 @@
> /* SPDX-License-Identifier: GPL-2.0 */
> +
> +#ifndef CONFIG_ARM_UNWIND
[Severity: High]
Does this exclude the fix in the exact configuration it is intended for?
The patch aims to fix module stack unwinding, which is controlled by
CONFIG_ARM_UNWIND. Because this block is wrapped in an #ifndef, the section
coalescing will be skipped when unwinding is enabled, leaving the unwind
tables unmerged and the issue unresolved.
> +SECTIONS {
> + .ARM.extab 0 : {
> + *(.ARM.extab)
> + * (".ARM.extab.text")
> + * (".ARM.extab.text.[0-9a-zA-Z_]*")
[Severity: Low]
This isn't a bug, but there is a space between the asterisk wildcard and
the section name list. While GNU ld ignores this whitespace, standard kernel
linker script conventions typically omit it.
[Severity: High]
Will GNU ld expand the wildcard patterns when they are enclosed in double
quotes?
Enclosing a section name in double quotes causes wildcard characters like
the asterisk and brackets to lose their special meaning and be treated as
literal characters. The linker will try to match a section literally named
".ARM.extab.text.[0-9a-zA-Z_]*" instead of expanding the glob to match
sections like ".ARM.extab.text.unlikely".
> + }
> + .ARM.exidx 0 : {
> + *(.ARM.exidx)
> + * (".ARM.exidx.text")
> + * (".ARM.exidx.text.[0-9a-zA-Z_]*")
> + }
> +}
> +#endif
--
Sashiko AI review · https://sashiko.dev/#/patchset/tencent_D9CDCA221D0AF5EDF260559734464D39B708@qq.com?part=1
^ permalink raw reply
* [PATCH v2] ARM: module.lds: fix unwind metadata for merged .text sections
From: Egg12138 @ 2026-05-29 16:46 UTC (permalink / raw)
To: Russell King
Cc: Josh Poimboeuf, Petr Mladek, linux-arm-kernel, linux-modules,
linux-kernel, Xiao Junzhe
In-Reply-To: <tencent_69DC6E2A5B77E4D567CA4F552DD3A835590A@qq.com>
From: Xiao Junzhe <egg12138@foxmail.com>
Commit 1ba9f8979426 ("vmlinux.lds: Unify TEXT_MAIN, DATA_MAIN,
and related macros") made scripts/module.lds.S merge module input
.text.* sections into the output .text section.
On ARM, the paired unwind input sections keep their original names. A
module can therefore contain .ARM.exidx.text.unlikely with sh_link
pointing at .text, while .text.unlikely no longer exists.
This is a valid ELF relationship, but ARM module_finalize() does not use
sh_link when registering module unwind tables. It derives the target
text section from the exidx section name instead:
.ARM.exidx.text.unlikely -> .text.unlikely
The lookup fails and the unwind table is not registered for the actual
.text range. This can make module stack unwinding fail with:
unwind: Index not found
Keep the ARM module unwind output names in sync with the text sections
that scripts/module.lds.S now produces. Coalesce the .ARM.exidx/.ARM.extab
section associated with .text.*, into the
stable output names expected by the existing ARM module unwind code.
Fixes: 1ba9f8979426 ("vmlinux.lds: Unify TEXT_MAIN, DATA_MAIN, and related macros")
Signed-off-by: Xiao Junzhe <egg12138@foxmail.com>
---
Changes in v2:
- Quote ARM unwind input section patterns and split them across separate
linker-script entries to avoid checkpatch false positives on wildcard
syntax.
- No functional change: the script still folds .ARM.extab.text.* and
.ARM.exidx.text.* into the stable .ARM.extab/.ARM.exidx output sections.
arch/arm/include/asm/module.lds.h | 16 ++++++++++++++++
1 file changed, 16 insertions(+)
diff --git a/arch/arm/include/asm/module.lds.h b/arch/arm/include/asm/module.lds.h
index 0e7cb4e314b4..19ceb8cf235f 100644
--- a/arch/arm/include/asm/module.lds.h
+++ b/arch/arm/include/asm/module.lds.h
@@ -1,4 +1,20 @@
/* SPDX-License-Identifier: GPL-2.0 */
+
+#ifndef CONFIG_ARM_UNWIND
+SECTIONS {
+ .ARM.extab 0 : {
+ *(.ARM.extab)
+ * (".ARM.extab.text")
+ * (".ARM.extab.text.[0-9a-zA-Z_]*")
+ }
+ .ARM.exidx 0 : {
+ *(.ARM.exidx)
+ * (".ARM.exidx.text")
+ * (".ARM.exidx.text.[0-9a-zA-Z_]*")
+ }
+}
+#endif
+
#ifdef CONFIG_ARM_MODULE_PLTS
SECTIONS {
.plt : { BYTE(0) }
--
2.43.0
^ permalink raw reply related
* Re: [PATCH] rust: module_param: return value by copy from `value`
From: Gary Guo @ 2026-05-29 12:22 UTC (permalink / raw)
To: Andreas Hindborg, Luis Chamberlain, Petr Pavlu, Daniel Gomez,
Sami Tolvanen, Aaron Tomlin, Miguel Ojeda, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Alice Ryhl, Trevor Gross,
Danilo Krummrich
Cc: linux-modules, linux-kernel, rust-for-linux
In-Reply-To: <20260529-modules-value-ref-v1-1-a0df7e9e6f2d@kernel.org>
On Fri May 29, 2026 at 9:41 AM BST, Andreas Hindborg wrote:
> For `Copy` parameter types it is more ergonomic to retrieve the
> parameter value by copy than through a shared reference. Change
> `ModuleParamAccess::value` to return `T` by copy when `T: Copy`,
> and rename the previous reference-returning accessor to
> `value_ref`. Update the in-tree caller in `rust_minimal`.
>
> Suggested-by: Alice Ryhl <aliceryhl@google.com>
> Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
> ---
> This change was suggested during review of the rust null block series [1].
>
> [1] https://lore.kernel.org/r/abfe2LbhLzXiGCkA@google.com
> ---
> rust/kernel/module_param.rs | 15 ++++++++++++++-
> samples/rust/rust_minimal.rs | 2 +-
> 2 files changed, 15 insertions(+), 2 deletions(-)
>
> diff --git a/rust/kernel/module_param.rs b/rust/kernel/module_param.rs
> index 6a8a7a875643..701210206bba 100644
> --- a/rust/kernel/module_param.rs
> +++ b/rust/kernel/module_param.rs
> @@ -132,9 +132,12 @@ pub const fn new(default: T) -> Self {
> }
>
> /// Get a shared reference to the parameter value.
> + ///
> + /// Returns a reference to the value supplied at module load time, or a
> + /// reference to the default value if the parameter has not been set.
> // Note: When sysfs access to parameters are enabled, we have to pass in a
> // held lock guard here.
> - pub fn value(&self) -> &T {
> + pub fn value_ref(&self) -> &T {
> self.value.as_ref().unwrap_or(&self.default)
> }
>
> @@ -146,6 +149,16 @@ pub const fn as_void_ptr(&self) -> *mut c_void {
> }
> }
>
> +impl<T: Copy> ModuleParamAccess<T> {
Instead of adding a new impl block, this should just be moved to immediately
after (or before) `value_ref` with a `where T: Copy` bound instead.
This makes related code close together for readers of the code.
Thanks,
Gary
> + /// Get a copy of the parameter value.
> + ///
> + /// Returns the value supplied at module load time, or the default value
> + /// if the parameter has not been set.
> + pub fn value(&self) -> T {
> + self.value.copy().unwrap_or(self.default)
> + }
> +}
> +
> #[doc(hidden)]
> /// Generate a static [`kernel_param_ops`](srctree/include/linux/moduleparam.h) struct.
> ///
> diff --git a/samples/rust/rust_minimal.rs b/samples/rust/rust_minimal.rs
> index 8eb9583571d7..60d03df6cd80 100644
> --- a/samples/rust/rust_minimal.rs
> +++ b/samples/rust/rust_minimal.rs
> @@ -28,7 +28,7 @@ fn init(_module: &'static ThisModule) -> Result<Self> {
> pr_info!("Am I built-in? {}\n", !cfg!(MODULE));
> pr_info!(
> "test_parameter: {}\n",
> - *module_parameters::test_parameter.value()
> + module_parameters::test_parameter.value()
> );
>
> let mut numbers = KVec::new();
>
> ---
> base-commit: 7fd2df204f342fc17d1a0bfcd474b24232fb0f32
> change-id: 20260529-modules-value-ref-e95a7ab94fdb
>
> Best regards,
^ permalink raw reply
* Re: [PATCH] rust: module_param: return value by copy from `value`
From: sashiko-bot @ 2026-05-29 8:55 UTC (permalink / raw)
To: Andreas Hindborg; +Cc: linux-modules, ojeda
In-Reply-To: <20260529-modules-value-ref-v1-1-a0df7e9e6f2d@kernel.org>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] Missing `#[inline]` annotations on small abstraction methods `value` and `value_ref` in `ModuleParamAccess`.
--
commit 6103e7f86e2528da9d35cfda1b1a7d24ab1f6516
Author: Andreas Hindborg <a.hindborg@kernel.org>
rust: module_param: return value by copy from value
For Copy parameter types, this patch changes ModuleParamAccess::value
to return by copy instead of by reference, improving ergonomics. It also
renames the reference-returning accessor to value_ref and updates the
caller in rust_minimal.
> diff --git a/rust/kernel/module_param.rs b/rust/kernel/module_param.rs
> index 6a8a7a875643a..701210206bba5 100644
> --- a/rust/kernel/module_param.rs
> +++ b/rust/kernel/module_param.rs
> @@ -132,9 +132,12 @@ pub const fn new(default: T) -> Self {
> }
>
> /// Get a shared reference to the parameter value.
> + ///
> + /// Returns a reference to the value supplied at module load time, or a
> + /// reference to the default value if the parameter has not been set.
> // Note: When sysfs access to parameters are enabled, we have to pass in a
> // held lock guard here.
> - pub fn value(&self) -> &T {
> + pub fn value_ref(&self) -> &T {
[Severity: Low]
This isn't a bug, but according to the Rust subsystem coding guidelines,
functions in abstractions that are small or forward to binding calls should
be annotated with #[inline]. Should value_ref() have an #[inline] annotation
added here?
> self.value.as_ref().unwrap_or(&self.default)
> }
[ ... ]
> @@ -146,6 +149,16 @@ pub const fn as_void_ptr(&self) -> *mut c_void {
> }
> }
>
> +impl<T: Copy> ModuleParamAccess<T> {
> + /// Get a copy of the parameter value.
> + ///
> + /// Returns the value supplied at module load time, or the default value
> + /// if the parameter has not been set.
> + pub fn value(&self) -> T {
[Severity: Low]
This isn't a bug, but similar to the above, should this small abstraction
method also be marked with #[inline] to allow compiler optimizations
across crate boundaries?
> + self.value.copy().unwrap_or(self.default)
> + }
> +}
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260529-modules-value-ref-v1-1-a0df7e9e6f2d@kernel.org?part=1
^ permalink raw reply
* [PATCH] rust: module_param: return value by copy from `value`
From: Andreas Hindborg @ 2026-05-29 8:41 UTC (permalink / raw)
To: Luis Chamberlain, Petr Pavlu, Daniel Gomez, Sami Tolvanen,
Aaron Tomlin, Miguel Ojeda, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Alice Ryhl, Trevor Gross,
Danilo Krummrich
Cc: linux-modules, linux-kernel, rust-for-linux, Andreas Hindborg
For `Copy` parameter types it is more ergonomic to retrieve the
parameter value by copy than through a shared reference. Change
`ModuleParamAccess::value` to return `T` by copy when `T: Copy`,
and rename the previous reference-returning accessor to
`value_ref`. Update the in-tree caller in `rust_minimal`.
Suggested-by: Alice Ryhl <aliceryhl@google.com>
Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
---
This change was suggested during review of the rust null block series [1].
[1] https://lore.kernel.org/r/abfe2LbhLzXiGCkA@google.com
---
rust/kernel/module_param.rs | 15 ++++++++++++++-
samples/rust/rust_minimal.rs | 2 +-
2 files changed, 15 insertions(+), 2 deletions(-)
diff --git a/rust/kernel/module_param.rs b/rust/kernel/module_param.rs
index 6a8a7a875643..701210206bba 100644
--- a/rust/kernel/module_param.rs
+++ b/rust/kernel/module_param.rs
@@ -132,9 +132,12 @@ pub const fn new(default: T) -> Self {
}
/// Get a shared reference to the parameter value.
+ ///
+ /// Returns a reference to the value supplied at module load time, or a
+ /// reference to the default value if the parameter has not been set.
// Note: When sysfs access to parameters are enabled, we have to pass in a
// held lock guard here.
- pub fn value(&self) -> &T {
+ pub fn value_ref(&self) -> &T {
self.value.as_ref().unwrap_or(&self.default)
}
@@ -146,6 +149,16 @@ pub const fn as_void_ptr(&self) -> *mut c_void {
}
}
+impl<T: Copy> ModuleParamAccess<T> {
+ /// Get a copy of the parameter value.
+ ///
+ /// Returns the value supplied at module load time, or the default value
+ /// if the parameter has not been set.
+ pub fn value(&self) -> T {
+ self.value.copy().unwrap_or(self.default)
+ }
+}
+
#[doc(hidden)]
/// Generate a static [`kernel_param_ops`](srctree/include/linux/moduleparam.h) struct.
///
diff --git a/samples/rust/rust_minimal.rs b/samples/rust/rust_minimal.rs
index 8eb9583571d7..60d03df6cd80 100644
--- a/samples/rust/rust_minimal.rs
+++ b/samples/rust/rust_minimal.rs
@@ -28,7 +28,7 @@ fn init(_module: &'static ThisModule) -> Result<Self> {
pr_info!("Am I built-in? {}\n", !cfg!(MODULE));
pr_info!(
"test_parameter: {}\n",
- *module_parameters::test_parameter.value()
+ module_parameters::test_parameter.value()
);
let mut numbers = KVec::new();
---
base-commit: 7fd2df204f342fc17d1a0bfcd474b24232fb0f32
change-id: 20260529-modules-value-ref-e95a7ab94fdb
Best regards,
--
Andreas Hindborg <a.hindborg@kernel.org>
^ permalink raw reply related
* Re: [PATCH] rust: module_param: add missing newline to pr_warn
From: Miguel Ojeda @ 2026-05-29 5:15 UTC (permalink / raw)
To: Kenny Glowner
Cc: Sami Tolvanen, Luis Chamberlain, Petr Pavlu, Daniel Gomez,
Miguel Ojeda, linux-modules, rust-for-linux, linux-kernel
In-Reply-To: <CANN+23v9vkzq_2o01z2VmkDnLaBTP8_Op5vvu=Ozg21T3o6m3w@mail.gmail.com>
On Fri, May 29, 2026 at 2:55 AM Kenny Glowner
<sisyphuscode0311@gmail.com> wrote:
>
> I hope I submitted everything correctly? I am trying to get used to it so I can provide better assistance on more mainstream issues. That's why I started with the most minor issue I could find. I appreciate all of the feedback.
Yes, it looks fine, thanks! :)
However, in general, please try to reply to the feedback, e.g. I asked
a question about the commit message.
Cheers,
Miguel
^ 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