Re: [PATCH v6 19/24] dyndbg: pin class param storage to u32
From: sashiko-bot@kernel.org
Date: 2026-07-08 02:37:28
Also in:
dri-devel
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Medium] Support for classmaps with more than 32 classes is broken, causing undefined behavior on 32-bit architectures and silent truncation on 64-bit architectures. --
--- Patch [19]: [PATCH v6 19/24] dyndbg: pin class param storage to u32 ---commit df6ebc9b56fab43581ec7860d30aab6f33272ee2 Author: Jim Cromie [off-list ref] dyndbg: pin class param storage to u32 This commit changes the storage of dyndbg class parameters from unsigned long to u32 for consistency across architectures. It also shrinks __drm_debug back to u32.
quoted hunk ↗ jump to hunk
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c index d99c69b9ad12f..af05f4ae3b55e 100644 --- a/lib/dynamic_debug.c +++ b/lib/dynamic_debug.c
[ ... ]
quoted hunk ↗ jump to hunk
@@ -643,24 +643,27 @@ static int ddebug_apply_class_bitmap(const struct ddebug_class_param *dcp, int bi, ct; if (*new_bits != old_bits) - v2pr_info("apply bitmap: 0x%lx to: 0x%lx for %s\n", *new_bits, + v2pr_info("apply bitmap: 0x%x to: 0x%x 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)) + bool new_b = !!(*new_bits & BIT(bi)); + bool old_b = !!(old_bits & BIT(bi));
[Severity: Medium] Since classmaps can theoretically contain up to 63 classes (as mentioned in the commit message), is there a risk of an out-of-bounds shift on 32-bit architectures? BIT(bi) expands to 1UL << bi, so if map->length is greater than 32, this shift would exceed the capacity of an unsigned long on 32-bit systems. On 64-bit systems, *new_bits is now a u32. When it is combined with BIT(bi) using a bitwise AND, the u32 is zero-extended. Will this cause new_b and old_b to always evaluate to 0 for classes >= 32, silently ignoring the requested state? [ ... ]
quoted hunk ↗ jump to hunk
@@ -704,24 +707,24 @@ static int param_set_dyndbg_module_classes(const char *instr,
[ ... ]
case DD_CLASS_TYPE_LEVEL_NUM:
/* input is bitpos, of highest verbosity to be enabled */
if (inrep > map->length) {
- pr_warn("%s: level:%ld exceeds max:%d, clamping\n",
+ pr_warn("%s: level:%u exceeds max:%d, clamping\n",
KP_NAME(kp), inrep, map->length);
inrep = map->length;
}
old_bits = CLASSMAP_BITMASK(*dcp->lvl);
new_bits = CLASSMAP_BITMASK(inrep);[Severity: Medium] Will this silently prevent enablement of classes >= 32? CLASSMAP_BITMASK was changed to limit the generated mask to 32 bits, but map->length is not restricted to 32. -- Sashiko AI review · https://sashiko.dev/#/patchset/20260707-dd-maint-2-v6-0-381f3edb0045@gmail.com?part=19