Linux filesystem development
 help / color / mirror / Atom feed
* [PATCH 0/2] sysctl: Restore minmax check for proc_do{int,ulong}vec_ms_jiffies_minmax().
@ 2026-09-05 23:36 Kuniyuki Iwashima
  2026-09-05 23:36 ` [PATCH 1/2] sysctl: Make proc_dointvec_ms_jiffies_minmax() enforce minmax again Kuniyuki Iwashima
  2026-09-05 23:36 ` [PATCH 2/2] sysctl: Make proc_doulongvec_ms_jiffies_minmax() " Kuniyuki Iwashima
  0 siblings, 2 replies; 13+ messages in thread
From: Kuniyuki Iwashima @ 2026-09-05 23:36 UTC (permalink / raw)
  To: Kees Cook, Joel Granados, Thomas Gleixner
  Cc: Kuniyuki Iwashima, Kuniyuki Iwashima, linux-fsdevel

The recent refactoring accidentally removed the minmax
check from proc_dointvec_ms_jiffies_minmax() and
proc_doulongvec_ms_jiffies_minmax().

The series restores the validation, and patch 2 adds
the INT_MAX limit for proc_doulongvec_ms_jiffies_minmax()
to avoid silent truncation.


Kuniyuki Iwashima (2):
  sysctl: Make proc_dointvec_ms_jiffies_minmax() enforce minmax again.
  sysctl: Make proc_doulongvec_ms_jiffies_minmax() enforce minmax again.

 kernel/time/jiffies.c | 17 ++++++++++-------
 1 file changed, 10 insertions(+), 7 deletions(-)

-- 
2.55.0.1003.g10538fe699-goog


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

* [PATCH 1/2] sysctl: Make proc_dointvec_ms_jiffies_minmax() enforce minmax again.
  2026-09-05 23:36 [PATCH 0/2] sysctl: Restore minmax check for proc_do{int,ulong}vec_ms_jiffies_minmax() Kuniyuki Iwashima
@ 2026-09-05 23:36 ` Kuniyuki Iwashima
  2026-09-08 11:06   ` Joel Granados
  2026-09-05 23:36 ` [PATCH 2/2] sysctl: Make proc_doulongvec_ms_jiffies_minmax() " Kuniyuki Iwashima
  1 sibling, 1 reply; 13+ messages in thread
From: Kuniyuki Iwashima @ 2026-09-05 23:36 UTC (permalink / raw)
  To: Kees Cook, Joel Granados, Thomas Gleixner
  Cc: Kuniyuki Iwashima, Kuniyuki Iwashima, linux-fsdevel

When the cited commit converted the SYSCTL_INT_CONV_CUSTOM macro to
functions, it accidentally changed do_proc_int_conv_ms_jiffies_minmax()
to pass false for k_ptr_range_check.

Since then, net.ipv[46].neigh.${dev}.interval_probe_time_ms no
longer enforces the lower bound.

  # sysctl net.ipv4.neigh.bond0.interval_probe_time_ms=0
  net.ipv4.neigh.bond0.interval_probe_time_ms = 0

Let's restore the minmax check.

With this:

  # sysctl net.ipv4.neigh.bond0.interval_probe_time_ms=0
  sysctl: setting key "net.ipv4.neigh.bond0.interval_probe_time_ms": Invalid argument

Fixes: d174174c6776 ("sysctl: replace SYSCTL_INT_CONV_CUSTOM macro with functions")
Signed-off-by: Kuniyuki Iwashima <kuniyu@google.com>
---
 kernel/time/jiffies.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/time/jiffies.c b/kernel/time/jiffies.c
index 213ae1d6a014..70926b73905a 100644
--- a/kernel/time/jiffies.c
+++ b/kernel/time/jiffies.c
@@ -181,7 +181,7 @@ static int do_proc_int_conv_ms_jiffies_minmax(bool *negp, ulong *u_ptr,
 					      int *k_ptr, int dir,
 					      const struct ctl_table *tbl)
 {
-	return proc_int_conv(negp, u_ptr, k_ptr, dir, tbl, false,
+	return proc_int_conv(negp, u_ptr, k_ptr, dir, tbl, true,
 			     sysctl_u2k_int_conv_ms, sysctl_k2u_int_conv_ms);
 }
 
-- 
2.55.0.1003.g10538fe699-goog


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

* [PATCH 2/2] sysctl: Make proc_doulongvec_ms_jiffies_minmax() enforce minmax again.
  2026-09-05 23:36 [PATCH 0/2] sysctl: Restore minmax check for proc_do{int,ulong}vec_ms_jiffies_minmax() Kuniyuki Iwashima
  2026-09-05 23:36 ` [PATCH 1/2] sysctl: Make proc_dointvec_ms_jiffies_minmax() enforce minmax again Kuniyuki Iwashima
@ 2026-09-05 23:36 ` Kuniyuki Iwashima
  2026-09-08 12:15   ` Joel Granados
  1 sibling, 1 reply; 13+ messages in thread
From: Kuniyuki Iwashima @ 2026-09-05 23:36 UTC (permalink / raw)
  To: Kees Cook, Joel Granados, Thomas Gleixner
  Cc: Kuniyuki Iwashima, Kuniyuki Iwashima, linux-fsdevel

Since the cited commit changed proc_doulongvec_ms_jiffies_minmax()
to use proc_ulong_conv(), proc_doulongvec_ms_jiffies_minmax() no
longer applies the range check.

It happened probably because do_proc_ulong_conv_ms_jiffies() does
not have the _minmax suffix.

In addition, sysctl_msecs_to_jiffies() casts u64 user input to u32,
and a truncated value could bypass the max check.

Let's rename do_proc_ulong_conv_ms_jiffies(), pass true to
k_ptr_range_check, and limit the max user input to INT_MAX in
sysctl_u2k_ulong_conv_ms() to avoid truncation and clamping to
MAX_JIFFY_OFFSET. (INT_MAX ms ~= 24 days is more than enough)

Fixes: b96b5c6708ea ("sysctl: Replace do_proc_do{int,ulong,uint}vec with do_proc_vec")
Signed-off-by: Kuniyuki Iwashima <kuniyu@google.com>
---
 kernel/time/jiffies.c | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/kernel/time/jiffies.c b/kernel/time/jiffies.c
index 70926b73905a..4737431fdb16 100644
--- a/kernel/time/jiffies.c
+++ b/kernel/time/jiffies.c
@@ -187,6 +187,9 @@ static int do_proc_int_conv_ms_jiffies_minmax(bool *negp, ulong *u_ptr,
 
 static int sysctl_u2k_ulong_conv_ms(const ulong *u_ptr, ulong *k_ptr)
 {
+	if (*u_ptr > INT_MAX)
+		return -EINVAL;
+
 	return proc_ulong_u2k_conv_uop(u_ptr, k_ptr, sysctl_msecs_to_jiffies);
 }
 
@@ -195,10 +198,10 @@ static int sysctl_k2u_ulong_conv_ms(ulong *u_ptr, const ulong *k_ptr)
 	return proc_ulong_k2u_conv_kop(u_ptr, k_ptr, sysctl_jiffies_to_msecs);
 }
 
-static int do_proc_ulong_conv_ms_jiffies(bool *negp, ulong *u_ptr, ulong *k_ptr,
-					 int dir, const struct ctl_table *tbl)
+static int do_proc_ulong_conv_ms_jiffies_minmax(bool *negp, ulong *u_ptr, ulong *k_ptr,
+						int dir, const struct ctl_table *tbl)
 {
-	return proc_ulong_conv(u_ptr, k_ptr, dir, tbl, false,
+	return proc_ulong_conv(u_ptr, k_ptr, dir, tbl, true,
 			       sysctl_u2k_ulong_conv_ms, sysctl_k2u_ulong_conv_ms);
 }
 
@@ -229,8 +232,8 @@ static int do_proc_int_conv_ms_jiffies_minmax(bool *negp, ulong *u_ptr,
 	return -ENOSYS;
 }
 
-static int do_proc_ulong_conv_ms_jiffies(bool *negp, ulong *u_ptr, ulong *k_ptr,
-					 int dir, const struct ctl_table *tbl)
+static int do_proc_ulong_conv_ms_jiffies_minmax(bool *negp, ulong *u_ptr, ulong *k_ptr,
+						int dir, const struct ctl_table *tbl)
 {
 	return -ENOSYS;
 }
@@ -333,7 +336,7 @@ int proc_doulongvec_ms_jiffies_minmax(const struct ctl_table *table, int dir,
 				      void *buffer, size_t *lenp, loff_t *ppos)
 {
 	return proc_doulongvec_conv(table, dir, buffer, lenp, ppos,
-				    do_proc_ulong_conv_ms_jiffies);
+				    do_proc_ulong_conv_ms_jiffies_minmax);
 }
 EXPORT_SYMBOL(proc_doulongvec_ms_jiffies_minmax);
 
-- 
2.55.0.1003.g10538fe699-goog


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

* Re: [PATCH 1/2] sysctl: Make proc_dointvec_ms_jiffies_minmax() enforce minmax again.
  2026-09-05 23:36 ` [PATCH 1/2] sysctl: Make proc_dointvec_ms_jiffies_minmax() enforce minmax again Kuniyuki Iwashima
@ 2026-09-08 11:06   ` Joel Granados
  2026-09-08 18:38     ` Kuniyuki Iwashima
  0 siblings, 1 reply; 13+ messages in thread
From: Joel Granados @ 2026-09-08 11:06 UTC (permalink / raw)
  To: Kuniyuki Iwashima
  Cc: Kees Cook, Thomas Gleixner, Kuniyuki Iwashima, linux-fsdevel

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

On Sat, Sep 05, 2026 at 11:36:30PM +0000, Kuniyuki Iwashima wrote:
> When the cited commit converted the SYSCTL_INT_CONV_CUSTOM macro to
> functions, it accidentally changed do_proc_int_conv_ms_jiffies_minmax()
> to pass false for k_ptr_range_check.
> 
> Since then, net.ipv[46].neigh.${dev}.interval_probe_time_ms no
> longer enforces the lower bound.
> 
>   # sysctl net.ipv4.neigh.bond0.interval_probe_time_ms=0
>   net.ipv4.neigh.bond0.interval_probe_time_ms = 0
> 
> Let's restore the minmax check.
> 
> With this:
> 
>   # sysctl net.ipv4.neigh.bond0.interval_probe_time_ms=0
>   sysctl: setting key "net.ipv4.neigh.bond0.interval_probe_time_ms": Invalid argument
> 
> Fixes: d174174c6776 ("sysctl: replace SYSCTL_INT_CONV_CUSTOM macro with functions")
> Signed-off-by: Kuniyuki Iwashima <kuniyu@google.com>
> ---
>  kernel/time/jiffies.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/kernel/time/jiffies.c b/kernel/time/jiffies.c
> index 213ae1d6a014..70926b73905a 100644
> --- a/kernel/time/jiffies.c
> +++ b/kernel/time/jiffies.c
> @@ -181,7 +181,7 @@ static int do_proc_int_conv_ms_jiffies_minmax(bool *negp, ulong *u_ptr,
>  					      int *k_ptr, int dir,
>  					      const struct ctl_table *tbl)
>  {
> -	return proc_int_conv(negp, u_ptr, k_ptr, dir, tbl, false,
> +	return proc_int_conv(negp, u_ptr, k_ptr, dir, tbl, true,
>  			     sysctl_u2k_int_conv_ms, sysctl_k2u_int_conv_ms);

oops!!!! Very good catch. 

I can see that this needs to be fixed, but your commit message needs
work. Please re-write like this:


 Add the range check to do_proc_int_conv_ms_jiffies_minmax that commit
 d174174c6776 ("sysctl: replace SYSCTL_INT_CONV_CUSTOM macro with
 functions") incorrectly removed.

 Fixes: d174174c6776 ("sysctl: replace SYSCTL_INT_CONV_CUSTOM macro with functions")
 Signed....

Why is that info about net is relevant here? I would remove it.

>  }
>  
> -- 
> 2.55.0.1003.g10538fe699-goog
> 

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

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

* Re: [PATCH 2/2] sysctl: Make proc_doulongvec_ms_jiffies_minmax() enforce minmax again.
  2026-09-05 23:36 ` [PATCH 2/2] sysctl: Make proc_doulongvec_ms_jiffies_minmax() " Kuniyuki Iwashima
@ 2026-09-08 12:15   ` Joel Granados
  2026-09-08 18:44     ` Kuniyuki Iwashima
  0 siblings, 1 reply; 13+ messages in thread
From: Joel Granados @ 2026-09-08 12:15 UTC (permalink / raw)
  To: Kuniyuki Iwashima
  Cc: Kees Cook, Thomas Gleixner, Kuniyuki Iwashima, linux-fsdevel

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

On Sat, Sep 05, 2026 at 11:36:31PM +0000, Kuniyuki Iwashima wrote:
> Since the cited commit changed proc_doulongvec_ms_jiffies_minmax()
> to use proc_ulong_conv(), proc_doulongvec_ms_jiffies_minmax() no
> longer applies the range check.
> 
> It happened probably because do_proc_ulong_conv_ms_jiffies() does
> not have the _minmax suffix.
> 
> In addition, sysctl_msecs_to_jiffies() casts u64 user input to u32,
> and a truncated value could bypass the max check.
This is something that is orthogonal to changing to true in the
proc_ulong_conv call. Please put this in its own commit.

This also begs the question: why not just use an int converter if the
range of the values is going to be u32?

> 
> Let's rename do_proc_ulong_conv_ms_jiffies(), pass true to
> k_ptr_range_check, and limit the max user input to INT_MAX in
> sysctl_u2k_ulong_conv_ms() to avoid truncation and clamping to
> MAX_JIFFY_OFFSET. (INT_MAX ms ~= 24 days is more than enough)
> 
> Fixes: b96b5c6708ea ("sysctl: Replace do_proc_do{int,ulong,uint}vec with do_proc_vec")
> Signed-off-by: Kuniyuki Iwashima <kuniyu@google.com>
> ---
>  kernel/time/jiffies.c | 15 +++++++++------
>  1 file changed, 9 insertions(+), 6 deletions(-)
> 
> diff --git a/kernel/time/jiffies.c b/kernel/time/jiffies.c
> index 70926b73905a..4737431fdb16 100644
> --- a/kernel/time/jiffies.c
> +++ b/kernel/time/jiffies.c
> @@ -187,6 +187,9 @@ static int do_proc_int_conv_ms_jiffies_minmax(bool *negp, ulong *u_ptr,
>  
>  static int sysctl_u2k_ulong_conv_ms(const ulong *u_ptr, ulong *k_ptr)
>  {
> +	if (*u_ptr > INT_MAX)
> +		return -EINVAL;
> +

Create a commit for this on its own.

>  	return proc_ulong_u2k_conv_uop(u_ptr, k_ptr, sysctl_msecs_to_jiffies);
>  }
>  
> @@ -195,10 +198,10 @@ static int sysctl_k2u_ulong_conv_ms(ulong *u_ptr, const ulong *k_ptr)
>  	return proc_ulong_k2u_conv_kop(u_ptr, k_ptr, sysctl_jiffies_to_msecs);
>  }
>  
> -static int do_proc_ulong_conv_ms_jiffies(bool *negp, ulong *u_ptr, ulong *k_ptr,
> -					 int dir, const struct ctl_table *tbl)
> +static int do_proc_ulong_conv_ms_jiffies_minmax(bool *negp, ulong *u_ptr, ulong *k_ptr,
> +						int dir, const struct ctl_table *tbl)
>  {
> -	return proc_ulong_conv(u_ptr, k_ptr, dir, tbl, false,
> +	return proc_ulong_conv(u_ptr, k_ptr, dir, tbl, true,
>  			       sysctl_u2k_ulong_conv_ms, sysctl_k2u_ulong_conv_ms);
>  }
>  
> @@ -229,8 +232,8 @@ static int do_proc_int_conv_ms_jiffies_minmax(bool *negp, ulong *u_ptr,
>  	return -ENOSYS;
>  }
>  
> -static int do_proc_ulong_conv_ms_jiffies(bool *negp, ulong *u_ptr, ulong *k_ptr,
> -					 int dir, const struct ctl_table *tbl)
> +static int do_proc_ulong_conv_ms_jiffies_minmax(bool *negp, ulong *u_ptr, ulong *k_ptr,
> +						int dir, const struct ctl_table *tbl)
>  {
>  	return -ENOSYS;
>  }
> @@ -333,7 +336,7 @@ int proc_doulongvec_ms_jiffies_minmax(const struct ctl_table *table, int dir,
>  				      void *buffer, size_t *lenp, loff_t *ppos)
>  {
>  	return proc_doulongvec_conv(table, dir, buffer, lenp, ppos,
> -				    do_proc_ulong_conv_ms_jiffies);
> +				    do_proc_ulong_conv_ms_jiffies_minmax);
>  }
>  EXPORT_SYMBOL(proc_doulongvec_ms_jiffies_minmax);

Good catch, but the commit wording is off. Please change to something
like

  Add the range check back to do_proc_ulong_conv_ms_jiffies that commit
  b96b5c6708ea ("sysctl: Replace do_proc_do{int,ulong,uint}vec with
  do_proc_vec") incorrectlry removed. Append "_minmax" to the end of the
  do_proc_ulong_conv_ms_jiffies so it is clear that there should be a
  range check.

  Fixes: b96b5c6708ea ("sysctl: Replace do_proc_do{int,ulong,uint}vec with do_proc_vec")
  Signed...




>  
> -- 
> 2.55.0.1003.g10538fe699-goog
> 

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

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

* Re: [PATCH 1/2] sysctl: Make proc_dointvec_ms_jiffies_minmax() enforce minmax again.
  2026-09-08 11:06   ` Joel Granados
@ 2026-09-08 18:38     ` Kuniyuki Iwashima
  2026-09-09 13:48       ` Joel Granados
  0 siblings, 1 reply; 13+ messages in thread
From: Kuniyuki Iwashima @ 2026-09-08 18:38 UTC (permalink / raw)
  To: Joel Granados
  Cc: Kees Cook, Thomas Gleixner, Kuniyuki Iwashima, linux-fsdevel

On Tue, Sep 8, 2026 at 4:06 AM Joel Granados <joel.granados@kernel.org> wrote:
>
> On Sat, Sep 05, 2026 at 11:36:30PM +0000, Kuniyuki Iwashima wrote:
> > When the cited commit converted the SYSCTL_INT_CONV_CUSTOM macro to
> > functions, it accidentally changed do_proc_int_conv_ms_jiffies_minmax()
> > to pass false for k_ptr_range_check.
> >
> > Since then, net.ipv[46].neigh.${dev}.interval_probe_time_ms no
> > longer enforces the lower bound.
> >
> >   # sysctl net.ipv4.neigh.bond0.interval_probe_time_ms=0
> >   net.ipv4.neigh.bond0.interval_probe_time_ms = 0
> >
> > Let's restore the minmax check.
> >
> > With this:
> >
> >   # sysctl net.ipv4.neigh.bond0.interval_probe_time_ms=0
> >   sysctl: setting key "net.ipv4.neigh.bond0.interval_probe_time_ms": Invalid argument
> >
> > Fixes: d174174c6776 ("sysctl: replace SYSCTL_INT_CONV_CUSTOM macro with functions")
> > Signed-off-by: Kuniyuki Iwashima <kuniyu@google.com>
> > ---
> >  kernel/time/jiffies.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/kernel/time/jiffies.c b/kernel/time/jiffies.c
> > index 213ae1d6a014..70926b73905a 100644
> > --- a/kernel/time/jiffies.c
> > +++ b/kernel/time/jiffies.c
> > @@ -181,7 +181,7 @@ static int do_proc_int_conv_ms_jiffies_minmax(bool *negp, ulong *u_ptr,
> >                                             int *k_ptr, int dir,
> >                                             const struct ctl_table *tbl)
> >  {
> > -     return proc_int_conv(negp, u_ptr, k_ptr, dir, tbl, false,
> > +     return proc_int_conv(negp, u_ptr, k_ptr, dir, tbl, true,
> >                            sysctl_u2k_int_conv_ms, sysctl_k2u_int_conv_ms);
>
> oops!!!! Very good catch.
>
> I can see that this needs to be fixed, but your commit message needs
> work. Please re-write like this:
>
>
>  Add the range check to do_proc_int_conv_ms_jiffies_minmax that commit
>  d174174c6776 ("sysctl: replace SYSCTL_INT_CONV_CUSTOM macro with
>  functions") incorrectly removed.
>
>  Fixes: d174174c6776 ("sysctl: replace SYSCTL_INT_CONV_CUSTOM macro with functions")
>  Signed....
>
> Why is that info about net is relevant here? I would remove it.

Because it's the only relevant user.

$ grep -rnI proc_dointvec_ms_jiffies_minmax
include/linux/jiffies.h:649:int proc_dointvec_ms_jiffies_minmax(const
struct ctl_table *table, int dir,
net/core/neighbour.c:3679: ret = proc_dointvec_ms_jiffies_minmax(&tmp,
write, buffer, lenp, ppos);
kernel/time/jiffies.c:311:int proc_dointvec_ms_jiffies_minmax(const
struct ctl_table *table, int dir,

>
> >  }
> >
> > --
> > 2.55.0.1003.g10538fe699-goog
> >

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

* Re: [PATCH 2/2] sysctl: Make proc_doulongvec_ms_jiffies_minmax() enforce minmax again.
  2026-09-08 12:15   ` Joel Granados
@ 2026-09-08 18:44     ` Kuniyuki Iwashima
  2026-09-09 13:44       ` Joel Granados
  0 siblings, 1 reply; 13+ messages in thread
From: Kuniyuki Iwashima @ 2026-09-08 18:44 UTC (permalink / raw)
  To: Joel Granados
  Cc: Kees Cook, Thomas Gleixner, Kuniyuki Iwashima, linux-fsdevel

On Tue, Sep 8, 2026 at 5:15 AM Joel Granados <joel.granados@kernel.org> wrote:
>
> On Sat, Sep 05, 2026 at 11:36:31PM +0000, Kuniyuki Iwashima wrote:
> > Since the cited commit changed proc_doulongvec_ms_jiffies_minmax()
> > to use proc_ulong_conv(), proc_doulongvec_ms_jiffies_minmax() no
> > longer applies the range check.
> >
> > It happened probably because do_proc_ulong_conv_ms_jiffies() does
> > not have the _minmax suffix.
> >
> > In addition, sysctl_msecs_to_jiffies() casts u64 user input to u32,
> > and a truncated value could bypass the max check.
> This is something that is orthogonal to changing to true in the
> proc_ulong_conv call. Please put this in its own commit.
>
> This also begs the question: why not just use an int converter if the
> range of the values is going to be u32?

I guess people wanted to use ulong just because jiffies
is unsigned long.

Looking at these users, none of them need the ulong range
and it can be u32 actually.

net/rds/sysctl.c:58: .proc_handler   = proc_doulongvec_ms_jiffies_minmax,
net/rds/sysctl.c:67: .proc_handler   = proc_doulongvec_ms_jiffies_minmax,
net/rxrpc/sysctl.c:59: .proc_handler = proc_doulongvec_ms_jiffies_minmax,
net/rxrpc/sysctl.c:68: .proc_handler = proc_doulongvec_ms_jiffies_minmax,
drivers/parport/procfs.c:369: .proc_handler = proc_doulongvec_ms_jiffies_minmax,
drivers/parport/procfs.c:399: .proc_handler = proc_doulongvec_ms_jiffies_minmax,


>
> >
> > Let's rename do_proc_ulong_conv_ms_jiffies(), pass true to
> > k_ptr_range_check, and limit the max user input to INT_MAX in
> > sysctl_u2k_ulong_conv_ms() to avoid truncation and clamping to
> > MAX_JIFFY_OFFSET. (INT_MAX ms ~= 24 days is more than enough)
> >
> > Fixes: b96b5c6708ea ("sysctl: Replace do_proc_do{int,ulong,uint}vec with do_proc_vec")
> > Signed-off-by: Kuniyuki Iwashima <kuniyu@google.com>
> > ---
> >  kernel/time/jiffies.c | 15 +++++++++------
> >  1 file changed, 9 insertions(+), 6 deletions(-)
> >
> > diff --git a/kernel/time/jiffies.c b/kernel/time/jiffies.c
> > index 70926b73905a..4737431fdb16 100644
> > --- a/kernel/time/jiffies.c
> > +++ b/kernel/time/jiffies.c
> > @@ -187,6 +187,9 @@ static int do_proc_int_conv_ms_jiffies_minmax(bool *negp, ulong *u_ptr,
> >
> >  static int sysctl_u2k_ulong_conv_ms(const ulong *u_ptr, ulong *k_ptr)
> >  {
> > +     if (*u_ptr > INT_MAX)
> > +             return -EINVAL;
> > +
>
> Create a commit for this on its own.
>
> >       return proc_ulong_u2k_conv_uop(u_ptr, k_ptr, sysctl_msecs_to_jiffies);
> >  }
> >
> > @@ -195,10 +198,10 @@ static int sysctl_k2u_ulong_conv_ms(ulong *u_ptr, const ulong *k_ptr)
> >       return proc_ulong_k2u_conv_kop(u_ptr, k_ptr, sysctl_jiffies_to_msecs);
> >  }
> >
> > -static int do_proc_ulong_conv_ms_jiffies(bool *negp, ulong *u_ptr, ulong *k_ptr,
> > -                                      int dir, const struct ctl_table *tbl)
> > +static int do_proc_ulong_conv_ms_jiffies_minmax(bool *negp, ulong *u_ptr, ulong *k_ptr,
> > +                                             int dir, const struct ctl_table *tbl)
> >  {
> > -     return proc_ulong_conv(u_ptr, k_ptr, dir, tbl, false,
> > +     return proc_ulong_conv(u_ptr, k_ptr, dir, tbl, true,
> >                              sysctl_u2k_ulong_conv_ms, sysctl_k2u_ulong_conv_ms);
> >  }
> >
> > @@ -229,8 +232,8 @@ static int do_proc_int_conv_ms_jiffies_minmax(bool *negp, ulong *u_ptr,
> >       return -ENOSYS;
> >  }
> >
> > -static int do_proc_ulong_conv_ms_jiffies(bool *negp, ulong *u_ptr, ulong *k_ptr,
> > -                                      int dir, const struct ctl_table *tbl)
> > +static int do_proc_ulong_conv_ms_jiffies_minmax(bool *negp, ulong *u_ptr, ulong *k_ptr,
> > +                                             int dir, const struct ctl_table *tbl)
> >  {
> >       return -ENOSYS;
> >  }
> > @@ -333,7 +336,7 @@ int proc_doulongvec_ms_jiffies_minmax(const struct ctl_table *table, int dir,
> >                                     void *buffer, size_t *lenp, loff_t *ppos)
> >  {
> >       return proc_doulongvec_conv(table, dir, buffer, lenp, ppos,
> > -                                 do_proc_ulong_conv_ms_jiffies);
> > +                                 do_proc_ulong_conv_ms_jiffies_minmax);
> >  }
> >  EXPORT_SYMBOL(proc_doulongvec_ms_jiffies_minmax);
>
> Good catch, but the commit wording is off. Please change to something
> like
>
>   Add the range check back to do_proc_ulong_conv_ms_jiffies that commit
>   b96b5c6708ea ("sysctl: Replace do_proc_do{int,ulong,uint}vec with
>   do_proc_vec") incorrectlry removed. Append "_minmax" to the end of the
>   do_proc_ulong_conv_ms_jiffies so it is clear that there should be a
>   range check.
>
>   Fixes: b96b5c6708ea ("sysctl: Replace do_proc_do{int,ulong,uint}vec with do_proc_vec")
>   Signed...
>
>
>
>
> >
> > --
> > 2.55.0.1003.g10538fe699-goog
> >

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

* Re: [PATCH 2/2] sysctl: Make proc_doulongvec_ms_jiffies_minmax() enforce minmax again.
  2026-09-08 18:44     ` Kuniyuki Iwashima
@ 2026-09-09 13:44       ` Joel Granados
  2026-09-09 17:07         ` Kuniyuki Iwashima
  0 siblings, 1 reply; 13+ messages in thread
From: Joel Granados @ 2026-09-09 13:44 UTC (permalink / raw)
  To: Kuniyuki Iwashima
  Cc: Kees Cook, Thomas Gleixner, Kuniyuki Iwashima, linux-fsdevel

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

On Tue, Sep 08, 2026 at 11:44:08AM -0700, Kuniyuki Iwashima wrote:
> On Tue, Sep 8, 2026 at 5:15 AM Joel Granados <joel.granados@kernel.org> wrote:
> >
> > On Sat, Sep 05, 2026 at 11:36:31PM +0000, Kuniyuki Iwashima wrote:
> > > Since the cited commit changed proc_doulongvec_ms_jiffies_minmax()
> > > to use proc_ulong_conv(), proc_doulongvec_ms_jiffies_minmax() no
> > > longer applies the range check.
> > >
> > > It happened probably because do_proc_ulong_conv_ms_jiffies() does
> > > not have the _minmax suffix.
> > >
> > > In addition, sysctl_msecs_to_jiffies() casts u64 user input to u32,
> > > and a truncated value could bypass the max check.
> > This is something that is orthogonal to changing to true in the
> > proc_ulong_conv call. Please put this in its own commit.
> >
> > This also begs the question: why not just use an int converter if the
> > range of the values is going to be u32?
> 
> I guess people wanted to use ulong just because jiffies
> is unsigned long.
> 
> Looking at these users, none of them need the ulong range
> and it can be u32 actually.

Exactly. But changing those calls from ulong to uint is up to net and
parport. I'm more interested in defining the behavior in
sysctl_u2k_ulong_conv_ms.

Here is my train of thought:
1. u64 values will get truncated when they pass through msecs_to_jiffies

2. The range of msecs_to_jiffies is:
   [0, INT_MAX]          --->  ceil(m × HZ / 1000)
   [INT_MAX+1, UINT_MAX] --->  MAX_JIFFY_OFFSET

   Which I interpret as whatever that falls out of [0, INT_MAX]
   milliseconds is just infinite jiffies (MAX_JIFFY_OFFSET).

3. All but one users of do_proc_ulong_conv_ms_jiffies are bound to
   values under MAX_JIFFY_OFFSET. The exception is net/rds/sysctl.c:67
   that has a max of ~0UL (is this too high?).

4. Since milliseconds to jiffies should be capped at MAX_JIFFY_OFFSET
   for big values. Should we just return MAX_JIFFY_OFFSET?
   Like this:

    diff --git i/kernel/time/jiffies.c w/kernel/time/jiffies.c
    index 213ae1d6a014..400353b0d6f9 100644
    --- i/kernel/time/jiffies.c
    +++ w/kernel/time/jiffies.c
    @@ -136,6 +136,8 @@ static int sysctl_k2u_int_conv_userhz(bool *negp, ulong *u_ptr, const int *k_ptr

     static ulong sysctl_msecs_to_jiffies(const ulong val)
     {
    +       if (val > UINT_MAX)
    +               return MAX_JIFFY_OFFSET;
            return msecs_to_jiffies(val);
     }


> 
> net/rds/sysctl.c:58: .proc_handler   = proc_doulongvec_ms_jiffies_minmax,
> net/rds/sysctl.c:67: .proc_handler   = proc_doulongvec_ms_jiffies_minmax,
> net/rxrpc/sysctl.c:59: .proc_handler = proc_doulongvec_ms_jiffies_minmax,
> net/rxrpc/sysctl.c:68: .proc_handler = proc_doulongvec_ms_jiffies_minmax,
> drivers/parport/procfs.c:369: .proc_handler = proc_doulongvec_ms_jiffies_minmax,
> drivers/parport/procfs.c:399: .proc_handler = proc_doulongvec_ms_jiffies_minmax,
> 
> 
<...snip...>
> >   do_proc_vec") incorrectlry removed. Append "_minmax" to the end of the
> >   do_proc_ulong_conv_ms_jiffies so it is clear that there should be a
> >   range check.
> >
> >   Fixes: b96b5c6708ea ("sysctl: Replace do_proc_do{int,ulong,uint}vec with do_proc_vec")
> >   Signed...
> >
> >
> >
> >
> > >
> > > --
> > > 2.55.0.1003.g10538fe699-goog
> > >

Best

Joel

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

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

* Re: [PATCH 1/2] sysctl: Make proc_dointvec_ms_jiffies_minmax() enforce minmax again.
  2026-09-08 18:38     ` Kuniyuki Iwashima
@ 2026-09-09 13:48       ` Joel Granados
  0 siblings, 0 replies; 13+ messages in thread
From: Joel Granados @ 2026-09-09 13:48 UTC (permalink / raw)
  To: Kuniyuki Iwashima
  Cc: Kees Cook, Thomas Gleixner, Kuniyuki Iwashima, linux-fsdevel

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

On Tue, Sep 08, 2026 at 11:38:52AM -0700, Kuniyuki Iwashima wrote:
> On Tue, Sep 8, 2026 at 4:06 AM Joel Granados <joel.granados@kernel.org> wrote:
> >
> > On Sat, Sep 05, 2026 at 11:36:30PM +0000, Kuniyuki Iwashima wrote:
> > > When the cited commit converted the SYSCTL_INT_CONV_CUSTOM macro to
> > > functions, it accidentally changed do_proc_int_conv_ms_jiffies_minmax()
> > > to pass false for k_ptr_range_check.
> > >
> > > Since then, net.ipv[46].neigh.${dev}.interval_probe_time_ms no
> > > longer enforces the lower bound.
> > >
> > >   # sysctl net.ipv4.neigh.bond0.interval_probe_time_ms=0
> > >   net.ipv4.neigh.bond0.interval_probe_time_ms = 0
> > >
> > > Let's restore the minmax check.
> > >
> > > With this:
> > >
> > >   # sysctl net.ipv4.neigh.bond0.interval_probe_time_ms=0
> > >   sysctl: setting key "net.ipv4.neigh.bond0.interval_probe_time_ms": Invalid argument
> > >
> > > Fixes: d174174c6776 ("sysctl: replace SYSCTL_INT_CONV_CUSTOM macro with functions")
> > > Signed-off-by: Kuniyuki Iwashima <kuniyu@google.com>
> > > ---
> > >  kernel/time/jiffies.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/kernel/time/jiffies.c b/kernel/time/jiffies.c
> > > index 213ae1d6a014..70926b73905a 100644
> > > --- a/kernel/time/jiffies.c
> > > +++ b/kernel/time/jiffies.c
> > > @@ -181,7 +181,7 @@ static int do_proc_int_conv_ms_jiffies_minmax(bool *negp, ulong *u_ptr,
> > >                                             int *k_ptr, int dir,
> > >                                             const struct ctl_table *tbl)
> > >  {
> > > -     return proc_int_conv(negp, u_ptr, k_ptr, dir, tbl, false,
> > > +     return proc_int_conv(negp, u_ptr, k_ptr, dir, tbl, true,
> > >                            sysctl_u2k_int_conv_ms, sysctl_k2u_int_conv_ms);
> >
> > oops!!!! Very good catch.
> >
> > I can see that this needs to be fixed, but your commit message needs
> > work. Please re-write like this:
> >
> >
> >  Add the range check to do_proc_int_conv_ms_jiffies_minmax that commit
> >  d174174c6776 ("sysctl: replace SYSCTL_INT_CONV_CUSTOM macro with
> >  functions") incorrectly removed.
> >
> >  Fixes: d174174c6776 ("sysctl: replace SYSCTL_INT_CONV_CUSTOM macro with functions")
> >  Signed....
> >
> > Why is that info about net is relevant here? I would remove it.
> 
> Because it's the only relevant user.

I can see that, but the range check was removed because of a oversight
in d174174c6776, not because of anything happening in the caller. I
would just drop that info from the commit message.

> 
> $ grep -rnI proc_dointvec_ms_jiffies_minmax
> include/linux/jiffies.h:649:int proc_dointvec_ms_jiffies_minmax(const
> struct ctl_table *table, int dir,
> net/core/neighbour.c:3679: ret = proc_dointvec_ms_jiffies_minmax(&tmp,
> write, buffer, lenp, ppos);
> kernel/time/jiffies.c:311:int proc_dointvec_ms_jiffies_minmax(const
> struct ctl_table *table, int dir,
> 
> >
> > >  }
> > >
> > > --
> > > 2.55.0.1003.g10538fe699-goog
> > >

Please let me know if you will send a V2. Else I'll just push them at
the beginning of next week.

Best

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

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

* Re: [PATCH 2/2] sysctl: Make proc_doulongvec_ms_jiffies_minmax() enforce minmax again.
  2026-09-09 13:44       ` Joel Granados
@ 2026-09-09 17:07         ` Kuniyuki Iwashima
  2026-09-10  9:04           ` Joel Granados
  0 siblings, 1 reply; 13+ messages in thread
From: Kuniyuki Iwashima @ 2026-09-09 17:07 UTC (permalink / raw)
  To: Joel Granados
  Cc: Kees Cook, Thomas Gleixner, Kuniyuki Iwashima, linux-fsdevel

On Wed, Sep 9, 2026 at 6:44 AM Joel Granados <joel.granados@kernel.org> wrote:
>
> On Tue, Sep 08, 2026 at 11:44:08AM -0700, Kuniyuki Iwashima wrote:
> > On Tue, Sep 8, 2026 at 5:15 AM Joel Granados <joel.granados@kernel.org> wrote:
> > >
> > > On Sat, Sep 05, 2026 at 11:36:31PM +0000, Kuniyuki Iwashima wrote:
> > > > Since the cited commit changed proc_doulongvec_ms_jiffies_minmax()
> > > > to use proc_ulong_conv(), proc_doulongvec_ms_jiffies_minmax() no
> > > > longer applies the range check.
> > > >
> > > > It happened probably because do_proc_ulong_conv_ms_jiffies() does
> > > > not have the _minmax suffix.
> > > >
> > > > In addition, sysctl_msecs_to_jiffies() casts u64 user input to u32,
> > > > and a truncated value could bypass the max check.
> > > This is something that is orthogonal to changing to true in the
> > > proc_ulong_conv call. Please put this in its own commit.
> > >
> > > This also begs the question: why not just use an int converter if the
> > > range of the values is going to be u32?
> >
> > I guess people wanted to use ulong just because jiffies
> > is unsigned long.
> >
> > Looking at these users, none of them need the ulong range
> > and it can be u32 actually.
>
> Exactly. But changing those calls from ulong to uint is up to net and
> parport.

Agreed, it should be done in the -next branch of each subsystem.

And later, we can remove do_proc_ulong_conv_ms_jiffies().


> I'm more interested in defining the behavior in
> sysctl_u2k_ulong_conv_ms.
>
> Here is my train of thought:
> 1. u64 values will get truncated when they pass through msecs_to_jiffies
>
> 2. The range of msecs_to_jiffies is:
>    [0, INT_MAX]          --->  ceil(m × HZ / 1000)
>    [INT_MAX+1, UINT_MAX] --->  MAX_JIFFY_OFFSET
>
>    Which I interpret as whatever that falls out of [0, INT_MAX]
>    milliseconds is just infinite jiffies (MAX_JIFFY_OFFSET).

nit: for HZ > 1000 (IIRC up to 1200 ?), the max of the range
gets a little bit smaller.


>
> 3. All but one users of do_proc_ulong_conv_ms_jiffies are bound to
>    values under MAX_JIFFY_OFFSET. The exception is net/rds/sysctl.c:67
>    that has a max of ~0UL (is this too high?).

I think this is just set as the theoretical max of @jiffies,
but not specifically for the knob.

The default of the knob is 1 sec (HZ), and even INT_MAX
(24 days) is too high for RDS reconnect.


>
> 4. Since milliseconds to jiffies should be capped at MAX_JIFFY_OFFSET
>    for big values. Should we just return MAX_JIFFY_OFFSET?
>    Like this:
>
>     diff --git i/kernel/time/jiffies.c w/kernel/time/jiffies.c
>     index 213ae1d6a014..400353b0d6f9 100644
>     --- i/kernel/time/jiffies.c
>     +++ w/kernel/time/jiffies.c
>     @@ -136,6 +136,8 @@ static int sysctl_k2u_int_conv_userhz(bool *negp, ulong *u_ptr, const int *k_ptr
>
>      static ulong sysctl_msecs_to_jiffies(const ulong val)
>      {
>     +       if (val > UINT_MAX)
>     +               return MAX_JIFFY_OFFSET;

Given the nit at 2., it should be like this

if (val > jiffies_to_msecs(MAX_JIFFY_OFFSET))
    return MAX_JIFFY_OFFSET;

as done in _msecs_to_jiffies().


>             return msecs_to_jiffies(val);
>      }
>
>
> >
> > net/rds/sysctl.c:58: .proc_handler   = proc_doulongvec_ms_jiffies_minmax,
> > net/rds/sysctl.c:67: .proc_handler   = proc_doulongvec_ms_jiffies_minmax,
> > net/rxrpc/sysctl.c:59: .proc_handler = proc_doulongvec_ms_jiffies_minmax,
> > net/rxrpc/sysctl.c:68: .proc_handler = proc_doulongvec_ms_jiffies_minmax,
> > drivers/parport/procfs.c:369: .proc_handler = proc_doulongvec_ms_jiffies_minmax,
> > drivers/parport/procfs.c:399: .proc_handler = proc_doulongvec_ms_jiffies_minmax,
> >
> >
> <...snip...>
> > >   do_proc_vec") incorrectlry removed. Append "_minmax" to the end of the
> > >   do_proc_ulong_conv_ms_jiffies so it is clear that there should be a
> > >   range check.
> > >
> > >   Fixes: b96b5c6708ea ("sysctl: Replace do_proc_do{int,ulong,uint}vec with do_proc_vec")
> > >   Signed...
> > >
> > >
> > >
> > >
> > > >
> > > > --
> > > > 2.55.0.1003.g10538fe699-goog
> > > >
>
> Best
>
> Joel

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

* Re: [PATCH 2/2] sysctl: Make proc_doulongvec_ms_jiffies_minmax() enforce minmax again.
  2026-09-09 17:07         ` Kuniyuki Iwashima
@ 2026-09-10  9:04           ` Joel Granados
  2026-09-10 10:45             ` Joel Granados
  0 siblings, 1 reply; 13+ messages in thread
From: Joel Granados @ 2026-09-10  9:04 UTC (permalink / raw)
  To: Kuniyuki Iwashima
  Cc: Kees Cook, Thomas Gleixner, Kuniyuki Iwashima, linux-fsdevel

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

On Wed, Sep 09, 2026 at 10:07:53AM -0700, Kuniyuki Iwashima wrote:
> On Wed, Sep 9, 2026 at 6:44 AM Joel Granados <joel.granados@kernel.org> wrote:
> >
> > On Tue, Sep 08, 2026 at 11:44:08AM -0700, Kuniyuki Iwashima wrote:
> > > On Tue, Sep 8, 2026 at 5:15 AM Joel Granados <joel.granados@kernel.org> wrote:
> > > >
> > > > On Sat, Sep 05, 2026 at 11:36:31PM +0000, Kuniyuki Iwashima wrote:
> > > > > Since the cited commit changed proc_doulongvec_ms_jiffies_minmax()
> > > > > to use proc_ulong_conv(), proc_doulongvec_ms_jiffies_minmax() no
> > > > > longer applies the range check.
> > > > >
> > > > > It happened probably because do_proc_ulong_conv_ms_jiffies() does
> > > > > not have the _minmax suffix.
> > > > >
> > > > > In addition, sysctl_msecs_to_jiffies() casts u64 user input to u32,
> > > > > and a truncated value could bypass the max check.
> > > > This is something that is orthogonal to changing to true in the
> > > > proc_ulong_conv call. Please put this in its own commit.
> > > >
> > > > This also begs the question: why not just use an int converter if the
> > > > range of the values is going to be u32?
> > >
> > > I guess people wanted to use ulong just because jiffies
> > > is unsigned long.
> > >
> > > Looking at these users, none of them need the ulong range
> > > and it can be u32 actually.
> >
> > Exactly. But changing those calls from ulong to uint is up to net and
> > parport.
> 
> Agreed, it should be done in the -next branch of each subsystem.
> 
> And later, we can remove do_proc_ulong_conv_ms_jiffies().
> 
> 
> > I'm more interested in defining the behavior in
> > sysctl_u2k_ulong_conv_ms.
> >
> > Here is my train of thought:
> > 1. u64 values will get truncated when they pass through msecs_to_jiffies
> >
> > 2. The range of msecs_to_jiffies is:
> >    [0, INT_MAX]          --->  ceil(m × HZ / 1000)
> >    [INT_MAX+1, UINT_MAX] --->  MAX_JIFFY_OFFSET
> >
> >    Which I interpret as whatever that falls out of [0, INT_MAX]
> >    milliseconds is just infinite jiffies (MAX_JIFFY_OFFSET).
> 
> nit: for HZ > 1000 (IIRC up to 1200 ?), the max of the range
> gets a little bit smaller.
> 
> 
> >
> > 3. All but one users of do_proc_ulong_conv_ms_jiffies are bound to
> >    values under MAX_JIFFY_OFFSET. The exception is net/rds/sysctl.c:67
> >    that has a max of ~0UL (is this too high?).
> 
> I think this is just set as the theoretical max of @jiffies,
> but not specifically for the knob.
> 
> The default of the knob is 1 sec (HZ), and even INT_MAX
> (24 days) is too high for RDS reconnect.
> 
> 
> >
> > 4. Since milliseconds to jiffies should be capped at MAX_JIFFY_OFFSET
> >    for big values. Should we just return MAX_JIFFY_OFFSET?
> >    Like this:
> >
> >     diff --git i/kernel/time/jiffies.c w/kernel/time/jiffies.c
> >     index 213ae1d6a014..400353b0d6f9 100644
> >     --- i/kernel/time/jiffies.c
> >     +++ w/kernel/time/jiffies.c
> >     @@ -136,6 +136,8 @@ static int sysctl_k2u_int_conv_userhz(bool *negp, ulong *u_ptr, const int *k_ptr
> >
> >      static ulong sysctl_msecs_to_jiffies(const ulong val)
> >      {
> >     +       if (val > UINT_MAX)
> >     +               return MAX_JIFFY_OFFSET;
> 
> Given the nit at 2., it should be like this
> 
> if (val > jiffies_to_msecs(MAX_JIFFY_OFFSET))
>     return MAX_JIFFY_OFFSET;
> 
> as done in _msecs_to_jiffies().

That is even better.

Please send a V2 of the 3 fixes discussed and I'll push them through
next week.

Best


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

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

* Re: [PATCH 2/2] sysctl: Make proc_doulongvec_ms_jiffies_minmax() enforce minmax again.
  2026-09-10  9:04           ` Joel Granados
@ 2026-09-10 10:45             ` Joel Granados
  2026-09-10 16:43               ` Kuniyuki Iwashima
  0 siblings, 1 reply; 13+ messages in thread
From: Joel Granados @ 2026-09-10 10:45 UTC (permalink / raw)
  To: Kuniyuki Iwashima
  Cc: Kees Cook, Thomas Gleixner, Kuniyuki Iwashima, linux-fsdevel

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

On Thu, Sep 10, 2026 at 11:04:12AM +0200, Joel Granados wrote:
> On Wed, Sep 09, 2026 at 10:07:53AM -0700, Kuniyuki Iwashima wrote:
> > On Wed, Sep 9, 2026 at 6:44 AM Joel Granados <joel.granados@kernel.org> wrote:
> > >
> > > On Tue, Sep 08, 2026 at 11:44:08AM -0700, Kuniyuki Iwashima wrote:
> > > > On Tue, Sep 8, 2026 at 5:15 AM Joel Granados <joel.granados@kernel.org> wrote:
> > > > >
> > > > > On Sat, Sep 05, 2026 at 11:36:31PM +0000, Kuniyuki Iwashima wrote:
> > > > > > Since the cited commit changed proc_doulongvec_ms_jiffies_minmax()
> > > > > > to use proc_ulong_conv(), proc_doulongvec_ms_jiffies_minmax() no
> > > > > > longer applies the range check.
> > > > > >
> > > > > > It happened probably because do_proc_ulong_conv_ms_jiffies() does
> > > > > > not have the _minmax suffix.
> > > > > >
> > > > > > In addition, sysctl_msecs_to_jiffies() casts u64 user input to u32,
> > > > > > and a truncated value could bypass the max check.
> > > > > This is something that is orthogonal to changing to true in the
> > > > > proc_ulong_conv call. Please put this in its own commit.
> > > > >
> > > > > This also begs the question: why not just use an int converter if the
> > > > > range of the values is going to be u32?
> > > >
> > > > I guess people wanted to use ulong just because jiffies
> > > > is unsigned long.
> > > >
> > > > Looking at these users, none of them need the ulong range
> > > > and it can be u32 actually.
> > >
> > > Exactly. But changing those calls from ulong to uint is up to net and
> > > parport.
> > 
> > Agreed, it should be done in the -next branch of each subsystem.
> > 
> > And later, we can remove do_proc_ulong_conv_ms_jiffies().
> > 
> > 
> > > I'm more interested in defining the behavior in
> > > sysctl_u2k_ulong_conv_ms.
> > >
> > > Here is my train of thought:
> > > 1. u64 values will get truncated when they pass through msecs_to_jiffies
> > >
> > > 2. The range of msecs_to_jiffies is:
> > >    [0, INT_MAX]          --->  ceil(m × HZ / 1000)
> > >    [INT_MAX+1, UINT_MAX] --->  MAX_JIFFY_OFFSET
> > >
> > >    Which I interpret as whatever that falls out of [0, INT_MAX]
> > >    milliseconds is just infinite jiffies (MAX_JIFFY_OFFSET).
> > 
> > nit: for HZ > 1000 (IIRC up to 1200 ?), the max of the range
> > gets a little bit smaller.
> > 
> > 
> > >
> > > 3. All but one users of do_proc_ulong_conv_ms_jiffies are bound to
> > >    values under MAX_JIFFY_OFFSET. The exception is net/rds/sysctl.c:67
> > >    that has a max of ~0UL (is this too high?).
> > 
> > I think this is just set as the theoretical max of @jiffies,
> > but not specifically for the knob.
> > 
> > The default of the knob is 1 sec (HZ), and even INT_MAX
> > (24 days) is too high for RDS reconnect.
> > 
> > 
> > >
> > > 4. Since milliseconds to jiffies should be capped at MAX_JIFFY_OFFSET
> > >    for big values. Should we just return MAX_JIFFY_OFFSET?
> > >    Like this:
> > >
> > >     diff --git i/kernel/time/jiffies.c w/kernel/time/jiffies.c
> > >     index 213ae1d6a014..400353b0d6f9 100644
> > >     --- i/kernel/time/jiffies.c
> > >     +++ w/kernel/time/jiffies.c
> > >     @@ -136,6 +136,8 @@ static int sysctl_k2u_int_conv_userhz(bool *negp, ulong *u_ptr, const int *k_ptr
> > >
> > >      static ulong sysctl_msecs_to_jiffies(const ulong val)
> > >      {
> > >     +       if (val > UINT_MAX)
> > >     +               return MAX_JIFFY_OFFSET;
> > 
> > Given the nit at 2., it should be like this
> > 
> > if (val > jiffies_to_msecs(MAX_JIFFY_OFFSET))
> >     return MAX_JIFFY_OFFSET;
> > 
> > as done in _msecs_to_jiffies().
> 
> That is even better.
> 
> Please send a V2 of the 3 fixes discussed and I'll push them through
> next week.

I have created [1] to test the fixes before pushing it upstream.
If you are ok with all my changes contained in [1], there is no need to
send V2. But if you want to change something, do send it out.

Best


[1] https://git.kernel.org/pub/scm/linux/kernel/git/joel.granados/linux.git/log/?h=jag/sysctl-fixes


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

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

* Re: [PATCH 2/2] sysctl: Make proc_doulongvec_ms_jiffies_minmax() enforce minmax again.
  2026-09-10 10:45             ` Joel Granados
@ 2026-09-10 16:43               ` Kuniyuki Iwashima
  0 siblings, 0 replies; 13+ messages in thread
From: Kuniyuki Iwashima @ 2026-09-10 16:43 UTC (permalink / raw)
  To: Joel Granados
  Cc: Kees Cook, Thomas Gleixner, Kuniyuki Iwashima, linux-fsdevel

On Thu, Sep 10, 2026 at 3:45 AM Joel Granados <joel.granados@kernel.org> wrote:
>
> On Thu, Sep 10, 2026 at 11:04:12AM +0200, Joel Granados wrote:
> > On Wed, Sep 09, 2026 at 10:07:53AM -0700, Kuniyuki Iwashima wrote:
> > > On Wed, Sep 9, 2026 at 6:44 AM Joel Granados <joel.granados@kernel.org> wrote:
> > > >
> > > > On Tue, Sep 08, 2026 at 11:44:08AM -0700, Kuniyuki Iwashima wrote:
> > > > > On Tue, Sep 8, 2026 at 5:15 AM Joel Granados <joel.granados@kernel.org> wrote:
> > > > > >
> > > > > > On Sat, Sep 05, 2026 at 11:36:31PM +0000, Kuniyuki Iwashima wrote:
> > > > > > > Since the cited commit changed proc_doulongvec_ms_jiffies_minmax()
> > > > > > > to use proc_ulong_conv(), proc_doulongvec_ms_jiffies_minmax() no
> > > > > > > longer applies the range check.
> > > > > > >
> > > > > > > It happened probably because do_proc_ulong_conv_ms_jiffies() does
> > > > > > > not have the _minmax suffix.
> > > > > > >
> > > > > > > In addition, sysctl_msecs_to_jiffies() casts u64 user input to u32,
> > > > > > > and a truncated value could bypass the max check.
> > > > > > This is something that is orthogonal to changing to true in the
> > > > > > proc_ulong_conv call. Please put this in its own commit.
> > > > > >
> > > > > > This also begs the question: why not just use an int converter if the
> > > > > > range of the values is going to be u32?
> > > > >
> > > > > I guess people wanted to use ulong just because jiffies
> > > > > is unsigned long.
> > > > >
> > > > > Looking at these users, none of them need the ulong range
> > > > > and it can be u32 actually.
> > > >
> > > > Exactly. But changing those calls from ulong to uint is up to net and
> > > > parport.
> > >
> > > Agreed, it should be done in the -next branch of each subsystem.
> > >
> > > And later, we can remove do_proc_ulong_conv_ms_jiffies().
> > >
> > >
> > > > I'm more interested in defining the behavior in
> > > > sysctl_u2k_ulong_conv_ms.
> > > >
> > > > Here is my train of thought:
> > > > 1. u64 values will get truncated when they pass through msecs_to_jiffies
> > > >
> > > > 2. The range of msecs_to_jiffies is:
> > > >    [0, INT_MAX]          --->  ceil(m × HZ / 1000)
> > > >    [INT_MAX+1, UINT_MAX] --->  MAX_JIFFY_OFFSET
> > > >
> > > >    Which I interpret as whatever that falls out of [0, INT_MAX]
> > > >    milliseconds is just infinite jiffies (MAX_JIFFY_OFFSET).
> > >
> > > nit: for HZ > 1000 (IIRC up to 1200 ?), the max of the range
> > > gets a little bit smaller.
> > >
> > >
> > > >
> > > > 3. All but one users of do_proc_ulong_conv_ms_jiffies are bound to
> > > >    values under MAX_JIFFY_OFFSET. The exception is net/rds/sysctl.c:67
> > > >    that has a max of ~0UL (is this too high?).
> > >
> > > I think this is just set as the theoretical max of @jiffies,
> > > but not specifically for the knob.
> > >
> > > The default of the knob is 1 sec (HZ), and even INT_MAX
> > > (24 days) is too high for RDS reconnect.
> > >
> > >
> > > >
> > > > 4. Since milliseconds to jiffies should be capped at MAX_JIFFY_OFFSET
> > > >    for big values. Should we just return MAX_JIFFY_OFFSET?
> > > >    Like this:
> > > >
> > > >     diff --git i/kernel/time/jiffies.c w/kernel/time/jiffies.c
> > > >     index 213ae1d6a014..400353b0d6f9 100644
> > > >     --- i/kernel/time/jiffies.c
> > > >     +++ w/kernel/time/jiffies.c
> > > >     @@ -136,6 +136,8 @@ static int sysctl_k2u_int_conv_userhz(bool *negp, ulong *u_ptr, const int *k_ptr
> > > >
> > > >      static ulong sysctl_msecs_to_jiffies(const ulong val)
> > > >      {
> > > >     +       if (val > UINT_MAX)
> > > >     +               return MAX_JIFFY_OFFSET;
> > >
> > > Given the nit at 2., it should be like this
> > >
> > > if (val > jiffies_to_msecs(MAX_JIFFY_OFFSET))
> > >     return MAX_JIFFY_OFFSET;
> > >
> > > as done in _msecs_to_jiffies().
> >
> > That is even better.
> >
> > Please send a V2 of the 3 fixes discussed and I'll push them through
> > next week.
>
> I have created [1] to test the fixes before pushing it upstream.
> If you are ok with all my changes contained in [1], there is no need to
> send V2. But if you want to change something, do send it out.

Patch 2 has a typo "incorrectlry", but otherwise looks good to me.
Feel free to push them with it fixed.

Thanks !


>
> Best
>
>
> [1] https://git.kernel.org/pub/scm/linux/kernel/git/joel.granados/linux.git/log/?h=jag/sysctl-fixes
>

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

end of thread, other threads:[~2026-09-10 16:43 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-09-05 23:36 [PATCH 0/2] sysctl: Restore minmax check for proc_do{int,ulong}vec_ms_jiffies_minmax() Kuniyuki Iwashima
2026-09-05 23:36 ` [PATCH 1/2] sysctl: Make proc_dointvec_ms_jiffies_minmax() enforce minmax again Kuniyuki Iwashima
2026-09-08 11:06   ` Joel Granados
2026-09-08 18:38     ` Kuniyuki Iwashima
2026-09-09 13:48       ` Joel Granados
2026-09-05 23:36 ` [PATCH 2/2] sysctl: Make proc_doulongvec_ms_jiffies_minmax() " Kuniyuki Iwashima
2026-09-08 12:15   ` Joel Granados
2026-09-08 18:44     ` Kuniyuki Iwashima
2026-09-09 13:44       ` Joel Granados
2026-09-09 17:07         ` Kuniyuki Iwashima
2026-09-10  9:04           ` Joel Granados
2026-09-10 10:45             ` Joel Granados
2026-09-10 16:43               ` Kuniyuki Iwashima

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