linux-kernel - Re: [PATCH v2] maccess: fix strncpy_from_user

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <08e3ec4c-4401-403e-9d81-5ee0abebba5c@gmail.com>
Date: Wed, 23 Apr 2025 12:37:46 +0100
From: Mykyta Yatsenko <mykyta.yatsenko5@...il.com>
To: Andrew Morton <akpm@...ux-foundation.org>
Cc: linux-mm@...ck.org, rostedt@...dmis.org, mhiramat@...nel.org,
 andrii@...nel.org, kernel-team@...a.com, linux-kernel@...r.kernel.org,
 Mykyta Yatsenko <yatsenko@...a.com>, Kees Cook <keescook@...omium.org>
Subject: Re: [PATCH v2] maccess: fix strncpy_from_user_nofault empty string
 handling

On 4/23/25 01:20, Andrew Morton wrote:
> On Tue, 22 Apr 2025 14:14:49 +0100 Mykyta Yatsenko <mykyta.yatsenko5@...il.com> wrote:
>
>> From: Mykyta Yatsenko <yatsenko@...a.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++;
>>   	}
>>