Thread (52 messages) 52 messages, 2 authors, 5d ago

Re: [PATCH v10 24/38] selftests/dyndbg: Enable FT_classmap_inheritance

From: jim.cromie@gmail.com
Date: 2026-09-20 17:25:54
Also in: dri-devel, sashiko-reviews

On Wed, Sep 16, 2026 at 10:24 AM [off-list ref] 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 [off-list ref]

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.
quoted
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
[ ... ]
quoted
@@ -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.
quoted
+    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?
quoted
+    verify_control_slice 'test_dynamic_debug'
+
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260916-dd-cmap-part2-clean-v10-0-af4cf4767707@gmail.com?part=24
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help