Re: [PATCH v4 16/25] dyndbg: replace classmap list with an array-slice
From: sashiko-bot@kernel.org
Date: 2026-06-02 23:01:42
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] ddebug_attach_module_classes() incorrectly assigns the first matching classmap subrange to all non-contiguous descriptor blocks of identically-named built-in modules, permanently dropping subsequent classmap subranges. -- commit 6fb00fe7f4da26525b10dfd1edfb384c35a9b281 Author: Jim Cromie [off-list ref] dyndbg: replace classmap list with an array-slice This commit replaces the list-based storage of dyndbg classmaps with contiguous array-slices. It simplifies classmap attachment by dropping list construction and reusing the module name filter to find subranges.
quoted hunk ↗ jump to hunk
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c index 9bd521651c484..b8a494835ef58 100644 --- a/lib/dynamic_debug.c +++ b/lib/dynamic_debug.c
[ ... ]
quoted hunk ↗ jump to hunk
@@ -1150,30 +1152,34 @@ static const struct proc_ops proc_fops = { .proc_write = ddebug_proc_write }; -static void ddebug_attach_module_classes(struct ddebug_table *dt, - struct ddebug_class_map *classes, - int num_classes) +static void ddebug_attach_module_classes(struct ddebug_table *dt, struct _ddebug_info *di) { struct ddebug_class_map *cm; - int i, j, ct = 0; + int i, nc = 0; - for (cm = classes, i = 0; i < num_classes; i++, cm++) { + /* + * Find this module's classmaps in a subrange/wholerange of + * the builtin/modular classmap vector/section. Save the start + * and length of the subrange at its edges. + */ + for (cm = di->classes, i = 0; i < di->num_classes; i++, cm++) { if (!strcmp(cm->mod_name, dt->mod_name)) { - - v2pr_info("class[%d]: module:%s base:%d len:%d ty:%d\n", i, - cm->mod_name, cm->base, cm->length, cm->map_type); - - for (j = 0; j < cm->length; j++) - v3pr_info(" %d: %d %s\n", j + cm->base, j, - cm->class_names[j]); - - list_add(&cm->link, &dt->maps); - ct++; + 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; + } + nc++; + } else if (nc) { + /* end of matching classmaps */ + break; }
[Severity: High]
Does this break prematurely for identically-named built-in modules?
Because KBUILD_MODNAME currently allows multiple built-in modules to have the
same name (e.g., "main"), these modules can have non-contiguous descriptor and
classmap blocks in the ELF section.
When ddebug_attach_module_classes() is called for a second "main" module,
it scans from the beginning of di->classes, finds the first "main" module's
classmaps, and immediately breaks upon the first non-match.
Will this cause the second module's actual classmaps to be permanently discarded,
assigning it the wrong classmap subrange instead?
Note: I saw this issue is resolved later in the series by commit
3e924d4e19d326f9d0f87715e82bd2970e66e217 ("dynamic_debug: use KBUILD_MODFILE
for unique builtin module names"), which prevents identically-named built-in
modules by using unique file paths.
}
- if (ct)
- vpr_info("module:%s attached %d classes\n", dt->mod_name, ct);
+ if (nc) {
+ dt->num_classes = nc;
+ vpr_info("module:%s attached %d classes\n", dt->mod_name, nc);
+ }
}-- Sashiko AI review · https://sashiko.dev/#/patchset/20260602-dd-maint-2-v4-0-19a1445585a8@gmail.com?part=16