Re: [PATCH v7 20/29] dyndbg: refactor param_set_dyndbg_classes and below
From: sashiko-bot@kernel.org
Date: 2026-07-21 21:11:46
Also in:
dri-devel
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 hunk ↗ jump to hunk
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 hunk ↗ jump to hunk
@@ -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? 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.
snprintf(query, QUERY_SIZE, "class %s %c%s", map->class_names[bi], test_bit(bi, new_bits) ? '+' : '-', dcp->flags);
[ ... ]
quoted hunk ↗ jump to hunk
@@ -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? It appears that *dcp->bits (the global parameter state) is unconditionally overwritten with inrep, even when a mod_name is provided. Should the state update be conditional on mod_name being NULL, to prevent modprobe handling from modifying the global state?
quoted hunk ↗ jump to hunk
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?
break; default:
-- Sashiko AI review · https://sashiko.dev/#/patchset/20260721-dd-maint-2-v7-0-010fbe73b311@gmail.com?part=20