Re: [PATCH v11 17/38] dyndbg: replace classmap list with an array-slice

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] dyndbg: interleaved classmaps are orphaned due to contiguous slice assumption
--

commit 8936e7007afda568cebc34aa496a40887ac6a5a1
Author: Jim Cromie <jim.cromie@gmail.com>

dyndbg: replace classmap list with an array-slice

Classmaps are currently individually list-linked onto dyndbg's per-module
ddebug_table for operation. This commit replaces the list building operation
with an approach that treats packed classmaps as an array-start-address and
array-length, reusing the select-by-modname condition to find the bounds.

> diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
> --- a/lib/dynamic_debug.c
> +++ b/lib/dynamic_debug.c

[ ... ]

> -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++) {

[Severity: Medium]
Since the scan in ddebug_attach_module_classes() always restarts at index 0,
could this cause regressions if multiple descriptor tables exist for the
same interleaved module? When distinct compiled objects share the same
KBUILD_MODNAME, it seems they might all incorrectly bind to the first block
instead of finding their respective fragments.

>  
>  		if (!strcmp(cm->mod_name, dt->mod_name)) {

[ ... ]

> +			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: Medium]
Does this premature break in ddebug_attach_module_classes() enforce an
assumption that classmaps are strictly contiguous?

If classmaps for a module are interleaved, breaking early means only the
first contiguous block is bound. Subsequent classmap fragments might be
permanently orphaned, leaving class'd pr_debug calls in those fragments
uncontrolled.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260923-dd-cmap-part2-clean-v11-0-9b6c217fdf2f@gmail.com?part=17