linux-kernel - Re: [PATCH] inotify: Use strscpy() for event->name copies

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 for Android: free password hash cracker in your pocket

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

Message-ID: <41850E19-F71C-4D69-91E6-7B095A9FFD5C@kernel.org>
Date: Mon, 16 Dec 2024 23:51:52 -0800
From: Kees Cook <kees@...nel.org>
To: Matthew Wilcox <willy@...radead.org>
CC: Jan Kara <jack@...e.cz>, Amir Goldstein <amir73il@...il.com>,
 linux-fsdevel@...r.kernel.org, linux-kernel@...r.kernel.org,
 linux-hardening@...r.kernel.org
Subject: Re: [PATCH] inotify: Use strscpy() for event->name copies



On December 16, 2024 8:12:07 PM PST, Matthew Wilcox <willy@...radead.org> wrote:
>On Mon, Dec 16, 2024 at 02:45:15PM -0800, Kees Cook wrote:
>> Since we have already allocated "len + 1" space for event->name, make sure
>> that name->name cannot ever accidentally cause a copy overflow by calling
>> strscpy() instead of the unbounded strcpy() routine. This assists in
>> the ongoing efforts to remove the unsafe strcpy() API[1] from the kernel.
>
>Since a qstr can't contain a NUL before the length, why not just use
>memcpy()?
>
>>  	event->name_len = len;
>>  	if (len)
>> -		strcpy(event->name, name->name);
>> +		strscpy(event->name, name->name, event->name_len + 1);

So that the destination is guaranteed to be NUL terminated no matter what's in the source. :) (i.e. try to limit unlikely conditions from expanding.)

-- 
Kees Cook