Re: [PATCH v9 28/39] dyndbg-test: add do_bulk testpoint, rename do_prints to do_classes

[sashiko-bot@kernel.org]

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 <jim.cromie@gmail.com>

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/20260908-dd-cmap-part2-clean-v9-0-3a3f621f4a10@gmail.com?part=28