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
| ||
|
Date: Tue, 5 Jan 2021 14:29:46 +0530 From: Dwaipayan Ray <dwaipayanray1@...il.com> To: Joe Perches <joe@...ches.com> Cc: linux-kernel-mentees@...ts.linuxfoundation.org, linux-kernel <linux-kernel@...r.kernel.org>, Lukas Bulwahn <lukas.bulwahn@...il.com> Subject: Re: [PATCH] checkpatch: add a new check for strcpy/strlcpy uses On Tue, Jan 5, 2021 at 2:14 PM Joe Perches <joe@...ches.com> wrote: > > On Tue, 2021-01-05 at 13:53 +0530, Dwaipayan Ray wrote: > > strcpy() performs no bounds checking on the destination buffer. > > This could result in linear overflows beyond the end of the buffer. > > > > strlcpy() reads the entire source buffer first. This read > > may exceed the destination size limit. This can be both inefficient > > and lead to linear read overflows. > > > > The safe replacement to both of these is to use strscpy() instead. > > Add a new checkpatch warning which alerts the user on finding usage of > > strcpy() or strlcpy(). > > I do not believe that strscpy is preferred over strcpy. > > When the size of the output buffer is known to be larger > than the input, strcpy is faster. > > There are about 2k uses of strcpy. > Is there a use where strcpy use actually matters? > I don't know offhand... > > But I believe compilers do not optimize away the uses of strscpy > to a simple memcpy like they do for strcpy with a const from > > strcpy(foo, "bar"); > Yes the optimization here definitely helps. So in case the programmer knows that the destination buffer is always larger, then strcpy() should be preferred? I think the documentation might have been too strict about strcpy() uses here: Documentation/process/deprecated.rst: "strcpy() performs no bounds checking on the destination buffer. This could result in linear overflows beyond the end of the buffer, leading to all kinds of misbehaviors. While `CONFIG_FORTIFY_SOURCE=y` and various compiler flags help reduce the risk of using this function, there is no good reason to add new uses of this function. The safe replacement is strscpy(),..." > And lastly there is a existing strlcpy test in checkpatch. > > commit 5dbdb2d87c29 ("checkpatch: prefer strscpy to strlcpy") > I will drop this patch. Thanks for your view. Thank you, Dwaipayan.
Powered by blists - more mailing lists