* [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