Subject: Re: [PATCH v2] netdevsim: Add trailing zero to terminate the string in nsim_nexthop_bucket_activity_write()

From: Petr Machata <[email-masked]>
Date: Thu, 24 Oct 2024 14:15:54 +0200

Simon Horman <[email-masked]> writes:

> On Tue, Oct 22, 2024 at 12:19:08PM -0500, Gax-c wrote:
>
>> diff --git a/drivers/net/netdevsim/fib.c b/drivers/net/netdevsim/fib.c
>> index 41e80f78b316..16c382c42227 100644
>> --- a/drivers/net/netdevsim/fib.c
>> +++ b/drivers/net/netdevsim/fib.c
>> @@ -1377,10 +1377,12 @@ static ssize_t nsim_nexthop_bucket_activity_write(struct file *file,
>>  
>>  	if (pos != 0)
>>  		return -EINVAL;
>> -	if (size > sizeof(buf))
>> +	if (size > sizeof(buf) - 1)
>
> I don't think this change for the best.
> If the input data is well formatted it will end with a '\0'.
> Which may be copied into the last byte of buf.
>
> With this change the maximum size of the input data is
> unnecessarily reduced by one.

The buffer is 128 bytes, and sscanf'd into it is a u32 and u16, say 18
bytes or so total. Arguably the buffer is unnecessarily large. I think
the -1 above doesn't hurt.

Though if (user_buf[size - 1]) return -EINVAL; would work, too?