Re: [PATCH v2 19/24] dyndbg,module: make proper substructs in _ddebug_info
From: sashiko-bot@kernel.org
Date: 2026-05-23 07:45:09
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider: - [High] Module loader searches for incorrectly named dynamic debug sections, completely disabling dynamic debug for all loadable modules. - [High] Built-in modules without class maps erroneously inherit all global system-wide class maps due to missing state reset. - [Low] Renaming `struct ddebug_class_param` breaks compilation of external users like the DRM subsystem. -- commit 647ef6a50774bbbe7f638336c42ae9d912b4ca12 Author: Jim Cromie [off-list ref] dyndbg,module: make proper substructs in _ddebug_info This commit recomposes struct _ddebug_info by moving the descriptors and class maps fields into new sub-structures struct _ddebug_descs and struct _ddebug_class_maps. It also normalizes all related struct names to use a _ddebug_ prefix.
quoted hunk ↗ jump to hunk
diff --git a/include/linux/dynamic_debug.h b/include/linux/dynamic_debug.h index a84dcc127e5a7..001bc84eadcab 100644 --- a/include/linux/dynamic_debug.h +++ b/include/linux/dynamic_debug.h
[ ... ]
quoted hunk ↗ jump to hunk
@@ -81,21 +81,33 @@ struct ddebug_class_map { enum ddebug_class_map_type map_type; }; -/* encapsulate linker provided built-in (or module) dyndbg data */ +/* + * @_ddebug_info: gathers module/builtin dyndbg_* __sections together. + * For builtins, it is used as a cursor, with the inner structs + * marking sub-vectors of the builtin __sections in DATA. + */ +struct _ddebug_descs { + struct _ddebug *start; + unsigned int len; +}; + +struct _ddebug_class_maps { + struct _ddebug_class_map *start; + unsigned int len; +}; + struct _ddebug_info { - struct _ddebug *descs; - struct ddebug_class_map *classes; - unsigned int num_descs; - unsigned int num_classes; + struct _ddebug_descs descs; + struct _ddebug_class_maps maps; }; -struct ddebug_class_param { +struct _ddebug_class_param {
[Severity: Low] Does renaming this structure break compilation for external users? For example, the DRM subsystem directly references the old structure name in drivers/gpu/drm/drm_print.c: "static struct ddebug_class_param drm_debug_bitmap" Failing to update these external users will result in a build failure for configurations enabling DRM.
union {
u64 *bits;
u64 *lvl;
};
char flags[8];
- const struct ddebug_class_map *map;
+ const struct _ddebug_class_map *map;
};
/*[ ... ]
quoted hunk ↗ jump to hunk
diff --git a/kernel/module/main.c b/kernel/module/main.c index 46dd8d25a6058..c2b6e70f2e773 100644 --- a/kernel/module/main.c +++ b/kernel/module/main.c@@ -2774,12 +2774,12 @@ static int find_module_sections(struct module *mod, struct load_info *info) pr_warn("%s: Ignoring obsolete parameters\n", mod->name); #ifdef CONFIG_DYNAMIC_DEBUG_CORE - mod->dyndbg_info.descs = section_objs(info, "__dyndbg", - sizeof(*mod->dyndbg_info.descs), - &mod->dyndbg_info.num_descs); - mod->dyndbg_info.classes = section_objs(info, "__dyndbg_classes", - sizeof(*mod->dyndbg_info.classes), - &mod->dyndbg_info.num_classes); + mod->dyndbg_info.descs.start = section_objs(info, "__dyndbg_descriptors", + sizeof(*mod->dyndbg_info.descs.start), + &mod->dyndbg_info.descs.len); + mod->dyndbg_info.maps.start = section_objs(info, "__dyndbg_class_maps", + sizeof(*mod->dyndbg_info.maps.start), + &mod->dyndbg_info.maps.len);
[Severity: High]
Does this correctly match the section names emitted by the linker?
It looks like the section names here were updated to "__dyndbg_descriptors"
and "__dyndbg_class_maps", but the macros like DECLARE_DYNDBG_CLASSMAP and
DEFINE_DYNAMIC_DEBUG_METADATA still emit to "__dyndbg" and "__dyndbg_classes".
Because the strings do not match, section_objs() will fail to find the
sections, setting their lengths to 0 and effectively discarding all
dynamic debug prints for loadable modules.
I see this regression is fixed later in the series by commit f9409a020a4bc
("dyndbg: improve section names").
#endif return 0;
[ ... ]
quoted hunk ↗ jump to hunk
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c index 580aa258f902f..169b94ff6f8ed 100644 --- a/lib/dynamic_debug.c +++ b/lib/dynamic_debug.c
[ ... ]
quoted hunk ↗ jump to hunk
@@ -1202,10 +1203,10 @@ static int ddebug_add_module(struct _ddebug_info *di, const char *modname) { struct ddebug_table *dt; - if (!di->num_descs) + if (!di->descs.len) return 0; - v3pr_info("add-module: %s %d sites\n", modname, di->num_descs); + v3pr_info("add-module: %s %d sites\n", modname, di->descs.len); dt = kzalloc_obj(*dt); if (dt == NULL) {@@ -1219,19 +1220,18 @@ static int ddebug_add_module(struct _ddebug_info *di, const char *modname) * this struct ddebug_table. */ dt->mod_name = modname; - dt->ddebugs = di->descs; - dt->num_ddebugs = di->num_descs; + dt->info = *di;
[Severity: High]
Does this code leave dt->info.maps initialized to the entire system-wide
class maps array if the built-in module has no class maps?
In dynamic_debug_init(), the global _ddebug_info di is initialized with
.maps spanning the entire __dyndbg_classes section.
When this is copied via dt->info = *di here, dt->info.maps also spans
the entire section.
If the module has no class maps, ddebug_attach_module_classes() will run, but
the local variable nc remains 0. The block if (nc) at the end is skipped,
and dt->info.maps is never updated to reflect 0 length.
As a result, built-in modules without class maps falsely match unrelated
class maps from other built-in modules.
I see this regression is fixed later in the series by commit 6fc0ea29f4f01
("dyndbg: hoist classmap-filter-by-modname up to ddebug_add_module").
INIT_LIST_HEAD(&dt->link); - if (di->classes && di->num_classes) + if (di->maps.len) ddebug_attach_module_classes(dt, di); mutex_lock(&ddebug_lock);
-- Sashiko AI review · https://sashiko.dev/#/patchset/20260523-dd-maint-2-v2-0-b937312aa083@gmail.com?part=19