On Wed, Sep 16, 2026 at 10:24 AM <[email protected]> wrote:
>
> Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider:
> - [Medium] selftests: dyndbg: load-time parameters truncated due to shell
quoting bug
> --
>
> commit f2f3cad7addd566a67a218932463053f35d4d782
> Author: Jim Cromie <[email protected]>
>
> selftests/dyndbg: Enable FT_classmap_inheritance
>
> Exercise classmap state inheritance between supermod and submod,
> verifying that class settings and controlling parameter states propagate
> from parent classmap definitions to dependent class users upon module
> load and during runtime reconfiguration.
>
> > diff --git a/tools/testing/selftests/dynamic_debug/dyndbg_selftest.sh
b/tools/testing/selftests/dynamic_debug/dyndbg_selftest.sh
> > index 09a8dbf239dcf..51338ebc74bf8 100755
> > --- a/tools/testing/selftests/dynamic_debug/dyndbg_selftest.sh
> > +++ b/tools/testing/selftests/dynamic_debug/dyndbg_selftest.sh
> [ ... ]
> > @@ -298,6 +299,78 @@ function FT_basic_queries {
> >      ddcmd "file $f =_" "$f"
> >  }
> >
> > +function FT_classmap_inheritance {
> > +    v_echo "${GREEN}# TEST_MOD_SUBMOD ${NC}"
> > +
> > +    ifrmmod test_dynamic_debug_submod
> > +    ifrmmod test_dynamic_debug
> > +
> > +    # modprobe with plain-old +p & 3 class enablements
> > +    my_modprobe test_dynamic_debug \
> > +     "dyndbg=+p;class D2_CORE +pf;class D2_KMS +pt;class D2_ATOMIC +pm"
>
> [Severity: Medium]
> Are these load-time parameters truncated during module load due to how the
> shell handles quotes?
>

yes it does.  thats the reason a subsequent patch adds the comma as
token-terminator.
Ive folded this problem out in next rev.

> When bash evaluates and strips the double quotes before executing
modprobe,
> modprobe constructs the kernel options string without adding quotes around
> the parameters. The kernel's parse_args() function (using next_arg() in
> lib/cmdline.c) splits module parameters at unquoted spaces.
>
> This means it treats D2_CORE and +pf as separate unknown parameters. The
> dyndbg parsing logic executes +p but fails on the truncated class command,
> completely ignoring the actual class configurations.
>
> Does this defeat the test's intent to verify load-time classmap
inheritance,
> allowing it to pass only because the golden hash recorded the incorrectly
> applied +p state?
>
> It appears this space-splitting flaw persists through the end of the patch
> series, and the newly added FT_test_classes also replicates it.
>
> > +    verify_control_slice '\[test_dynamic_debug\]'
> > +
> > +    set_param 5 /sys/module/test_dynamic_debug/parameters/p_level_num
> > +    verify_control_slice '\[test_dynamic_debug\]'
> > +
> > +    my_modprobe test_dynamic_debug_submod
> > +    verify_control_slice 'test_dynamic_debug_submod'
> > +
> > +    # fresh start, to clear all above flags (test-fn limits)
> > +    ifrmmod test_dynamic_debug_submod
> > +    ifrmmod test_dynamic_debug
> > +
> > +    # load submod, which loads supermod
> > +    my_modprobe test_dynamic_debug_submod \
> > +     "dyndbg=+p;class D2_CORE +pfs;class D2_KMS +pts;class D2_ATOMIC
+pmf"
>
> [Severity: Medium]
> Does this submod load suffer from the same space-splitting issue?
>
> > +    verify_control_slice 'test_dynamic_debug'
> > +
>
> --
> Sashiko AI review ·
https://sashiko.dev/#/patchset/[email protected]?part=24

Reply via email to