Thread (39 messages) 39 messages, 2 authors, 2d ago

Re: [PATCH v7 26/29] dyndbg: move mod_name down from struct ddebug_table to _ddebug_info

From: sashiko-bot@kernel.org
Date: 2026-07-21 21:13:03
Also in: dri-devel

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] When processing a built-in module that has no class maps, `ddebug_attach_module_classes()` fails to reset `dt->info.maps.len` to 0, causing the module to erroneously inherit the global array of all built-in class maps.
--
--- Patch [26]: [PATCH v7 26/29] dyndbg: move mod_name down from struct ddebug_table to _ddebug_info ---
commit bd18115a5b87d80ee2f5c15c166c46a301cbd184
Author: Jim Cromie [off-list ref]

    dyndbg: move mod_name down from struct ddebug_table to _ddebug_info
    
    struct _ddebug_info already has most of dyndbg's info for a module;
    push debug_table.mod_name down into it, finishing the encapsulation.

This commit moves the module name from struct ddebug_table into
struct _ddebug_info. This encapsulates the module information, allowing
several callchains to pass _ddebug_info directly rather than the full table.
quoted hunk ↗ jump to hunk
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index e7742fcca50ba..905277fa495b4 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
[ ... ]
quoted hunk ↗ jump to hunk
@@ -1187,7 +1187,7 @@ static void ddebug_attach_module_classes(struct ddebug_table *dt, struct _ddebug
 	 * and length of the subrange at its edges.
 	 */
 	for_subvec(i, cm, di, maps) {
-		if (!strcmp(cm->mod_name, dt->mod_name)) {
+		if (!strcmp(cm->mod_name, dt->info.mod_name)) {
 			if (!nc) {
 				v2pr_info("start subrange, class[%d]: module:%s base:%d len:%d ty:%d\n",
 					  i, cm->mod_name, cm->base, cm->length, cm->map_type);
@@ -1201,7 +1201,7 @@ static void ddebug_attach_module_classes(struct ddebug_table *dt, struct _ddebug
 	}
 	if (nc) {
 		dt->info.maps.len = nc;
-		vpr_info("module:%s attached %d classes\n", dt->mod_name, nc);
+		vpr_info("module:%s attached %d classes\n", dt->info.mod_name, nc);
 	}
 }
[Severity: Medium]
When processing a built-in module that has no class maps, does
ddebug_attach_module_classes() fail to reset dt->info.maps.len to 0?

During boot, dynamic_debug_init() creates a stack variable di where maps.start
and maps.len cover the entire __dyndbg_class_maps section. For each built-in
module, ddebug_add_module(&di) is called. It performs a shallow copy
dt->info = *di, assigning the full length.

It then calls ddebug_attach_module_classes(). If the module has no class maps,
the loop finds no matches (nc == 0), skipping the if (nc) block:

	if (nc) {
		dt->info.maps.len = nc;
		vpr_info("module:%s attached %d classes\n", dt->info.mod_name, nc);
	}

Because of this, does dt->info.maps.len improperly retain the length of all
built-in class maps, causing the module to erroneously inherit the global array
of all built-in class maps?

This logic error could allow users to apply unrelated class strings to a
module's debug prints via the control interface, leading to incorrect functional
behavior of the dynamic debug API.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260721-dd-maint-2-v7-0-010fbe73b311@gmail.com?part=26
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help