Re: [PATCH v2] maccess: fix strncpy_from_user_nofault empty string handling

[Mykyta Yatsenko <mykyta.yatsenko5@gmail.com>]

On 4/23/25 01:20, Andrew Morton wrote:
> On Tue, 22 Apr 2025 14:14:49 +0100 Mykyta Yatsenko <mykyta.yatsenko5@gmail.com> wrote:
>
>> From: Mykyta Yatsenko <yatsenko@meta.com>
>>
>> strncpy_from_user_nofault should return the length of the copied string
>> including the trailing NUL, but if the argument unsafe_addr points to
>> an empty string ({'\0'}), the return value is 0.
>>
>> This happens as strncpy_from_user copies terminal symbol into dst
>> and returns 0 (as expected), but strncpy_from_user_nofault does not
>> modify ret as it is not equal to count and not greater than 0, so 0 is
>> returned, which contradicts the contract.
>>
>> ...
>>
> Thanks.
>
> Does this fix any known runtime issue?  If so, please fully describe this?
Not that I'm aware of. The issue could be found when trying to copy empty
user space string in BPF program (and relying on return value).There are 
some usage of
`strncpy_from_user_nofault` in tracing subsystem, but I'm not sure how to
hit those code paths.
>
>> --- a/mm/maccess.c
>> +++ b/mm/maccess.c
>> @@ -196,7 +196,7 @@ long strncpy_from_user_nofault(char *dst, const void __user *unsafe_addr,
>>   	if (ret >= count) {
>>   		ret = count;
>>   		dst[ret - 1] = '\0';
>> -	} else if (ret > 0) {
>> +	} else if (ret >= 0) {
>>   		ret++;
>>   	}
>>