Re: [PATCH v7 20/29] dyndbg: refactor param_set_dyndbg_classes and below
From: jim.cromie@gmail.com
Date: 2026-07-23 18:44:57
Also in:
dri-devel, sashiko-reviews
On Tue, Jul 21, 2026 at 3:11 PM [off-list ref] wrote:
quoted hunk ↗ jump to hunk
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Medium] The refactored `param_set_dyndbg_module_classes` is structurally incapable of correctly applying existing global classmap settings to newly loaded modules, and it corrupts the global state. ----- Patch [20]: [PATCH v7 20/29] dyndbg: refactor param_set_dyndbg_classes and below ---commit fd7e6888285afde6b911cef67e7e2452e7180dba Author: Jim Cromie [off-list ref] dyndbg: refactor param_set_dyndbg_classes and below This commit refactors the param_set_dyndbg_classes callchain to support module-specific classmap updates, primarily intended for modprobe handling. It splits the function into upper and lower parts, threading a modname parameter through to ddebug_exec_queries. It also adds a check in ddebug_apply_class_bitmap to skip printing if bits haven't changed.quoted
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c index f66e5373e61e2..a619cefd801f7 100644 --- a/lib/dynamic_debug.c +++ b/lib/dynamic_debug.c[ ... ]quoted
@@ -625,7 +626,9 @@ static int ddebug_apply_class_bitmap(const struct ddebug_class_param *dcp, int matches = 0; int bi, ct; - v2pr_info("apply: 0x%lx to: 0x%lx\n", *new_bits, *old_bits); + if (*new_bits != *old_bits) + v2pr_info("apply bitmap: 0x%lx to: 0x%lx for %s\n", *new_bits, + *old_bits, query_modname ?: "'*'"); for (bi = 0; bi < map->length; bi++) { if (test_bit(bi, new_bits) == test_bit(bi, old_bits)) continue;[Severity: Medium] When applying the current global state to a newly probed module, will this continue statement incorrectly skip all configuration queries?
this is a param callback. the modprobe has finished, and the modprobe args setting classmaps are already done. the param-callback only handles param updates
Since old_bits represents the global state, test_bit() will evaluate to true when applying the same global configuration to a new module, silently skipping required query updates.
quoted
snprintf(query, QUERY_SIZE, "class %s %c%s", map->class_names[bi], test_bit(bi, new_bits) ? '+' : '-', dcp->flags);[ ... ]quoted
@@ -683,8 +693,8 @@ int param_set_dyndbg_classes(const char *instr, const struct kernel_param *kp) KP_NAME(kp), inrep, CLASSMAP_BITMASK(map->length)); inrep &= CLASSMAP_BITMASK(map->length); } - v2pr_info("bits:%lx > %s\n", inrep, KP_NAME(kp)); - totct += ddebug_apply_class_bitmap(dcp, &inrep, dcp->bits); + v2pr_info("bits:0x%lx > %s.%s\n", inrep, mod_name ?: "*", KP_NAME(kp)); + totct += ddebug_apply_class_bitmap(dcp, &inrep, dcp->bits, mod_name); *dcp->bits = inrep;[Severity: Medium] Does this code inadvertently corrupt the global state if a module-specific override is applied?
the param belongs to a module, its not "global". and I dont know what override means here. the callback MUST update the internal state, or error out.
It appears that *dcp->bits (the global parameter state) is unconditionally overwritten with inrep, even when a mod_name is provided.
the param belongs to a module. so does the param state.
Should the state update be conditional on mod_name being NULL, to prevent modprobe handling from modifying the global state?quoted
break; case DD_CLASS_TYPE_LEVEL_NUM:@@ -697,7 +707,7 @@ int param_set_dyndbg_classes(const char *instr, const struct kernel_param *kp) old_bits = CLASSMAP_BITMASK(*dcp->lvl); new_bits = CLASSMAP_BITMASK(inrep); v2pr_info("lvl:%ld bits:0x%lx > %s\n", inrep, new_bits, KP_NAME(kp)); - totct += ddebug_apply_class_bitmap(dcp, &new_bits, &old_bits); + totct += ddebug_apply_class_bitmap(dcp, &new_bits, &old_bits, mod_name); *dcp->lvl = inrep;[Severity: Medium] Similarly, is the global *dcp->lvl state unconditionally overwritten here when a module-specific override is applied for LEVEL_NUM map types?
the state belongs to the module.
quoted
break; default:-- Sashiko AI review · https://sashiko.dev/#/patchset/20260721-dd-maint-2-v7-0-010fbe73b311@gmail.com?part=20