Re: [PATCH v3 19/24] dyndbg,module: make proper substructs in _ddebug_info

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Built-in modules without class maps incorrectly inherit the global built-in class maps array, violating state invariants and causing performance regressions on queries.
--

commit 97c08d5dc7f7ec501676f73addba8ad9f6c67352
Author: Jim Cromie <jim.cromie@gmail.com>

dyndbg,module: make proper substructs in _ddebug_info

This patch recomposes struct _ddebug_info by inserting proper sub-structs
for descriptors and class maps. It updates the usage of this struct as a
cursor for built-in module data and module state access. Note that the
regression detailed below was fixed later in the patch series by commit
b54eb6f1ef1ecdd4f503388db9a2426bb84d49d1.

> diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
> --- a/lib/dynamic_debug.c
> +++ b/lib/dynamic_debug.c
> @@ -1176,12 +1177,12 @@ static void ddebug_attach_module_classes(struct ddebug_table *dt, struct _ddebug
>  	 * the builtin/modular classmap vector/section.  Save the start
>  	 * and length of the subrange at its edges.
>  	 */
> -	for_subvec(i, cm, di, classes) {
> +	for_subvec(i, cm, di, maps) {
>  		if (!strcmp(cm->mod_name, dt->mod_name)) {

[ ... ]

>  		} else if (nc) {
>  			/* end of matching classmaps */
>  			break;
>  		}
>  	}
>  	if (nc) {
> -		dt->num_classes = nc;
> +		dt->info.maps.len = nc;

[Severity: Medium]
When a built-in module does not have any class maps (so nc remains 0), this
function will exit without setting dt->info.maps.len to 0. 

Because dt->info was populated by a shallow copy of *di earlier in
ddebug_add_module(), this leaves dt->info.maps.len set to the global built-in
class maps array length. Does this cause modules without class maps to
incorrectly retain the entire global class maps array bounds?

This appears to degrade query time complexity, as ddebug_change() will falsely
find class maps in these modules and needlessly iterate over all their
descriptors.

>  		vpr_info("module:%s attached %d classes\n", dt->mod_name, nc);
>  	}
>  }

[ ... ]

> @@ -1220,19 +1221,18 @@ static int ddebug_add_module(struct _ddebug_info *di, const char *modname)
>  	 * this struct ddebug_table.
>  	 */
>  	dt->mod_name = modname;
> -	dt->ddebugs = di->descs;
> -	dt->num_ddebugs = di->num_descs;
> +	dt->info = *di;

[Severity: Medium]
This shallow struct copy initializes the module's maps with the global array
bounds. Before this patch, dt->num_classes was correctly initialized to 0
by kzalloc_obj(*dt) and only updated if classes were found.

If ddebug_attach_module_classes() doesn't overwrite dt->info.maps.len for
modules with no class maps, does this break the contiguous subrange invariant?

>  
>  	INIT_LIST_HEAD(&dt->link);
>  
> -	if (di->classes && di->num_classes)
> +	if (di->maps.len)
>  		ddebug_attach_module_classes(dt, di);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260601-dd-maint-2-v3-0-4a15b241bd3c@gmail.com?part=19