Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: - [Medium] TOCTOU race in cross-module dynamic debug parameter synchronization during module load. - [Low] Missing fallback macro for `DYNAMIC_DEBUG_CLASSMAP_USE_` when `CONFIG_DYNAMIC_DEBUG` is disabled. --
commit 094b72a96b9738b25bbc12d704057690585b0e4e Author: Jim Cromie <[email protected]> Subject: dyndbg-API: replace DECLARE_DYNDBG_CLASSMAP This commit deprecates DECLARE_DYNDBG_CLASSMAP and replaces it with DYNAMIC_DEBUG_CLASSMAP_DEFINE and DYNAMIC_DEBUG_CLASSMAP_USE. It introduces the __dyndbg_class_users section to coordinate module classmap use across the DRM subsystem. This decoupling resolves a regression where child drivers failed to initialize properly. > diff --git a/include/linux/dynamic_debug.h b/include/linux/dynamic_debug.h > index 661599a1302de..17fc3a29d97b8 100644 > --- a/include/linux/dynamic_debug.h > +++ b/include/linux/dynamic_debug.h [ ... ] > @@ -327,6 +457,8 @@ void __dynamic_ibdev_dbg(struct _ddebug *descriptor, > #include <linux/errno.h> > #include <linux/printk.h> > > +#define DYNAMIC_DEBUG_CLASSMAP_DEFINE(_var, _mapty, _base, ...) > +#define DYNAMIC_DEBUG_CLASSMAP_USE(_var) [Severity: Low] Is there a missing fallback definition for DYNAMIC_DEBUG_CLASSMAP_USE_ here? Drivers using DYNAMIC_DEBUG_CLASSMAP_USE_ with a custom offset will likely fail to compile due to an undeclared macro when CONFIG_DYNAMIC_DEBUG is disabled. > #define DEFINE_DYNAMIC_DEBUG_METADATA(name, fmt) > #define DYNAMIC_DEBUG_BRANCH(descriptor) false > diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c > index 5f1cf9d76080b..c0e95442871ce 100644 > --- a/lib/dynamic_debug.c > +++ b/lib/dynamic_debug.c [ ... ] > @@ -1238,12 +1404,17 @@ static int ddebug_add_module(struct _ddebug_info *di) > */ > dt->info = *di; > dd_set_module_subrange(i, cm, &dt->info, maps); > + dd_set_module_subrange(i, cli, &dt->info, users); > > mutex_lock(&ddebug_lock); > list_add_tail(&dt->link, &ddebug_tables); > mutex_unlock(&ddebug_lock); > > - vpr_info("%3u debug prints in module %s\n", di->descs.len, > di->mod_name); > + if (dt->info.users.len) > + ddebug_apply_class_users(&dt->info); [Severity: Medium] Does dropping ddebug_lock before calling ddebug_apply_class_users() introduce a TOCTOU race window? When a new module is added to ddebug_tables under ddebug_lock, it becomes visible to sysfs. If a concurrent sysfs write triggers a parameter update, it will apply the class bitmap and update the parameter value across all loaded modules. Since ddebug_apply_class_users() (which eventually calls ddebug_sync_classbits()) executes without the lock, it might read a stale parameter value if the sysfs thread updates the callsites but hasn't yet updated the parameter itself. Could this result in the newly loaded module having its callsites improperly overwritten with a stale value? > + > + vpr_info("%3u debug prints in module %s\n", > + dt->info.descs.len, dt->info.mod_name); > return 0; > } > -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=27
