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