Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: - [Medium] The function do_bulk() contains a loop that will run infinitely if the module parameter 'ct' is set to UINT_MAX, leading to an unkillable task and a denial of service. - [Low] New classes V8 and Vu8 were added to the enums, but were not added to the classmap definition or the test output function, resulting in dead code. --
commit 7bf906f06540c9e08d2719aba493b1c2f6501853 Author: Jim Cromie <[email protected]> dyndbg-test: add do_bulk testpoint, rename do_prints to do_classes This commit updates test_dynamic_debug.ko by renaming do_prints to do_classes and introducing a new do_bulk feature. It aims to generate significant workloads with a single write to sysfs nodes and consolidate the parameter operations for these features. > diff --git a/lib/test_dynamic_debug.c b/lib/test_dynamic_debug.c > index 01ce07001d4c7..b7f9ed9b2794e 100644 > --- a/lib/test_dynamic_debug.c > +++ b/lib/test_dynamic_debug.c [ ... ] > @@ -103,7 +123,10 @@ enum cat_disjoint_bits { > D2_DRMRES }; > > /* numeric verbosity, V2 > V1 related. V1 is > D2_DRMRES */ > -enum cat_level_num { V1 = 16, V2, V3, V4, V5, V6, V7 }; > +enum cat_level_num { V1 = 16, V2, V3, V4, V5, V6, V7, V8 }; > + > +/* test _USE_ w offset */ > +enum cat_level_offset { Vu1 = V1 + 8, Vu2, Vu3, Vu4, Vu5, Vu6, Vu7, Vu8 }; [Severity: Low] This isn't a bug, but since V8 and Vu8 were added to these enumerations, should they also be added to the classmap definitions and tested in do_levels()? Currently, it appears they are left out of the corresponding prdbg() calls and will not generate any log output. [ ... ] > @@ -213,17 +237,42 @@ static void do_levels(void) > #endif > } > > -static void do_prints(void) > +static void do_classes(unsigned int ct) > { [ ... ] > +static void do_bulk(unsigned int ct) > +{ > + unsigned int i; > + > + pr_debug("do_bulk %u times:\n", ct); > + for (i = 1; i <= ct; i++) { [Severity: Medium] If a user sets the sysfs parameter to UINT_MAX, could this loop condition become always true? The commit message mentions using unsigned int to prevent wrapping lockups on UINT_MAX. However, if ct reaches UINT_MAX, the condition (i <= ct) remains true when i reaches UINT_MAX. Then, i++ wraps i back to 0, which is also less than UINT_MAX, meaning the loop would never terminate and the task would become unkillable. Should ct be clamped during parsing in param_set_do_repeats(), or the loop structure modified to safely handle UINT_MAX? > + pr_debug("bulk msg %u.0\n", i); > + pr_debug("bulk msg %u.1\n", i); [ ... ] -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=28
