Re: [PATCH v2 18/24] dyndbg: Upgrade class param storage to u64 for 64-bit classmaps
From: sashiko-bot@kernel.org
Date: 2026-05-23 07:42:49
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: New issues: - [High] Buffer Overflow / Memory Corruption due to mismatched types in `ddebug_class_param`. Pre-existing issues: - [High] Undefined Behavior in `CLASSMAP_BITMASK` triggers shift overflow for 64-bit classmaps. -- commit 58d2c659362026860249b9d71580382d0d84aa4e Author: Jim Cromie [off-list ref] dyndbg: Upgrade class param storage to u64 for 64-bit classmaps This commit upgrades the internal storage types in struct ddebug_class_param from unsigned long to u64 pointers. This change aims to safely support 64-bit classmaps across all architectures without truncation.
quoted hunk ↗ jump to hunk
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c index e9481ef21825d..580aa258f902f 100644 --- a/lib/dynamic_debug.c +++ b/lib/dynamic_debug.c@@ -661,7 +663,7 @@ static int ddebug_apply_class_bitmap(const struct ddebug_class_param *dcp, /* stub to later conditionally add "$module." prefix where not already done */ #define KP_NAME(kp) kp->name -#define CLASSMAP_BITMASK(width) ((1UL << (width)) - 1) +#define CLASSMAP_BITMASK(width) ((1ULL << (width)) - 1)
[Severity: High] This isn't a bug introduced by this patch, but does this macro invoke undefined behavior if the classmap width reaches 64 bits? If width is 64, shifting 1ULL by 64 is undefined behavior in C. On some architectures like x86, the shift count is masked by 63, causing 1ULL << 64 to evaluate to 1ULL << 0 (which is 1). This would result in a bitmask of 0, wiping out valid input bits and breaking the 64-bit feature. [ ... ]
quoted hunk ↗ jump to hunk
@@ -696,24 +698,24 @@ static int param_set_dyndbg_module_classes(const char *instr, case DD_CLASS_TYPE_DISJOINT_BITS: /* expect bits. mask and warn if too many */ if (inrep & ~CLASSMAP_BITMASK(map->length)) { - pr_warn("%s: input: 0x%lx exceeds mask: 0x%lx, masking\n", + pr_warn("%s: input: 0x%llx exceeds mask: 0x%llx, masking\n", KP_NAME(kp), inrep, CLASSMAP_BITMASK(map->length)); inrep &= CLASSMAP_BITMASK(map->length); } - v2pr_info("bits:0x%lx > %s.%s\n", inrep, mod_name ?: "*", KP_NAME(kp)); + v2pr_info("bits:0x%llx > %s.%s\n", inrep, mod_name ?: "*", KP_NAME(kp)); totct += ddebug_apply_class_bitmap(dcp, &inrep, *dcp->bits, mod_name); *dcp->bits = inrep;
[Severity: High] Can this cause memory corruption on 32-bit architectures? The commit message notes that __drm_debug is not yet updated to u64, meaning it remains an unsigned long. On 32-bit systems, unsigned long is 4 bytes. Since dcp->bits is now a u64 pointer, won't assigning inrep perform an 8-byte write into a 4-byte allocation, corrupting adjacent memory? -- Sashiko AI review · https://sashiko.dev/#/patchset/20260523-dd-maint-2-v2-0-b937312aa083@gmail.com?part=18