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, Ive seen it locally. it is the reason for comma-terminator patch later in series. next rev adds 2nd level of quoting here, then switches to comma-terminator once that patch is added. > 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
