Re: [PATCH v8 31/43] 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 1 potential issue(s) to consider:
- [High] Infinite loop in do_bulk() due to signed/unsigned comparison and missing CPU yields in test module loops.
--

commit a7ab45223d01dd1193ddd552d158d079f7cd23de
Author: Jim Cromie <jim.cromie@gmail.com>

dyndbg-test: add do_bulk testpoint, rename do_prints to do_classes

This commit enhances the test_dynamic_debug module by renaming the do_prints
sysnode to do_classes and adding a new do_bulk parameter. Both parameters
now accept a work-count to generate significant workloads with a single
write.

> diff --git a/lib/test_dynamic_debug.c b/lib/test_dynamic_debug.c
> index 01ce07001d4c7..39499e52d7c04 100644
> --- a/lib/test_dynamic_debug.c
> +++ b/lib/test_dynamic_debug.c
[ ... ]
> @@ -213,17 +236,40 @@ static void do_levels(void)
>  #endif
>  }
>  
> -static void do_prints(void)
> +static void do_classes(unsigned int ct)
>  {
> -	pr_debug("do_prints:\n");
> -	do_cats();
> -	do_levels();
> +	/* maybe clamp this */
> +	pr_debug("do_classes %d times:\n", ct);
> +	for (; ct; ct--) {
> +		do_cats();
> +		do_levels();
> +	}

[Severity: High]
Since this loop can run for a very long time if a large value is written to
the sysfs node, could it cause CPU or RCU stalls? Would it be safer to add a
cond_resched() inside the loop to prevent lockups?

> +}
> +
> +static void do_bulk(unsigned int ct)
> +{
> +	int i;
> +
> +	pr_debug("do_bulk %d times:\n", ct);
> +	for (i = 1; i <= ct; i++) {

[Severity: High]
Does this loop condition introduce a potential infinite loop?

The loop counter 'i' is declared as a signed int, while 'ct' is an
unsigned int. If a user writes UINT_MAX (4294967295) to the do_bulk sysfs
parameter, 'i' will be promoted to unsigned int for the comparison. Since an
unsigned int can never exceed UINT_MAX, the condition 'i <= ct' will always
be true.

Could this lead to a hard kernel lockup if an extremely large integer is
provided? Should 'i' be declared as an unsigned int, and the loop bounded
or clamped to avoid wrapping?

> +		pr_debug("bulk msg %d.0\n", i);
> +		pr_debug("bulk msg %d.1\n", i);
[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260905-dd-cmap-part2-clean-v8-0-a4cc0674f6fd@gmail.com?part=31