Re: [PATCH v2 2/6] sysctl: Fixes nsm_local_state bounds

Nicolas Bouchinet <[email protected]>
Newsgroups gmane.linux.kernel,gmane.linux.drivers.rdma,gmane.linux.scsi,gmane.comp.file-systems.coda.general,gmane.linux.nfs
Message-ID <[email protected]>
On 2/24/25 15:38, Chuck Lever wrote:
> On 2/24/25 4:58 AM, [email protected] wrote:
>> From: Nicolas Bouchinet <[email protected]>
>>
>> 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 <[email protected]>
>> ---
>>   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
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.