linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] vdso: Define NSEC_PER_SEC as 64-bit to prevent overflow
@ 2025-08-05 16:20 Wake Liu
  0 siblings, 0 replies; 9+ messages in thread
From: Wake Liu @ 2025-08-05 16:20 UTC (permalink / raw)
  To: Andy Lutomirski, Thomas Gleixner, Vincenzo Frascino; +Cc: linux-kernel, wakel

The constant NSEC_PER_SEC (1,000,000,000) is defined as a long literal.
On 32-bit systems like arm32, where long is 32 bits, this value can
overflow when used in 64-bit calculations.

This can lead to incorrect time calculations within the VDSO.

To fix this, define NSEC_PER_SEC as a long long literal (1000000000LL),
ensuring it is treated as a 64-bit value on all architectures and
preventing potential overflows.

Signed-off-by: Wake Liu <wakel@google.com>
---
 include/vdso/time64.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/vdso/time64.h b/include/vdso/time64.h
index b40cfa2aa33c..07641b27d139 100644
--- a/include/vdso/time64.h
+++ b/include/vdso/time64.h
@@ -8,7 +8,7 @@
 #define NSEC_PER_USEC	1000L
 #define NSEC_PER_MSEC	1000000L
 #define USEC_PER_SEC	1000000L
-#define NSEC_PER_SEC	1000000000L
+#define NSEC_PER_SEC	1000000000LL
 #define PSEC_PER_SEC	1000000000000LL
 #define FSEC_PER_SEC	1000000000000000LL
 
-- 
2.50.1.565.gc32cd1483b-goog


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH] vdso: Define NSEC_PER_SEC as 64-bit to prevent overflow
@ 2025-08-05 16:21 Wake Liu
  2025-08-06  8:44 ` Thomas Gleixner
                   ` (3 more replies)
  0 siblings, 4 replies; 9+ messages in thread
From: Wake Liu @ 2025-08-05 16:21 UTC (permalink / raw)
  To: Andy Lutomirski, Thomas Gleixner, Vincenzo Frascino; +Cc: linux-kernel, wakel

The constant NSEC_PER_SEC (1,000,000,000) is defined as a long literal.
On 32-bit systems like arm32, where long is 32 bits, this value can
overflow when used in 64-bit calculations.

This can lead to incorrect time calculations within the VDSO.

To fix this, define NSEC_PER_SEC as a long long literal (1000000000LL),
ensuring it is treated as a 64-bit value on all architectures and
preventing potential overflows.

Signed-off-by: Wake Liu <wakel@google.com>
---
 include/vdso/time64.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/vdso/time64.h b/include/vdso/time64.h
index b40cfa2aa33c..07641b27d139 100644
--- a/include/vdso/time64.h
+++ b/include/vdso/time64.h
@@ -8,7 +8,7 @@
 #define NSEC_PER_USEC	1000L
 #define NSEC_PER_MSEC	1000000L
 #define USEC_PER_SEC	1000000L
-#define NSEC_PER_SEC	1000000000L
+#define NSEC_PER_SEC	1000000000LL
 #define PSEC_PER_SEC	1000000000000LL
 #define FSEC_PER_SEC	1000000000000000LL
 
-- 
2.50.1.565.gc32cd1483b-goog


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH] vdso: Define NSEC_PER_SEC as 64-bit to prevent overflow
  2025-08-05 16:21 [PATCH] vdso: Define NSEC_PER_SEC as 64-bit to prevent overflow Wake Liu
@ 2025-08-06  8:44 ` Thomas Gleixner
  2025-08-06  8:55 ` Thomas Gleixner
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 9+ messages in thread
From: Thomas Gleixner @ 2025-08-06  8:44 UTC (permalink / raw)
  To: Wake Liu, Andy Lutomirski, Vincenzo Frascino; +Cc: linux-kernel, wakel

On Wed, Aug 06 2025 at 00:21, Wake Liu wrote:

Why are you sending the same patch twice within a minute?


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] vdso: Define NSEC_PER_SEC as 64-bit to prevent overflow
  2025-08-05 16:21 [PATCH] vdso: Define NSEC_PER_SEC as 64-bit to prevent overflow Wake Liu
  2025-08-06  8:44 ` Thomas Gleixner
@ 2025-08-06  8:55 ` Thomas Gleixner
  2025-08-09  9:49   ` Wake Liu
  2025-08-06 20:28 ` kernel test robot
  2025-08-07  7:37 ` kernel test robot
  3 siblings, 1 reply; 9+ messages in thread
From: Thomas Gleixner @ 2025-08-06  8:55 UTC (permalink / raw)
  To: Wake Liu, Andy Lutomirski, Vincenzo Frascino; +Cc: linux-kernel, wakel

On Wed, Aug 06 2025 at 00:21, Wake Liu wrote:
> The constant NSEC_PER_SEC (1,000,000,000) is defined as a long literal.
> On 32-bit systems like arm32, where long is 32 bits, this value can
> overflow when used in 64-bit calculations.

How so? Where is the overflow exactly?

The only usage of NSEC_PER_SEC in the VDSO is:

# git grep NSEC_PER_SEC lib/vdso/
lib/vdso/gettimeofday.c:        ts->tv_sec = sec + __iter_div_u64_rem(ns, NSEC_PER_SEC, &ns);

and __iter_div_u64_rem() is an inline:

static __always_inline u32
__iter_div_u64_rem(u64 dividend, u32 divisor, u64 *remainder)

So how exactly is this causing an overflow?

Also by your description this would be a problem throughout the kernel.

Thanks,

        tglx



^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] vdso: Define NSEC_PER_SEC as 64-bit to prevent overflow
  2025-08-05 16:21 [PATCH] vdso: Define NSEC_PER_SEC as 64-bit to prevent overflow Wake Liu
  2025-08-06  8:44 ` Thomas Gleixner
  2025-08-06  8:55 ` Thomas Gleixner
@ 2025-08-06 20:28 ` kernel test robot
  2025-08-07  7:37 ` kernel test robot
  3 siblings, 0 replies; 9+ messages in thread
From: kernel test robot @ 2025-08-06 20:28 UTC (permalink / raw)
  To: Wake Liu, Andy Lutomirski, Thomas Gleixner, Vincenzo Frascino
  Cc: oe-kbuild-all, linux-kernel, wakel

Hi Wake,

kernel test robot noticed the following build warnings:

[auto build test WARNING on tip/timers/vdso]
[also build test WARNING on linus/master v6.16 next-20250806]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Wake-Liu/vdso-Define-NSEC_PER_SEC-as-64-bit-to-prevent-overflow/20250806-125650
base:   tip/timers/vdso
patch link:    https://lore.kernel.org/r/20250805162153.952693-1-wakel%40google.com
patch subject: [PATCH] vdso: Define NSEC_PER_SEC as 64-bit to prevent overflow
config: powerpc-randconfig-002-20250807 (https://download.01.org/0day-ci/archive/20250807/202508070443.SWaWZTdB-lkp@intel.com/config)
compiler: powerpc-linux-gcc (GCC) 10.5.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250807/202508070443.SWaWZTdB-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202508070443.SWaWZTdB-lkp@intel.com/

All warnings (new ones prefixed by >>):

   arch/powerpc/xmon/xmon.c: In function 'show_uptime':
>> arch/powerpc/xmon/xmon.c:1008:27: warning: format '%lu' expects argument of type 'long unsigned int', but argument 3 has type 'long long int' [-Wformat=]
    1008 |   printf("Uptime: %lu.%.2lu seconds\n", (unsigned long)uptime.tv_sec,
         |                       ~~~~^
         |                           |
         |                           long unsigned int
         |                       %.2llu
    1009 |    ((unsigned long)uptime.tv_nsec / (NSEC_PER_SEC/100)));
         |    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
         |                                   |
         |                                   long long int


vim +1008 arch/powerpc/xmon/xmon.c

^1da177e4c3f41 arch/ppc64/xmon/xmon.c   Linus Torvalds       2005-04-16   996  
59d3391e8cf274 arch/powerpc/xmon/xmon.c Guilherme G. Piccoli 2017-09-18   997  /* Based on uptime_proc_show(). */
59d3391e8cf274 arch/powerpc/xmon/xmon.c Guilherme G. Piccoli 2017-09-18   998  static void
59d3391e8cf274 arch/powerpc/xmon/xmon.c Guilherme G. Piccoli 2017-09-18   999  show_uptime(void)
59d3391e8cf274 arch/powerpc/xmon/xmon.c Guilherme G. Piccoli 2017-09-18  1000  {
f6bd74fa084eb9 arch/powerpc/xmon/xmon.c Arnd Bergmann        2018-06-18  1001  	struct timespec64 uptime;
59d3391e8cf274 arch/powerpc/xmon/xmon.c Guilherme G. Piccoli 2017-09-18  1002  
59d3391e8cf274 arch/powerpc/xmon/xmon.c Guilherme G. Piccoli 2017-09-18  1003  	if (setjmp(bus_error_jmp) == 0) {
59d3391e8cf274 arch/powerpc/xmon/xmon.c Guilherme G. Piccoli 2017-09-18  1004  		catch_memory_errors = 1;
59d3391e8cf274 arch/powerpc/xmon/xmon.c Guilherme G. Piccoli 2017-09-18  1005  		sync();
59d3391e8cf274 arch/powerpc/xmon/xmon.c Guilherme G. Piccoli 2017-09-18  1006  
f6bd74fa084eb9 arch/powerpc/xmon/xmon.c Arnd Bergmann        2018-06-18  1007  		ktime_get_coarse_boottime_ts64(&uptime);
59d3391e8cf274 arch/powerpc/xmon/xmon.c Guilherme G. Piccoli 2017-09-18 @1008  		printf("Uptime: %lu.%.2lu seconds\n", (unsigned long)uptime.tv_sec,
59d3391e8cf274 arch/powerpc/xmon/xmon.c Guilherme G. Piccoli 2017-09-18  1009  			((unsigned long)uptime.tv_nsec / (NSEC_PER_SEC/100)));
59d3391e8cf274 arch/powerpc/xmon/xmon.c Guilherme G. Piccoli 2017-09-18  1010  
59d3391e8cf274 arch/powerpc/xmon/xmon.c Guilherme G. Piccoli 2017-09-18  1011  		sync();
59d3391e8cf274 arch/powerpc/xmon/xmon.c Guilherme G. Piccoli 2017-09-18  1012  		__delay(200);						\
59d3391e8cf274 arch/powerpc/xmon/xmon.c Guilherme G. Piccoli 2017-09-18  1013  	}
59d3391e8cf274 arch/powerpc/xmon/xmon.c Guilherme G. Piccoli 2017-09-18  1014  	catch_memory_errors = 0;
59d3391e8cf274 arch/powerpc/xmon/xmon.c Guilherme G. Piccoli 2017-09-18  1015  }
59d3391e8cf274 arch/powerpc/xmon/xmon.c Guilherme G. Piccoli 2017-09-18  1016  

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] vdso: Define NSEC_PER_SEC as 64-bit to prevent overflow
  2025-08-05 16:21 [PATCH] vdso: Define NSEC_PER_SEC as 64-bit to prevent overflow Wake Liu
                   ` (2 preceding siblings ...)
  2025-08-06 20:28 ` kernel test robot
@ 2025-08-07  7:37 ` kernel test robot
  3 siblings, 0 replies; 9+ messages in thread
From: kernel test robot @ 2025-08-07  7:37 UTC (permalink / raw)
  To: Wake Liu, Andy Lutomirski, Thomas Gleixner, Vincenzo Frascino
  Cc: oe-kbuild-all, linux-kernel, wakel

Hi Wake,

kernel test robot noticed the following build errors:

[auto build test ERROR on tip/timers/vdso]
[also build test ERROR on linus/master v6.16 next-20250806]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Wake-Liu/vdso-Define-NSEC_PER_SEC-as-64-bit-to-prevent-overflow/20250806-125650
base:   tip/timers/vdso
patch link:    https://lore.kernel.org/r/20250805162153.952693-1-wakel%40google.com
patch subject: [PATCH] vdso: Define NSEC_PER_SEC as 64-bit to prevent overflow
config: m68k-randconfig-r064-20250807 (https://download.01.org/0day-ci/archive/20250807/202508071518.qaTdbvoM-lkp@intel.com/config)
compiler: m68k-linux-gcc (GCC) 9.5.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250807/202508071518.qaTdbvoM-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202508071518.qaTdbvoM-lkp@intel.com/

All errors (new ones prefixed by >>, old ones prefixed by <<):

>> ERROR: modpost: "__udivdi3" [drivers/leds/leds-sun50i-a100.ko] undefined!
>> ERROR: modpost: "__divdi3" [drivers/leds/leds-sun50i-a100.ko] undefined!
>> ERROR: modpost: "__divdi3" [drivers/clk/qcom/clk-spmi-pmic-div.ko] undefined!
>> ERROR: modpost: "__udivdi3" [drivers/tty/serial/8250/8250_bcm7271.ko] undefined!
>> ERROR: modpost: "__divdi3" [drivers/tty/serial/8250/8250_bcm7271.ko] undefined!
>> ERROR: modpost: "__udivdi3" [drivers/spi/spi-axi-spi-engine.ko] undefined!
>> ERROR: modpost: "__divdi3" [drivers/spi/spi-axi-spi-engine.ko] undefined!
>> ERROR: modpost: "__divdi3" [drivers/spi/spi-dw.ko] undefined!
>> ERROR: modpost: "__udivdi3" [drivers/spi/spi-dw.ko] undefined!
>> ERROR: modpost: "__udivdi3" [drivers/input/touchscreen/ads7846.ko] undefined!
WARNING: modpost: suppressed 3 unresolved symbol warnings because there were too many)

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] vdso: Define NSEC_PER_SEC as 64-bit to prevent overflow
  2025-08-06  8:55 ` Thomas Gleixner
@ 2025-08-09  9:49   ` Wake Liu
  2025-08-10  8:03     ` Thomas Gleixner
  0 siblings, 1 reply; 9+ messages in thread
From: Wake Liu @ 2025-08-09  9:49 UTC (permalink / raw)
  To: Thomas Gleixner; +Cc: Andy Lutomirski, Vincenzo Frascino, linux-kernel

Hi Thomas,

Here's the command I used in android.

```
/data/nativetest/vts_linux_kselftest_arm_32/set-timer-lat
```
And the strace message.
```
Setting timers for every 1 seconds
CLOCK_REALTIME         ABSTIME PERIODIC timer fired early:       1 : [FAILED]
CLOCK_REALTIME         ABSTIME PERIODIC max latency:     747886 ns : [OK]
CLOCK_REALTIME         RELTIME PERIODIC timer fired early:       0 : [OK]
CLOCK_REALTIME         RELTIME PERIODIC max latency: 4296109428 ns : [FAILED]
CLOCK_REALTIME         ABSTIME ONE-SHOT timer fired early:       0 : [OK]
CLOCK_REALTIME         ABSTIME ONE-SHOT max latency:     715292 ns : [OK]
CLOCK_REALTIME         ABSTIME ONE-SHOT count:                   1 : [OK]
CLOCK_REALTIME         RELTIME ONE-SHOT timer fired early:       0 : [OK]
CLOCK_REALTIME         RELTIME ONE-SHOT max latency:     628703 ns : [OK]
CLOCK_REALTIME         RELTIME ONE-SHOT count:                   1 : [OK]
CLOCK_MONOTONIC        ABSTIME PERIODIC timer fired early:       1 : [FAILED]
CLOCK_MONOTONIC        ABSTIME PERIODIC max latency:     738160 ns : [OK]
CLOCK_MONOTONIC        RELTIME PERIODIC timer fired early:       1 : [FAILED]
CLOCK_MONOTONIC        RELTIME PERIODIC max latency:     708172 ns : [OK]
CLOCK_MONOTONIC        ABSTIME ONE-SHOT timer fired early:       0 : [OK]
CLOCK_MONOTONIC        ABSTIME ONE-SHOT max latency:     586385 ns : [OK]
CLOCK_MONOTONIC        ABSTIME ONE-SHOT count:                   1 : [OK]
CLOCK_MONOTONIC        RELTIME ONE-SHOT timer fired early:       0 : [OK]
CLOCK_MONOTONIC        RELTIME ONE-SHOT max latency:     818237 ns : [OK]
CLOCK_MONOTONIC        RELTIME ONE-SHOT count:                   1 : [OK]
CLOCK_BOOTTIME         ABSTIME PERIODIC timer fired early:       1 : [FAILED]
CLOCK_BOOTTIME         ABSTIME PERIODIC max latency:     795249 ns : [OK]
CLOCK_BOOTTIME         RELTIME PERIODIC timer fired early:       0 : [OK]
CLOCK_BOOTTIME         RELTIME PERIODIC max latency: 4295613660 ns : [FAILED]
CLOCK_BOOTTIME         ABSTIME ONE-SHOT timer fired early:       0 : [OK]
CLOCK_BOOTTIME         ABSTIME ONE-SHOT max latency:     978516 ns : [OK]
CLOCK_BOOTTIME         ABSTIME ONE-SHOT count:                   1 : [OK]
CLOCK_BOOTTIME         RELTIME ONE-SHOT timer fired early:       0 : [OK]
CLOCK_BOOTTIME         RELTIME ONE-SHOT max latency:     628948 ns : [OK]
CLOCK_BOOTTIME         RELTIME ONE-SHOT count:                   1 : [OK]
CLOCK_REALTIME_ALARM   ABSTIME PERIODIC timer fired early:       1 : [FAILED]
CLOCK_REALTIME_ALARM   ABSTIME PERIODIC max latency:    1511436 ns : [OK]
CLOCK_REALTIME_ALARM   RELTIME PERIODIC timer fired early:       1 : [FAILED]
CLOCK_REALTIME_ALARM   RELTIME PERIODIC max latency:     758913 ns : [OK]
CLOCK_REALTIME_ALARM   ABSTIME ONE-SHOT timer fired early:       0 : [OK]
CLOCK_REALTIME_ALARM   ABSTIME ONE-SHOT max latency:     610881 ns : [OK]
CLOCK_REALTIME_ALARM   ABSTIME ONE-SHOT count:                   1 : [OK]
CLOCK_REALTIME_ALARM   RELTIME ONE-SHOT timer fired early:       0 : [OK]
CLOCK_REALTIME_ALARM   RELTIME ONE-SHOT max latency:     633708 ns : [OK]
CLOCK_REALTIME_ALARM   RELTIME ONE-SHOT count:                   1 : [OK]
CLOCK_BOOTTIME_ALARM   ABSTIME PERIODIC timer fired early:       0 : [OK]
CLOCK_BOOTTIME_ALARM   ABSTIME PERIODIC max latency: 4295683321 ns : [FAILED]
CLOCK_BOOTTIME_ALARM   RELTIME PERIODIC timer fired early:       0 : [OK]
CLOCK_BOOTTIME_ALARM   RELTIME PERIODIC max latency:     839520 ns : [OK]
CLOCK_BOOTTIME_ALARM   ABSTIME ONE-SHOT timer fired early:       0 : [OK]
CLOCK_BOOTTIME_ALARM   ABSTIME ONE-SHOT max latency:     601401 ns : [OK]
CLOCK_BOOTTIME_ALARM   ABSTIME ONE-SHOT count:                   1 : [OK]
CLOCK_BOOTTIME_ALARM   RELTIME ONE-SHOT timer fired early:       0 : [OK]
CLOCK_BOOTTIME_ALARM   RELTIME ONE-SHOT max latency:     628175 ns : [OK]
CLOCK_BOOTTIME_ALARM   RELTIME ONE-SHOT count:                   1 : [OK]
CLOCK_TAI              ABSTIME PERIODIC timer fired early:       0 : [OK]
CLOCK_TAI              ABSTIME PERIODIC max latency: 4295577120 ns : [FAILED]
CLOCK_TAI              RELTIME PERIODIC timer fired early:       0 : [OK]
CLOCK_TAI              RELTIME PERIODIC max latency:     921184 ns : [OK]
CLOCK_TAI              ABSTIME ONE-SHOT timer fired early:       0 : [OK]
CLOCK_TAI              ABSTIME ONE-SHOT max latency:     590048 ns : [OK]
CLOCK_TAI              ABSTIME ONE-SHOT count:                   1 : [OK]
CLOCK_TAI              RELTIME ONE-SHOT timer fired early:       0 : [OK]
CLOCK_TAI              RELTIME ONE-SHOT max latency:    1007202 ns : [OK]
CLOCK_TAI              RELTIME ONE-SHOT count:                   1 : [OK]
# Totals: pass:0 fail:0 xfail:0 xpass:0 skip:0 error:0
```

So there is some odd stuff going on in how the time comparisons are
done of 32 bit builds.

Looks like there is some integer overflow on 32 bit builds in the
timespec_sub function:

long long timespec_sub(struct timespec a, struct timespec b)
{
long long ret = NSEC_PER_SEC * b.tv_sec + b.tv_nsec;

ret -= NSEC_PER_SEC * a.tv_sec + a.tv_nsec;
return ret;
}

on 32 bit builds NSEC_PER_SEC and b.tv_sec are only 32 bit values
which I'm guessing is causing the overflow.

NSEC_PER_SEC is defined as 1000000000L but if we change this to
1000000000LL then the test starts passing.



Thomas Gleixner <tglx@linutronix.de> 於 2025年8月6日 週三 下午4:55寫道:
>
> On Wed, Aug 06 2025 at 00:21, Wake Liu wrote:
> > The constant NSEC_PER_SEC (1,000,000,000) is defined as a long literal.
> > On 32-bit systems like arm32, where long is 32 bits, this value can
> > overflow when used in 64-bit calculations.
>
> How so? Where is the overflow exactly?
>
> The only usage of NSEC_PER_SEC in the VDSO is:
>
> # git grep NSEC_PER_SEC lib/vdso/
> lib/vdso/gettimeofday.c:        ts->tv_sec = sec + __iter_div_u64_rem(ns, NSEC_PER_SEC, &ns);
>
> and __iter_div_u64_rem() is an inline:
>
> static __always_inline u32
> __iter_div_u64_rem(u64 dividend, u32 divisor, u64 *remainder)
>
> So how exactly is this causing an overflow?
>
> Also by your description this would be a problem throughout the kernel.
>
> Thanks,
>
>         tglx
>
>


-- 
Best Regards,
Wake Liu

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] vdso: Define NSEC_PER_SEC as 64-bit to prevent overflow
  2025-08-09  9:49   ` Wake Liu
@ 2025-08-10  8:03     ` Thomas Gleixner
  2025-08-10  8:28       ` Thomas Weißschuh
  0 siblings, 1 reply; 9+ messages in thread
From: Thomas Gleixner @ 2025-08-10  8:03 UTC (permalink / raw)
  To: Wake Liu; +Cc: Andy Lutomirski, Vincenzo Frascino, linux-kernel

On Sat, Aug 09 2025 at 17:49, Wake Liu wrote:

Please do not top post and trim your replies.

> CLOCK_REALTIME         ABSTIME PERIODIC timer fired early:       1 : [FAILED]
> Looks like there is some integer overflow on 32 bit builds in the
> timespec_sub function:
>
> long long timespec_sub(struct timespec a, struct timespec b)

That's in selftests and not in the kernel VDSO code.

Can you please be precise with your subject and changelog? You claimed:

 "This can lead to incorrect time calculations within the VDSO"

No, it can't not because the VDSO does not use it that way.

> {
> long long ret = NSEC_PER_SEC * b.tv_sec + b.tv_nsec;
>
> ret -= NSEC_PER_SEC * a.tv_sec + a.tv_nsec;
> return ret;
> }
>
> on 32 bit builds NSEC_PER_SEC and b.tv_sec are only 32 bit values
> which I'm guessing is causing the overflow.

Guessing is not a valid technical problem solving approach.

> NSEC_PER_SEC is defined as 1000000000L but if we change this to
> 1000000000LL then the test starts passing.

Yes, that function bogus on 32bit.

There are at least ten different ways to implement this function 32-bit
safe, but you picked the worst option of all, which breaks the kernel
build as you got told by 0-day.

I'm sure you can figure out how to ensure that the multiplication is
actually expanding to 64-bit or use some other approach to calculate
that delta.

And please make that an inline function and put it into a header file in
the timer selftest directory, and replace _all_ copies of it in the
various timer tests.

Thanks,

        tglx

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] vdso: Define NSEC_PER_SEC as 64-bit to prevent overflow
  2025-08-10  8:03     ` Thomas Gleixner
@ 2025-08-10  8:28       ` Thomas Weißschuh
  0 siblings, 0 replies; 9+ messages in thread
From: Thomas Weißschuh @ 2025-08-10  8:28 UTC (permalink / raw)
  To: Wake Liu
  Cc: Thomas Gleixner, Andy Lutomirski, Vincenzo Frascino, linux-kernel

Hi Wake,

On 2025-08-10 10:03:19+0200, Thomas Gleixner wrote:
> On Sat, Aug 09 2025 at 17:49, Wake Liu wrote:
> > long long timespec_sub(struct timespec a, struct timespec b)
> > {
> > long long ret = NSEC_PER_SEC * b.tv_sec + b.tv_nsec;
> >
> > ret -= NSEC_PER_SEC * a.tv_sec + a.tv_nsec;
> > return ret;
> > }

(...)

> And please make that an inline function and put it into a header file in
> the timer selftest directory, and replace _all_ copies of it in the
> various timer tests.

When you create that header file, please add a dedicated definition
of NSEC_PER_SEC to it. The selftests shouldn't be using internal kernel
headers in the first place. Especially not for a single, trivial constant.


Thomas

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2025-08-10  8:28 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-08-05 16:21 [PATCH] vdso: Define NSEC_PER_SEC as 64-bit to prevent overflow Wake Liu
2025-08-06  8:44 ` Thomas Gleixner
2025-08-06  8:55 ` Thomas Gleixner
2025-08-09  9:49   ` Wake Liu
2025-08-10  8:03     ` Thomas Gleixner
2025-08-10  8:28       ` Thomas Weißschuh
2025-08-06 20:28 ` kernel test robot
2025-08-07  7:37 ` kernel test robot
  -- strict thread matches above, loose matches on Subject: below --
2025-08-05 16:20 Wake Liu

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).