linux-kernel - Re: [PATCH v2] selftests/timers: Consolidate and fix 32-bit overflow in timespec

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <87qzw06y8t.ffs@tglx>
Date: Sun, 21 Sep 2025 09:49:54 +0200
From: Thomas Gleixner <tglx@...utronix.de>
To: Wake Liu <wakel@...gle.com>, Anna-Maria Behnsen
 <anna-maria@...utronix.de>, Frederic Weisbecker <frederic@...nel.org>,
 John Stultz <jstultz@...gle.com>, Shuah Khan <shuah@...nel.org>,
 linux-kernel@...r.kernel.org, linux-kselftest@...r.kernel.org
Cc: Stephen Boyd <sboyd@...nel.org>, wakel@...gle.com
Subject: Re: [PATCH v2] selftests/timers: Consolidate and fix 32-bit
 overflow in timespec_sub

On Tue, Sep 16 2025 at 03:19, Wake Liu wrote:
> The timespec_sub function, as implemented in several timer

timespec_sub()

https://www.kernel.org/doc/html/latest/process/maintainer-tip.html#function-references-in-changelogs

> selftests, is prone to integer overflow on 32-bit systems.
>
> The calculation `NSEC_PER_SEC * b.tv_sec` is performed using
> 32-bit arithmetic, and the result overflows before being
> stored in the 64-bit `ret` variable. This leads to incorrect
> time delta calculations and test failures.
>
> As suggested by tglx, this patch fixes the issue by:

s/this patch fixes/fix/



> 1. Creating a new `static inline` helper function,
>    `timespec_to_ns`, which safely converts a `timespec` to
>    nanoseconds by casting `tv_sec` to `long long` before
>    multiplying with `NSEC_PER_SEC`.
>
> 2. Placing the new helper and a rewritten `timespec_sub` into
>    a common header: tools/testing/selftests/timers/helpers.h.
>
> 3. Removing the duplicated, buggy implementations from all
>    timer selftests and replacing them with an #include of the
>    new header.
>
> This consolidates the code and ensures the calculation is
> correctly performed using 64-bit arithmetic across all tests.

This lacks a Signed-off-by.

> Changes in v2:
>   - Per tglx's feedback, instead of changing NSEC_PER_SEC globally,
>     this version consolidates the buggy timespec_sub() implementations
>     into a new 32-bit safe inline function in a shared header.
>   - Amended the commit message to be more descriptive.

change logs go behind the '---' separator as they are not part of the
commit message. It's documented how to format a change log properly.

> -#define UNREASONABLE_LAT (NSEC_PER_SEC * 5) /* hopefully we resume in 5 secs */
> +#define UNREASONABLE_LAT (NSEC_PER_SEC * 5LL) /* hopefully we resume in 5 secs */

How is this change and the pile of similar ones related to $subject and
why are they required in the first place?
  
> index 000000000000..652f20247091
> --- /dev/null
> +++ b/tools/testing/selftests/timers/helpers.h
> @@ -0,0 +1,31 @@

Lacks a SPDX identifier.

scripts/checkpatch.pl exists for a reason.

Thanks,

        tglx