On Wed, Sep 23, 2026 at 4:49 PM <[email protected]> wrote:
>
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s)
> to consider:
> - [Medium] dyndbg: TOCTOU race in ddebug_sync_classbits allows stale module
> settings
> - [Low] dyndbg: incompatible pointer type in module section load
> --
>
> commit 0e12bed35baa8c038def6b06ffc95416176ee997
> Author: Jim Cromie <[email protected]>
>
> dyndbg-API: replace DECLARE_DYNDBG_CLASSMAP
>
> This commit replaces DECLARE_DYNDBG_CLASSMAP with an explicit client-server
> model using DEFINE and USE macros. It resolves boot-time parameter
> propagation failures by establishing a robust reference linkage. At module
> load time, dyndbg now scans and resolves parent classmap references.
>
> > diff --git a/kernel/module/main.c b/kernel/module/main.c
> > --- a/kernel/module/main.c
> > +++ b/kernel/module/main.c
> [ ... ]
> > @@ -2791,6 +2791,9 @@ static int find_module_sections(struct module *mod,
> > struct load_info *info)
> > mod->dyndbg_info.maps.start = section_objs(info,
> > "__dyndbg_class_maps",
> >
> > sizeof(*mod->dyndbg_info.maps.start),
> >
> > &mod->dyndbg_info.maps.len);
> > + mod->dyndbg_info.users.start = section_objs(info,
> > "__dyndbg_class_users",
> > +
> > sizeof(*mod->dyndbg_info.users.start),
> > +
> > &mod->dyndbg_info.users.len);
>
> [Severity: Low]
> The len field in struct _ddebug_class_users is declared as a signed int, but
> section_objs() expects an unsigned int pointer for its num parameter. Could
> passing &mod->dyndbg_info.users.len here trigger an incompatible pointer
> type compiler warning and result in a build error under standard
> configurations?
>
fixed for v12
> [ ... ]
> > 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_sync_classbits(const struct kernel_param *kp, const
> > char *modname)
> > +{
> > + const struct ddebug_class_param *dcp = kp->arg;
> > + u32 val, new_bits;
> > +
> > + if (!dcp || !dcp->map)
> > + return;
> > +
> > + switch (dcp->map->map_type) {
> > + case DD_CLASS_TYPE_DISJOINT_BITS:
> > + val = READ_ONCE(*dcp->bits);
> > + new_bits = val;
> > + v2pr_info(" %s: classbits: 0x%x\n", KP_NAME(kp), new_bits);
> > + ddebug_apply_class_bitmap(dcp, &new_bits, 0, modname);
>
> [Severity: Medium]
> Because dcp->bits is read locklessly via READ_ONCE() before
> ddebug_apply_class_bitmap() takes ddebug_lock, is there a race condition
> here?
>
> If a user concurrently writes to the corresponding sysfs parameter, the sysfs
> thread will update existing modules under the lock, release it, and then
> update the parameter value.
>
> If a concurrent module load thread executing ddebug_sync_classbits() reads
> the old parameter value right before the sysfs thread writes the new value,
> and then acquires the lock to apply the settings:
>
> sysfs thread:
> param_set_dyndbg_module_classes()
> mutex_lock(&ddebug_lock);
> // updates existing modules
> mutex_unlock(&ddebug_lock);
>
> Module load thread in ddebug_sync_classbits():
> val = READ_ONCE(*dcp->bits); // reads old stale value
>
> sysfs thread:
> *dcp->bits = new_value;
>
> Module load thread in ddebug_sync_classbits():
> ddebug_apply_class_bitmap(...)
> mutex_lock(&ddebug_lock);
> // applies stale value
>
> Would the newly loading module miss the concurrent sysfs update and
> initialize with stale debug class settings?
>
Holding ddebug_lock during the READ_ONCE() in ddebug_sync_classbits()
would deadlock against ddebug_change(), which acquires ddebug_lock to
iterate ddebug_tables and toggle descriptor flags.
ddebug_add_module() publishes dt to ddebug_tables under ddebug_lock
before calling ddebug_sync_classbits(). Because ddebug_change()
acquires ddebug_lock while walking ddebug_tables, any concurrent sysfs
write either completes before publication (so ddebug_sync_classbits()
reads the new value) or executes after publication (so ddebug_change()
finds and updates the new module directly).
> --
> Sashiko AI review ·
> https://sashiko.dev/#/patchset/[email protected]?part=23
