Re: [PATCH v3] tracing/hwlat: Do the latency calculations in nanoseconds

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Medium] tracing/hwlat: broken backward-clock check order in
  get_sample()
--

commit e1ff23abe0f3e347e9f8fcfd5d38bde48773198a
Author: Wenwu Hou <hwenwur@gmail.com>

tracing/hwlat: Do the latency calculations in nanoseconds

This commit changes get_sample() to compute latency deltas in nanoseconds
and fixes the backward-clock error paths to clear
trace_hwlat_callback_enabled before returning.

> diff --git a/kernel/trace/trace_hwlat.c b/kernel/trace/trace_hwlat.c
> index 3fe274b84f1c2..d8540c24b1ca3 100644
> --- a/kernel/trace/trace_hwlat.c
> +++ b/kernel/trace/trace_hwlat.c
> @@ -229,18 +227,19 @@ static int get_sample(void)
>  
>  		if (time_u64(last_t2)) {
>  			/* Check the delta from outer loop (t2 to next t1) */
> -			outer_diff = time_to_us(time_sub(t1, last_t2));
> +			outer_diff = time_sub(t1, last_t2);
>  			/* This shouldn't happen */
>  			if (outer_diff < 0) {
>  				hwlat_err(BANNER "time running backwards\n");
> -				goto out;
> +				ret = -1;
> +				break;
>  			}
>  			if (outer_diff > outer_sample)
>  				outer_sample = outer_diff;
>  		}
>  		last_t2 = t2;
>  
> -		total = time_to_us(time_sub(t2, start)); /* sample width */
> +		total = time_sub(t2, start); /* sample width */
>  
>  		/* Check for possible overflows */
>  		if (total < last_total) {
> @@ -250,7 +249,7 @@ static int get_sample(void)
>  		last_total = total;
>  
>  		/* This checks the inner loop (t1 to t2) */
> -		diff = time_to_us(time_sub(t2, t1));     /* current diff */
> +		diff = time_sub(t2, t1);     /* current diff */
>  
>  		if (diff > thresh || outer_diff > thresh) {
>  			if (!count)
> @@ -261,7 +260,8 @@ static int get_sample(void)
>  		/* This shouldn't happen */
>  		if (diff < 0) {
>  			hwlat_err(BANNER "time running backwards\n");
> -			goto out;
> +			ret = -1;
> +			break;
>  		}

[Severity: Medium]
Can a backward clock jump between t1 and t2 bypass the diff < 0 check and
record uninitialized stack memory in s.timestamp?

Because total < last_total is checked before diff = time_sub(t2, t1), any
backward jump between t1 and t2 where t2 drops below the previous t2 (or
below start on the first iteration) trips total < last_total first.  That
logs "Time total overflowed" instead of "time running backwards" and breaks
out of the loop with ret == 0 before ktime_get_real_ts64(&s.timestamp) runs.
If outer_diff exceeded thresh earlier in the same iteration while count was
still 0, outer_sample > thresh then causes trace_hwlat_sample(&s) to copy
an uninitialized s.timestamp from the stack into the ring buffer.

Also, diff > thresh is evaluated before diff < 0, which converts a negative
s64 diff to a large u64 when comparing against unsigned u64 thresh.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261113094918.2402297-1-hwenwur@gmail.com?part=1