Re: [PATCH v7 25/29] dyndbg,module: make proper substructs in _ddebug_info
From: sashiko-bot@kernel.org
Date: 2026-07-21 21:09:29
Also in:
dri-devel
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Medium] Built-in modules without their own dynamic debug class maps incorrectly inherit the entire array of built-in class maps, causing cross-contamination of debug class settings. --
--- Patch [25]: [PATCH v7 25/29] dyndbg,module: make proper substructs in _ddebug_info ---commit 9690152a047a31662e977dac229f6c3d6cb68cd2 Author: Jim Cromie [off-list ref] dyndbg,module: make proper substructs in _ddebug_info This commit recomposes struct _ddebug_info by inserting proper sub-structs for descriptors and class maps, and updates the dynamic debug core to use these new structures.
quoted hunk ↗ jump to hunk
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c index 60b67face46e1..e7742fcca50ba 100644 --- a/lib/dynamic_debug.c +++ b/lib/dynamic_debug.c
[ ... ]
quoted hunk ↗ jump to hunk
@@ -1185,12 +1186,12 @@ static void ddebug_attach_module_classes(struct ddebug_table *dt, struct _ddebug * the builtin/modular classmap vector/section. Save the start * and length of the subrange at its edges. */ - for_subvec(i, cm, di, classes) { + for_subvec(i, cm, di, maps) { if (!strcmp(cm->mod_name, dt->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); - dt->classes = cm; + dt->info.maps.start = cm; } nc++; } else if (nc) {@@ -1199,7 +1200,7 @@ static void ddebug_attach_module_classes(struct ddebug_table *dt, struct _ddebug } } if (nc) { - dt->num_classes = nc; + dt->info.maps.len = nc; vpr_info("module:%s attached %d classes\n", dt->mod_name, nc); } }
[Severity: Medium] When a module has no classes and nc remains 0, does dt->info.maps retain the values originally copied from di? Since dt->info = *di is performed before this function is called, this means the module will incorrectly inherit the entire global array of built-in class maps. Could this cause cross-contamination of debug class settings between unrelated modules? [ ... ]
quoted hunk ↗ jump to hunk
@@ -1212,10 +1213,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) {@@ -1229,19 +1230,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; INIT_LIST_HEAD(&dt->link); - if (di->classes && di->num_classes) + if (di->maps.len) ddebug_attach_module_classes(dt, di);
[Severity: Medium] If di->maps.len > 0 (which is true for built-ins since di spans all global maps), but the module itself has no classes, ddebug_attach_module_classes() won't reset dt->info.maps.len to 0. Would it be better to clear dt->info.maps.len and dt->info.maps.start here before calling ddebug_attach_module_classes() to prevent the module from accidentally keeping the global state? -- Sashiko AI review · https://sashiko.dev/#/patchset/20260721-dd-maint-2-v7-0-010fbe73b311@gmail.com?part=25