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 <[email protected]>

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/[email protected]?part=17

Reply via email to