Thank you for your contribution! Sashiko AI review found 1 potential issue(s) 
to consider:
- [High] A classmap conflict in a single built-in module causes 
dynamic_debug_init() to fail, completely disabling dynamic debugging 
system-wide.
--

commit 10601ffba9f53c0ad961efc6f72432bdf809f6d1
Author: Jim Cromie <[email protected]>

dyndbg: detect class_id reservation conflicts

This commit detects if a module defines multiple classmaps that overlap in the
0..62 class-id space. It adds overlap checking in ddebug_add_module() and
returns -EINVAL when conflicts are detected, ensuring that class_id to
classname lookup works properly.

> diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
> index ba618dcf9677f..0f89b6784ab26 100644
> --- a/lib/dynamic_debug.c
> +++ b/lib/dynamic_debug.c
[ ... ]
> @@ -1409,16 +1425,24 @@ static int ddebug_add_module(struct _ddebug_info *di)
>       dd_set_module_subrange(i, cm, &dt->info, maps);
>       dd_set_module_subrange(i, cli, &dt->info, users);
>  
> +     /* insure 2+ classmaps share the per-module 0..62 class_id space */
> +     for_subvec(i, cm, &dt->info, maps)
> +             if (ddebug_class_range_overlap(cm, &reserved_ids))
> +                     goto cleanup;
> +
>       mutex_lock(&ddebug_lock);
>       list_add_tail(&dt->link, &ddebug_tables);
>       mutex_unlock(&ddebug_lock);
> -
>       if (dt->info.users.len)
>               ddebug_apply_class_users(&dt->info);
>  
>       vpr_info("%3u debug prints in module %s\n",
>                dt->info.descs.len, dt->info.mod_name);
>       return 0;
> +cleanup:
> +     WARN_ONCE(1, "dyndbg multi-classmap conflict in %s\n", di->mod_name);
> +     kfree(dt);
> +     return -EINVAL;
>  }

[Severity: High]
Does returning an error here cause a systemic failure for built-in modules?

Looking at dynamic_debug_init(), it iterates over all built-in modules
and calls ddebug_add_module() for each one. If any single module fails,
it aborts initialization:

lib/dynamic_debug.c:dynamic_debug_init() {
    ...
                if (strcmp(modname, iter->modname)) {
                        ...
                        ret = ddebug_add_module(&di);
                        if (ret)
                                goto out_err;
    ...
out_err:
        ddebug_remove_all_tables();
        return 0;
}

If a single built-in module has an overlapping classmap, the new -EINVAL return
bubbles up and triggers the out_err path. This destroys all previously loaded
tables and completely disables dynamic debugging system-wide, rather than just
skipping the misconfigured module. Should ddebug_add_module() failure be
non-fatal for built-in modules, or perhaps handled more gracefully?

-- 
Sashiko AI review ยท 
https://sashiko.dev/#/patchset/[email protected]?part=29

Reply via email to