Re: [PATCH v8 27/43] dyndbg-API: replace DECLARE_DYNDBG_CLASSMAP

[sashiko-bot@kernel.org]

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 <jim.cromie@gmail.com>
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/20260905-dd-cmap-part2-clean-v8-0-a4cc0674f6fd@gmail.com?part=27