| Message ID | 20250224095826.16458-3-nicolas.bouchinet@clip-os.org (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Fixes multiple sysctl bound checks | expand |
On 2/24/25 4:58 AM, nicolas.bouchinet@clip-os.org wrote: > From: Nicolas Bouchinet <nicolas.bouchinet@ssi.gouv.fr> > > Bound nsm_local_state sysctl writings between SYSCTL_ZERO > and SYSCTL_INT_MAX. > > The proc_handler has thus been updated to proc_dointvec_minmax. > > Signed-off-by: Nicolas Bouchinet <nicolas.bouchinet@ssi.gouv.fr> > --- > fs/lockd/svc.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/fs/lockd/svc.c b/fs/lockd/svc.c > index 2c8eedc6c2cc9..984ab233af8b6 100644 > --- a/fs/lockd/svc.c > +++ b/fs/lockd/svc.c > @@ -461,7 +461,9 @@ static const struct ctl_table nlm_sysctls[] = { > .data = &nsm_local_state, > .maxlen = sizeof(int), > .mode = 0644, > - .proc_handler = proc_dointvec, > + .proc_handler = proc_dointvec_minmax, > + .extra1 = SYSCTL_ZERO, > + .extra2 = SYSCTL_INT_MAX, > }, > }; > Hi Nicolas - nsm_local_state is an unsigned 32-bit integer. The type of that value is defined by spec, because this value is exchanged between peers on the network. Perhaps this patch should replace proc_dointvec with proc_douintvec instead.
On 2/24/25 15:38, Chuck Lever wrote: > On 2/24/25 4:58 AM, nicolas.bouchinet@clip-os.org wrote: >> From: Nicolas Bouchinet <nicolas.bouchinet@ssi.gouv.fr> >> >> Bound nsm_local_state sysctl writings between SYSCTL_ZERO >> and SYSCTL_INT_MAX. >> >> The proc_handler has thus been updated to proc_dointvec_minmax. >> >> Signed-off-by: Nicolas Bouchinet <nicolas.bouchinet@ssi.gouv.fr> >> --- >> fs/lockd/svc.c | 4 +++- >> 1 file changed, 3 insertions(+), 1 deletion(-) >> >> diff --git a/fs/lockd/svc.c b/fs/lockd/svc.c >> index 2c8eedc6c2cc9..984ab233af8b6 100644 >> --- a/fs/lockd/svc.c >> +++ b/fs/lockd/svc.c >> @@ -461,7 +461,9 @@ static const struct ctl_table nlm_sysctls[] = { >> .data = &nsm_local_state, >> .maxlen = sizeof(int), >> .mode = 0644, >> - .proc_handler = proc_dointvec, >> + .proc_handler = proc_dointvec_minmax, >> + .extra1 = SYSCTL_ZERO, >> + .extra2 = SYSCTL_INT_MAX, >> }, >> }; >> > Hi Nicolas - > > nsm_local_state is an unsigned 32-bit integer. The type of that value is > defined by spec, because this value is exchanged between peers on the > network. > > Perhaps this patch should replace proc_dointvec with proc_douintvec > instead. > > Hi Chuck, Thank's for your review. If `nsm_local_state` should be set to the full range of an uint32_t by a user writing in the sysctl, then yes it should use `proc_douintvec` instead of limiting it to SYSCTL_INT_MAX value (INT_MAX). I've used `proc_dointvec_minmax` since it already used `proc_dointvec` and thus was already capped at INT_MAX. Best regards, Nicolas
diff --git a/fs/lockd/svc.c b/fs/lockd/svc.c index 2c8eedc6c2cc9..984ab233af8b6 100644 --- a/fs/lockd/svc.c +++ b/fs/lockd/svc.c @@ -461,7 +461,9 @@ static const struct ctl_table nlm_sysctls[] = { .data = &nsm_local_state, .maxlen = sizeof(int), .mode = 0644, - .proc_handler = proc_dointvec, + .proc_handler = proc_dointvec_minmax, + .extra1 = SYSCTL_ZERO, + .extra2 = SYSCTL_INT_MAX, }, };