Re: [PATCH v2 03/12] perf tests workloads: Support sub-second durations in noploop and thloop

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Floating-point parsing fails to reject NaN and excessively large inputs, leading to undefined behavior and an infinite loop.
--

commit 27288129892e9ee8db8d116f0ab2af7cffc87383
Author: Ian Rogers <irogers@google.com>

perf tests workloads: Support sub-second durations in noploop and thloop

This commit modifies the noploop and thloop workloads to support sub-second
sleep durations. It changes the duration argument parsing from atoi() to
atof() and adds a fallback to ualarm() for fractional durations less than
1.0 second.

> diff --git a/tools/perf/tests/workloads/noploop.c b/tools/perf/tests/workloads/noploop.c
> --- a/tools/perf/tests/workloads/noploop.c
> +++ b/tools/perf/tests/workloads/noploop.c
[ ... ]
> @@ -15,15 +15,26 @@ static void sighandler(int sig __maybe_unused)
>  
>  static int noploop(int argc, const char **argv)
>  {
[ ... ]
> +	if (sec <= 0.0) {
> +		fprintf(stderr, "Error: seconds (%f) must be > 0\n", sec);
> +		return 1;
> +	}
>  
>  	signal(SIGINT, sighandler);
>  	signal(SIGALRM, sighandler);
> -	alarm(sec);
> +
> +	if (sec < 1.0) {
> +		useconds_t usecs = (useconds_t)(sec * 1000000.0);
> +
> +		ualarm(usecs > 0 ? usecs : 1, 0);
> +	} else
> +		alarm((unsigned int)sec);

[Severity: Medium]
Could this lead to an infinite loop if a user provides an invalid value
like "NaN"?

The atof() function can parse strings like "NaN", which evaluates to false
for both the sec <= 0.0 validation check and the sec < 1.0 branch.

This would cause execution to fall through to the alarm((unsigned int)sec)
call. Casting NaN to an unsigned int is undefined behavior, which on common
architectures evaluates to 0. Since alarm(0) cancels any pending alarms
instead of setting one, will the test suite hang indefinitely in the
subsequent while (!done) loop?

> diff --git a/tools/perf/tests/workloads/thloop.c b/tools/perf/tests/workloads/thloop.c
> --- a/tools/perf/tests/workloads/thloop.c
> +++ b/tools/perf/tests/workloads/thloop.c
[ ... ]
> @@ -67,7 +68,12 @@ static int thloop(int argc, const char **argv)
>  			goto out;
>  		}
>  	}
> -	alarm(sec);
> +	if (sec < 1.0) {
> +		useconds_t usecs = (useconds_t)(sec * 1000000.0);
> +
> +		ualarm(usecs > 0 ? usecs : 1, 0);
> +	} else
> +		alarm((unsigned int)sec);

[Severity: Medium]
Does this have the same NaN handling issue as noploop()?

Bypassing the check allows an invalid timer value which disables the alarm
when cast to unsigned int, causing the thread pool workload to run forever
when it reaches test_loop().

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260616061404.41929-1-irogers@google.com?part=3