Re: [PATCH v2 16/24] 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:
- [High] ddebug_attach_module_classes() assumes all classmaps for a given module are perfectly contiguous, prematurely breaking the loop and causing disjoint classmaps for built-in modules with duplicate names (e.g., "main") to be ignored and incorrectly shared.
--

commit 0adb4bd721cea60d2371ac3aa8cc0fb11eab6469
Author: Jim Cromie <jim.cromie@gmail.com>

dyndbg: replace classmap list with an array-slice

This commit replaces the linked list used to manage classmaps with an
array slice representation. It modifies ddebug_attach_module_classes()
to find a contiguous subrange of classmaps for a module and stores its
start and length, removing the list_head from struct ddebug_class_map.

> diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
> index d4cce0f4f197d..7f03b331d185b 100644
> --- a/lib/dynamic_debug.c
> +++ b/lib/dynamic_debug.c

[ ... ]

> @@ -1147,30 +1149,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 logic correctly handle disjoint classmaps for built-in modules
with duplicate names (such as "main")?

Since di->classes is initialized to __start___dyndbg_classes in
dynamic_debug_init() and is never advanced, ddebug_add_module() processes
the classmaps array from the beginning each time.

Because built-in modules with duplicate names are not contiguous in memory,
dynamic_debug_init() creates multiple ddebug_table structures for them.

When the second "main" chunk is processed, wouldn't this search from the
beginning, find the first module's classmaps, and then prematurely exit
the loop here when it reaches the end of that first contiguous block?

Could this cause the second module's actual classmaps to be permanently
ignored, resulting in subsequent duplicate module names incorrectly
using the wrong classmaps for resolution and control?

>  		}
>  	}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260523-dd-maint-2-v2-0-b937312aa083@gmail.com?part=16