All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] posix-cpu-timers: use u64 multiplication in update_rlimit_cpu()
@ 2026-06-16 11:20 Zhan Xusheng
  2026-06-16 12:53 ` Ingo Molnar
  2026-06-17 16:04 ` [tip: timers/urgent] posix-cpu-timers: Use " tip-bot2 for Zhan Xusheng
  0 siblings, 2 replies; 6+ messages in thread
From: Zhan Xusheng @ 2026-06-16 11:20 UTC (permalink / raw)
  To: Thomas Gleixner
  Cc: Anna-Maria Behnsen, Frederic Weisbecker, Ingo Molnar,
	linux-kernel, Zhan Xusheng

update_rlimit_cpu() converts the RLIMIT_CPU value to nanoseconds with

        u64 nsecs = rlim_new * NSEC_PER_SEC;

On 32-bit kernels both rlim_new (unsigned long) and NSEC_PER_SEC
(1000000000L) are 32-bit, so the multiplication is performed in
unsigned long and truncated for rlim_new > 4 before being widened to
u64.

The same file already casts to u64 for the matching computation in
check_process_timers():

        u64 softns = (u64)soft * NSEC_PER_SEC;

As a result, the truncated value is installed into the CPUCLOCK_PROF
expiry cache (nextevt), causing the process CPU timer to be programmed
to fire prematurely for any RLIMIT_CPU soft limit >= 5 seconds. The
actual SIGXCPU/SIGKILL decision in check_process_timers() already casts
to u64 and is therefore correct, so limit enforcement is not broken;
only the expiry-cache programming is wrong. Apply the same cast here so
both paths convert rlim_cur identically.

64-bit kernels are unaffected.

Fixes: 858cf3a8c599 ("timers/itimer: Convert internal cputime_t units to nsec")
Signed-off-by: Zhan Xusheng <zhanxusheng@xiaomi.com>
---
 kernel/time/posix-cpu-timers.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/time/posix-cpu-timers.c b/kernel/time/posix-cpu-timers.c
index 74775b94d11b..5e633d8750d1 100644
--- a/kernel/time/posix-cpu-timers.c
+++ b/kernel/time/posix-cpu-timers.c
@@ -41,7 +41,7 @@ void posix_cputimers_group_init(struct posix_cputimers *pct, u64 cpu_limit)
  */
 int update_rlimit_cpu(struct task_struct *task, unsigned long rlim_new)
 {
-	u64 nsecs = rlim_new * NSEC_PER_SEC;
+	u64 nsecs = (u64)rlim_new * NSEC_PER_SEC;
 	unsigned long irq_fl;
 
 	if (!lock_task_sighand(task, &irq_fl))
-- 
2.43.0


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

* Re: [PATCH] posix-cpu-timers: use u64 multiplication in update_rlimit_cpu()
  2026-06-16 11:20 [PATCH] posix-cpu-timers: use u64 multiplication in update_rlimit_cpu() Zhan Xusheng
@ 2026-06-16 12:53 ` Ingo Molnar
  2026-06-17 15:07   ` Thomas Gleixner
  2026-06-17 16:04 ` [tip: timers/urgent] posix-cpu-timers: Use " tip-bot2 for Zhan Xusheng
  1 sibling, 1 reply; 6+ messages in thread
From: Ingo Molnar @ 2026-06-16 12:53 UTC (permalink / raw)
  To: Zhan Xusheng
  Cc: Thomas Gleixner, Anna-Maria Behnsen, Frederic Weisbecker,
	linux-kernel, Zhan Xusheng


* Zhan Xusheng <zhanxusheng1024@gmail.com> wrote:

> update_rlimit_cpu() converts the RLIMIT_CPU value to nanoseconds with
> 
>         u64 nsecs = rlim_new * NSEC_PER_SEC;
> 
> On 32-bit kernels both rlim_new (unsigned long) and NSEC_PER_SEC
> (1000000000L) are 32-bit, so the multiplication is performed in
> unsigned long and truncated for rlim_new > 4 before being widened to
> u64.
> 
> The same file already casts to u64 for the matching computation in
> check_process_timers():
> 
>         u64 softns = (u64)soft * NSEC_PER_SEC;
> 
> As a result, the truncated value is installed into the CPUCLOCK_PROF
> expiry cache (nextevt), causing the process CPU timer to be programmed
> to fire prematurely for any RLIMIT_CPU soft limit >= 5 seconds. The
> actual SIGXCPU/SIGKILL decision in check_process_timers() already casts
> to u64 and is therefore correct, so limit enforcement is not broken;
> only the expiry-cache programming is wrong. Apply the same cast here so
> both paths convert rlim_cur identically.
> 
> 64-bit kernels are unaffected.
> 
> Fixes: 858cf3a8c599 ("timers/itimer: Convert internal cputime_t units to nsec")
> Signed-off-by: Zhan Xusheng <zhanxusheng@xiaomi.com>
> ---
>  kernel/time/posix-cpu-timers.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/kernel/time/posix-cpu-timers.c b/kernel/time/posix-cpu-timers.c
> index 74775b94d11b..5e633d8750d1 100644
> --- a/kernel/time/posix-cpu-timers.c
> +++ b/kernel/time/posix-cpu-timers.c
> @@ -41,7 +41,7 @@ void posix_cputimers_group_init(struct posix_cputimers *pct, u64 cpu_limit)
>   */
>  int update_rlimit_cpu(struct task_struct *task, unsigned long rlim_new)
>  {
> -	u64 nsecs = rlim_new * NSEC_PER_SEC;
> +	u64 nsecs = (u64)rlim_new * NSEC_PER_SEC;

Wouldn't it be more robust to define NSEC_PER_SEC not
as 1000000000L, but 1000000000LL? (Together with sorting
out the inevitable side effects.)

Ie. in any logic that uses NSEC_PER_SEC multiplication we
should default to 64-bit arithmetics and not silent
truncation to 32-bit width.

With that we could remove a number of '(u64)' forced
type conversions as well.

Thanks,

	Ingo

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

* Re: [PATCH] posix-cpu-timers: use u64 multiplication in update_rlimit_cpu()
       [not found] <REPLACE-WITH-INGO-MESSAGE-ID>
@ 2026-06-16 15:43 ` Zhan Xusheng
  0 siblings, 0 replies; 6+ messages in thread
From: Zhan Xusheng @ 2026-06-16 15:43 UTC (permalink / raw)
  To: Ingo Molnar; +Cc: Thomas Gleixner, linux-kernel, Zhan Xusheng

From: Zhan Xusheng <zhanxusheng1024@gmail.com>

* Ingo Molnar <mingo@kernel.org> wrote:
> Wouldn't it be more robust to define NSEC_PER_SEC not as 1000000000L,
> but 1000000000LL? (Together with sorting out the inevitable side
> effects.) Ie. in any logic that uses NSEC_PER_SEC multiplication we
> should default to 64-bit arithmetics and not silent truncation to
> 32-bit width. With that we could remove a number of '(u64)' forced
> type conversions as well.

Agreed, that would be a more robust long-term solution.

I tried it locally and did an i386 allmodconfig build to see what falls
out.

The most interesting thing it immediately exposes is a genuine 32-bit
truncation bug: drivers/scsi/lpfc uses

    60 * NSEC_PER_SEC

in an unsigned long, which should be 60000000000 but is silently
truncated to 4165425152 on 32-bit today.

Beyond that, switching NSEC_PER_SEC to LL falls into three classes:

 - ~7 -Wformat fixups where %lu/%ld now receive long long, e.g.
   fs/proc/uptime.c, drivers/ata/libata-transport.c,
   drivers/scsi/scsi_debug.c, i915 backlight, meson-ir-tx.

 - the lpfc-style constant truncation above.

 - the bulk: ~79 files with expressions like x * NSEC_PER_SEC / var.
   These currently stay in 32-bit arithmetic (and silently truncate
   when the product exceeds 32 bits); with LL they become 64-bit
   divisions and pull in __divdi3/__udivdi3, i.e. a link failure on
   32-bit until converted to div_u64()/do_div(). They span drivers/iio,
   spi, pwm, net, mtd, gpu, ... plus a few core files (kernel/time,
   kernel/events, net/core). (i386 only here; arm would add
   __aeabi_ldivmod and possibly more.)

So it's really a multi-subsystem cleanup series rather than a single
macro change. My inclination would be to fix the affected call sites
first (including the truncation cases), then flip NSEC_PER_SEC to LL
once the tree builds cleanly on 32-bit.

The same likely applies to the sibling constants (USEC_PER_SEC,
NSEC_PER_MSEC, ...), so it may be worth treating as a broader cleanup.

Thanks,
  Zhan

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

* Re: [PATCH] posix-cpu-timers: use u64 multiplication in update_rlimit_cpu()
  2026-06-16 12:53 ` Ingo Molnar
@ 2026-06-17 15:07   ` Thomas Gleixner
  2026-06-17 21:50     ` David Laight
  0 siblings, 1 reply; 6+ messages in thread
From: Thomas Gleixner @ 2026-06-17 15:07 UTC (permalink / raw)
  To: Ingo Molnar, Zhan Xusheng
  Cc: Anna-Maria Behnsen, Frederic Weisbecker, linux-kernel,
	Zhan Xusheng

On Tue, Jun 16 2026 at 14:53, Ingo Molnar wrote:
> * Zhan Xusheng <zhanxusheng1024@gmail.com> wrote:
>
>> update_rlimit_cpu() converts the RLIMIT_CPU value to nanoseconds with
>> 
>>         u64 nsecs = rlim_new * NSEC_PER_SEC;
>> 
>> On 32-bit kernels both rlim_new (unsigned long) and NSEC_PER_SEC
>> (1000000000L) are 32-bit, so the multiplication is performed in
>> unsigned long and truncated for rlim_new > 4 before being widened to
>> u64.
>> 
>> The same file already casts to u64 for the matching computation in
>> check_process_timers():
>> 
>>         u64 softns = (u64)soft * NSEC_PER_SEC;
>> 
>> As a result, the truncated value is installed into the CPUCLOCK_PROF
>> expiry cache (nextevt), causing the process CPU timer to be programmed
>> to fire prematurely for any RLIMIT_CPU soft limit >= 5 seconds. The
>> actual SIGXCPU/SIGKILL decision in check_process_timers() already casts
>> to u64 and is therefore correct, so limit enforcement is not broken;
>> only the expiry-cache programming is wrong. Apply the same cast here so
>> both paths convert rlim_cur identically.
>> 
>> 64-bit kernels are unaffected.
>> 
>> Fixes: 858cf3a8c599 ("timers/itimer: Convert internal cputime_t units to nsec")
>> Signed-off-by: Zhan Xusheng <zhanxusheng@xiaomi.com>
>> ---
>>  kernel/time/posix-cpu-timers.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>> 
>> diff --git a/kernel/time/posix-cpu-timers.c b/kernel/time/posix-cpu-timers.c
>> index 74775b94d11b..5e633d8750d1 100644
>> --- a/kernel/time/posix-cpu-timers.c
>> +++ b/kernel/time/posix-cpu-timers.c
>> @@ -41,7 +41,7 @@ void posix_cputimers_group_init(struct posix_cputimers *pct, u64 cpu_limit)
>>   */
>>  int update_rlimit_cpu(struct task_struct *task, unsigned long rlim_new)
>>  {
>> -	u64 nsecs = rlim_new * NSEC_PER_SEC;
>> +	u64 nsecs = (u64)rlim_new * NSEC_PER_SEC;
>
> Wouldn't it be more robust to define NSEC_PER_SEC not
> as 1000000000L, but 1000000000LL? (Together with sorting
> out the inevitable side effects.)

There are a lot of them on 32bit builds. IIRC correctly there was an
attempt to do exactly that a few years ago and it didn't go well.

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

* [tip: timers/urgent] posix-cpu-timers: Use u64 multiplication in update_rlimit_cpu()
  2026-06-16 11:20 [PATCH] posix-cpu-timers: use u64 multiplication in update_rlimit_cpu() Zhan Xusheng
  2026-06-16 12:53 ` Ingo Molnar
@ 2026-06-17 16:04 ` tip-bot2 for Zhan Xusheng
  1 sibling, 0 replies; 6+ messages in thread
From: tip-bot2 for Zhan Xusheng @ 2026-06-17 16:04 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: Zhan Xusheng, Thomas Gleixner, stable, x86, linux-kernel

The following commit has been merged into the timers/urgent branch of tip:

Commit-ID:     26aff38fefb1d6cd87e22525f41cc8f1aa61b24f
Gitweb:        https://git.kernel.org/tip/26aff38fefb1d6cd87e22525f41cc8f1aa61b24f
Author:        Zhan Xusheng <zhanxusheng1024@gmail.com>
AuthorDate:    Tue, 16 Jun 2026 19:20:17 +08:00
Committer:     Thomas Gleixner <tglx@kernel.org>
CommitterDate: Wed, 17 Jun 2026 17:58:34 +02:00

posix-cpu-timers: Use u64 multiplication in update_rlimit_cpu()

update_rlimit_cpu() converts the RLIMIT_CPU value to nanoseconds with

        u64 nsecs = rlim_new * NSEC_PER_SEC;

On 32-bit kernels both rlim_new (unsigned long) and NSEC_PER_SEC
(1000000000L) are 32-bit, so the multiplication is performed in unsigned
long and truncated for rlim_new > 4 seconds before being widened to u64.

The same file already casts to u64 for the matching computation in
check_process_timers():

        u64 softns = (u64)soft * NSEC_PER_SEC;

As a result, the truncated value is installed into the CPUCLOCK_PROF
expiry cache (nextevt), causing the process CPU timer to be programmed
to fire prematurely for any RLIMIT_CPU soft limit >= 5 seconds. The
actual SIGXCPU/SIGKILL decision in check_process_timers() already casts
to u64 and is therefore correct, so limit enforcement is not broken;
only the expiry-cache programming is wrong. Apply the same cast here so
both paths convert rlim_cur identically.

64-bit kernels are unaffected.

Fixes: 858cf3a8c599 ("timers/itimer: Convert internal cputime_t units to nsec")
Signed-off-by: Zhan Xusheng <zhanxusheng@xiaomi.com>
Signed-off-by: Thomas Gleixner <tglx@kernel.org>
Cc: stable@vger.kernel.org
Link: https://patch.msgid.link/20260616112017.1681372-1-zhanxusheng@xiaomi.com
---
 kernel/time/posix-cpu-timers.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/time/posix-cpu-timers.c b/kernel/time/posix-cpu-timers.c
index 74775b9..5e633d8 100644
--- a/kernel/time/posix-cpu-timers.c
+++ b/kernel/time/posix-cpu-timers.c
@@ -41,7 +41,7 @@ void posix_cputimers_group_init(struct posix_cputimers *pct, u64 cpu_limit)
  */
 int update_rlimit_cpu(struct task_struct *task, unsigned long rlim_new)
 {
-	u64 nsecs = rlim_new * NSEC_PER_SEC;
+	u64 nsecs = (u64)rlim_new * NSEC_PER_SEC;
 	unsigned long irq_fl;
 
 	if (!lock_task_sighand(task, &irq_fl))

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

* Re: [PATCH] posix-cpu-timers: use u64 multiplication in update_rlimit_cpu()
  2026-06-17 15:07   ` Thomas Gleixner
@ 2026-06-17 21:50     ` David Laight
  0 siblings, 0 replies; 6+ messages in thread
From: David Laight @ 2026-06-17 21:50 UTC (permalink / raw)
  To: Thomas Gleixner
  Cc: Ingo Molnar, Zhan Xusheng, Anna-Maria Behnsen,
	Frederic Weisbecker, linux-kernel, Zhan Xusheng

On Wed, 17 Jun 2026 17:07:28 +0200
Thomas Gleixner <tglx@kernel.org> wrote:

> On Tue, Jun 16 2026 at 14:53, Ingo Molnar wrote:
> > * Zhan Xusheng <zhanxusheng1024@gmail.com> wrote:
> >  
> >> update_rlimit_cpu() converts the RLIMIT_CPU value to nanoseconds with
> >> 
> >>         u64 nsecs = rlim_new * NSEC_PER_SEC;
> >> 
> >> On 32-bit kernels both rlim_new (unsigned long) and NSEC_PER_SEC
> >> (1000000000L) are 32-bit, so the multiplication is performed in
> >> unsigned long and truncated for rlim_new > 4 before being widened to
> >> u64.
> >> 
> >> The same file already casts to u64 for the matching computation in
> >> check_process_timers():
> >> 
> >>         u64 softns = (u64)soft * NSEC_PER_SEC;
> >> 
> >> As a result, the truncated value is installed into the CPUCLOCK_PROF
> >> expiry cache (nextevt), causing the process CPU timer to be programmed
> >> to fire prematurely for any RLIMIT_CPU soft limit >= 5 seconds. The
> >> actual SIGXCPU/SIGKILL decision in check_process_timers() already casts
> >> to u64 and is therefore correct, so limit enforcement is not broken;
> >> only the expiry-cache programming is wrong. Apply the same cast here so
> >> both paths convert rlim_cur identically.
> >> 
> >> 64-bit kernels are unaffected.
> >> 
> >> Fixes: 858cf3a8c599 ("timers/itimer: Convert internal cputime_t units to nsec")
> >> Signed-off-by: Zhan Xusheng <zhanxusheng@xiaomi.com>
> >> ---
> >>  kernel/time/posix-cpu-timers.c | 2 +-
> >>  1 file changed, 1 insertion(+), 1 deletion(-)
> >> 
> >> diff --git a/kernel/time/posix-cpu-timers.c b/kernel/time/posix-cpu-timers.c
> >> index 74775b94d11b..5e633d8750d1 100644
> >> --- a/kernel/time/posix-cpu-timers.c
> >> +++ b/kernel/time/posix-cpu-timers.c
> >> @@ -41,7 +41,7 @@ void posix_cputimers_group_init(struct posix_cputimers *pct, u64 cpu_limit)
> >>   */
> >>  int update_rlimit_cpu(struct task_struct *task, unsigned long rlim_new)
> >>  {
> >> -	u64 nsecs = rlim_new * NSEC_PER_SEC;
> >> +	u64 nsecs = (u64)rlim_new * NSEC_PER_SEC;  
> >
> > Wouldn't it be more robust to define NSEC_PER_SEC not
> > as 1000000000L, but 1000000000LL? (Together with sorting
> > out the inevitable side effects.)  
> 
> There are a lot of them on 32bit builds. IIRC correctly there was an
> attempt to do exactly that a few years ago and it didn't go well.
> 

If the result is assigned to a 32bit variable (and must be max 4 seconds)
the the compiler shouldn't bother to calculate the high 32 bits.

But, thinking, if you want the 64bit product of two 32bit values you
are much better off using an asm block.
clang is not to bad, but gcc has a nasty habit of zero-extending both
values and doing a full 64x64 multiply.
x86 then runs out of registers and spills everything to stack.
When I was doing the 64x64/64 code I found it both spilling constant
zeros and doing multiplies by constant zero.

Thinks more, maybe it is ok when the constant is small but u64.
(Too late in the day to check)

Possibly a SECS_TO_NSECS(seconds) that generates a 64bit product from
the two 32bit values might be the best solution.
After all you are pretty much going to need 64bits for the result.

	David

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

end of thread, other threads:[~2026-06-17 21:50 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-06-16 11:20 [PATCH] posix-cpu-timers: use u64 multiplication in update_rlimit_cpu() Zhan Xusheng
2026-06-16 12:53 ` Ingo Molnar
2026-06-17 15:07   ` Thomas Gleixner
2026-06-17 21:50     ` David Laight
2026-06-17 16:04 ` [tip: timers/urgent] posix-cpu-timers: Use " tip-bot2 for Zhan Xusheng
     [not found] <REPLACE-WITH-INGO-MESSAGE-ID>
2026-06-16 15:43 ` [PATCH] posix-cpu-timers: use " Zhan Xusheng

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.