mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH v2 0/2] sysctl: Two jiffies converter regressions from 2dc164a48e6f
@ 2026-09-28  2:02 Zhan Xusheng
  2026-09-28  2:02 ` [PATCH v2 1/2] sysctl: Negate before converting in the int read path Zhan Xusheng
  2026-09-28  2:02 ` [PATCH v2 2/2] time/jiffies: Saturate in mult_hz() instead of wrapping Zhan Xusheng
  0 siblings, 2 replies; 4+ messages in thread
From: Zhan Xusheng @ 2026-09-28  2:02 UTC (permalink / raw)
  To: Joel Granados
  Cc: Zhan Xusheng, Kees Cook, Kuniyuki Iwashima, Bradley Morgan,
	linux-kernel, linux-fsdevel, stable

Yes, the fixes; the tests are already in sysctl-next.

Only the changelogs changed.  1/2 now also names
proc_dointvec_userhz_jiffies(), and 2/2 labels its reproducer with the
CONFIG_HZ it needs.  The code is identical to v1.

Based on v7.3-rc4+ (62f4c998b297); applies to sysctl-fixes as well.

Link to v1: https://lore.kernel.org/r/20260922031229.2300283-1-zhanxusheng@xiaomi.com

Zhan Xusheng (2):
  sysctl: Negate before converting in the int read path
  time/jiffies: Saturate in mult_hz() instead of wrapping

 kernel/sysctl.c       | 2 +-
 kernel/time/jiffies.c | 2 ++
 2 files changed, 3 insertions(+), 1 deletion(-)


base-commit: 62f4c998b297cf233997a2b4cd6fc2d2df0319c9
-- 
2.43.0


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

* [PATCH v2 1/2] sysctl: Negate before converting in the int read path
  2026-09-28  2:02 [PATCH v2 0/2] sysctl: Two jiffies converter regressions from 2dc164a48e6f Zhan Xusheng
@ 2026-09-28  2:02 ` Zhan Xusheng
  2026-09-28  2:02 ` [PATCH v2 2/2] time/jiffies: Saturate in mult_hz() instead of wrapping Zhan Xusheng
  1 sibling, 0 replies; 4+ messages in thread
From: Zhan Xusheng @ 2026-09-28  2:02 UTC (permalink / raw)
  To: Joel Granados
  Cc: Zhan Xusheng, Kees Cook, Kuniyuki Iwashima, Bradley Morgan,
	linux-kernel, linux-fsdevel, stable

proc_int_k2u_conv_kop() reports the sign through *negp and the magnitude
through *u_ptr, but for a negative value it hands the sign-extended int to
the converter and negates the result:

	*u_ptr = k_ptr_op ? -k_ptr_op((ulong)val) : -(ulong)val;

With div_hz() and CONFIG_HZ=1000 a stored -1000 becomes
(ulong)-1000 / 1000 == 18446744073709550, and negating that wraps:

	# echo -1 > /proc/sys/net/ipv4/tcp_fin_timeout
	# cat /proc/sys/net/ipv4/tcp_fin_timeout
	-18428297329635842066

The magnitude is HZ dependent but the wrap is not: the negation happens
after the division at every CONFIG_HZ.

Take the magnitude first and convert that, which is what the open-coded
version did before commit 2dc164a48e6f ("sysctl: Create converter
functions with two new macros") folded it into a macro.  The
k_ptr_op == NULL branch was already correct.

All three int converters that pass a k_ptr_op are affected:
proc_dointvec_jiffies(), proc_dointvec_userhz_jiffies() and
proc_dointvec_ms_jiffies().

Fixes: 2dc164a48e6f ("sysctl: Create converter functions with two new macros")
Cc: stable@vger.kernel.org
Reviewed-by: Bradley Morgan <brads@mainlining.org>
Signed-off-by: Zhan Xusheng <zhanxusheng@xiaomi.com>
---
 kernel/sysctl.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/sysctl.c b/kernel/sysctl.c
index f7b75985d542..38597f26b34c 100644
--- a/kernel/sysctl.c
+++ b/kernel/sysctl.c
@@ -483,7 +483,7 @@ int proc_int_k2u_conv_kop(ulong *u_ptr, const int *k_ptr, bool *negp,
 
 	if (val < 0) {
 		*negp = true;
-		*u_ptr = k_ptr_op ? -k_ptr_op((ulong)val) : -(ulong)val;
+		*u_ptr = k_ptr_op ? k_ptr_op(-(ulong)val) : -(ulong)val;
 	} else {
 		*negp = false;
 		*u_ptr = k_ptr_op ? k_ptr_op((ulong)val) : (ulong) val;
-- 
2.43.0


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

* [PATCH v2 2/2] time/jiffies: Saturate in mult_hz() instead of wrapping
  2026-09-28  2:02 [PATCH v2 0/2] sysctl: Two jiffies converter regressions from 2dc164a48e6f Zhan Xusheng
  2026-09-28  2:02 ` [PATCH v2 1/2] sysctl: Negate before converting in the int read path Zhan Xusheng
@ 2026-09-28  2:02 ` Zhan Xusheng
  2026-09-29  8:11   ` Joel Granados
  1 sibling, 1 reply; 4+ messages in thread
From: Zhan Xusheng @ 2026-09-28  2:02 UTC (permalink / raw)
  To: Joel Granados
  Cc: Zhan Xusheng, Kees Cook, Kuniyuki Iwashima, Bradley Morgan,
	linux-kernel, linux-fsdevel, stable

mult_hz() converts a user-supplied seconds value to jiffies for
proc_dointvec_jiffies().  proc_int_u2k_conv_uop() rejects a result above
INT_MAX, but it inspects the product, so a product that wraps arrives as a
small value and is stored.

The input has to exceed ULONG_MAX / HZ for the product to wrap, so the
value below is specific to CONFIG_HZ=1000:

	# echo 18446744073709552 > /proc/sys/net/ipv4/tcp_keepalive_time
	# cat /proc/sys/net/ipv4/tcp_keepalive_time
	0

18446744073709551, one less, is correctly rejected.  Dozens of sysctls
use proc_dointvec_jiffies(), among them tcp_keepalive_time,
tcp_fin_timeout and the conntrack timeouts.

Bound the input in the shape clock_t_to_jiffies() already uses and leave
the INT_MAX policy to the caller.  The bound was open-coded as
"*lvalp > INT_MAX / HZ" until commit 2dc164a48e6f ("sysctl: Create
converter functions with two new macros").

Fixes: 2dc164a48e6f ("sysctl: Create converter functions with two new macros")
Cc: stable@vger.kernel.org
Signed-off-by: Zhan Xusheng <zhanxusheng@xiaomi.com>
---
 kernel/time/jiffies.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/kernel/time/jiffies.c b/kernel/time/jiffies.c
index 80c354811538..9b3487d40cd6 100644
--- a/kernel/time/jiffies.c
+++ b/kernel/time/jiffies.c
@@ -101,6 +101,8 @@ void __init register_refined_jiffies(long cycles_per_second)
 #ifdef CONFIG_SYSCTL
 static ulong mult_hz(const ulong val)
 {
+	if (val >= ULONG_MAX / HZ)
+		return ULONG_MAX;
 	return val * HZ;
 }
 
-- 
2.43.0


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

* Re: [PATCH v2 2/2] time/jiffies: Saturate in mult_hz() instead of wrapping
  2026-09-28  2:02 ` [PATCH v2 2/2] time/jiffies: Saturate in mult_hz() instead of wrapping Zhan Xusheng
@ 2026-09-29  8:11   ` Joel Granados
  0 siblings, 0 replies; 4+ messages in thread
From: Joel Granados @ 2026-09-29  8:11 UTC (permalink / raw)
  To: Zhan Xusheng
  Cc: Zhan Xusheng, Kees Cook, Kuniyuki Iwashima, Bradley Morgan,
	linux-kernel, linux-fsdevel, stable

[-- Attachment #1: Type: text/plain, Size: 2558 bytes --]

On Mon, Sep 28, 2026 at 10:02:05AM +0800, Zhan Xusheng wrote:
> mult_hz() converts a user-supplied seconds value to jiffies for
> proc_dointvec_jiffies().  proc_int_u2k_conv_uop() rejects a result above
> INT_MAX, but it inspects the product, so a product that wraps arrives as a
> small value and is stored.
> 
> The input has to exceed ULONG_MAX / HZ for the product to wrap, so the
> value below is specific to CONFIG_HZ=1000:
> 
> 	# echo 18446744073709552 > /proc/sys/net/ipv4/tcp_keepalive_time
> 	# cat /proc/sys/net/ipv4/tcp_keepalive_time
> 	0
> 
> 18446744073709551, one less, is correctly rejected.  Dozens of sysctls
> use proc_dointvec_jiffies(), among them tcp_keepalive_time,
> tcp_fin_timeout and the conntrack timeouts.
> 
> Bound the input in the shape clock_t_to_jiffies() already uses and leave
> the INT_MAX policy to the caller.  The bound was open-coded as
> "*lvalp > INT_MAX / HZ" until commit 2dc164a48e6f ("sysctl: Create
Not sure where this came from, but it is not in 2dc164a48e6f.

> converter functions with two new macros").
> 
> Fixes: 2dc164a48e6f ("sysctl: Create converter functions with two new macros")
> Cc: stable@vger.kernel.org
> Signed-off-by: Zhan Xusheng <zhanxusheng@xiaomi.com>
> ---
>  kernel/time/jiffies.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/kernel/time/jiffies.c b/kernel/time/jiffies.c
> index 80c354811538..9b3487d40cd6 100644
> --- a/kernel/time/jiffies.c
> +++ b/kernel/time/jiffies.c
> @@ -101,6 +101,8 @@ void __init register_refined_jiffies(long cycles_per_second)
>  #ifdef CONFIG_SYSCTL
>  static ulong mult_hz(const ulong val)
>  {
> +	if (val >= ULONG_MAX / HZ)
> +		return ULONG_MAX;
>  	return val * HZ;
>  }
>  
> -- 
> 2.43.0
> 

I'm going to push the fix as is with a modified commit :

  time/jiffies: Saturate in mult_hz() instead of wrapping

  Return ULONG_MAX for values that don't fit an unsigned long.
  proc_int_u2k_conv_uop() now correctly rejects a result above INT_MAX.

  This is the erroneous behaviour that is being fixed. The input has to
  exceed ULONG_MAX / HZ for the product to wrap, so the value below is
  specific to CONFIG_HZ=1000:

    # echo 18446744073709552 > /proc/sys/net/ipv4/tcp_keepalive_time
    # cat /proc/sys/net/ipv4/tcp_keepalive_time
    0

  That value is now rejected with an error.

  The original bound ("*u_ptr > INT_MAX / HZ") was removed in commit
  2dc164a48e6f ("sysctl: Create converter functions with two new macros").


Thx for the patch

Best

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

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

end of thread, other threads:[~2026-09-29  8:11 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-09-28  2:02 [PATCH v2 0/2] sysctl: Two jiffies converter regressions from 2dc164a48e6f Zhan Xusheng
2026-09-28  2:02 ` [PATCH v2 1/2] sysctl: Negate before converting in the int read path Zhan Xusheng
2026-09-28  2:02 ` [PATCH v2 2/2] time/jiffies: Saturate in mult_hz() instead of wrapping Zhan Xusheng
2026-09-29  8:11   ` Joel Granados

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®