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
| ||
|
Message-ID: <202310182248.9E197FFD5@keescook> Date: Wed, 18 Oct 2023 23:01:54 -0700 From: Kees Cook <keescook@...omium.org> To: Christoph Hellwig <hch@....de> Cc: Justin Stitt <justinstitt@...gle.com>, Keith Busch <kbusch@...nel.org>, Jens Axboe <axboe@...nel.dk>, Sagi Grimberg <sagi@...mberg.me>, linux-nvme@...ts.infradead.org, linux-kernel@...r.kernel.org, linux-hardening@...r.kernel.org, ksummit@...ts.linux.dev Subject: Re: the nul-terminated string helper desk chair rearrangement On Thu, Oct 19, 2023 at 07:46:42AM +0200, Christoph Hellwig wrote: > On Wed, Oct 18, 2023 at 10:48:49PM +0000, Justin Stitt wrote: > > strncpy() is deprecated for use on NUL-terminated destination strings > > [1] and as such we should prefer more robust and less ambiguous string > > interfaces. > > If we want that we need to stop pretendening direct manipulation of > nul-terminate strings is a good idea. I suspect the churn of replacing > one helper with another, maybe slightly better, one probably > introduces more bugs than it fixes. > > If we want to attack the issue for real we need to use something > better. > > lib/seq_buf.c is a good start for a lot of simple cases that just > append to strings including creating complex ones. Kent had a bunch > of good ideas on how to improve it, but couldn't be convinced to > contribute to it instead of duplicating the functionality which > is a bit sad, but I think we need to switch to something like > seq_buf that actually has a counted string instead of all this messing > around with the null-terminated strings. When doing more complex string creation, I agree. I spent some time doing this while I was looking at removing strcat() and strlcat(); this is where seq_buf shines. (And seq_buf is actually both: it maintains its %NUL termination _and_ does the length counting.) The only thing clunky about it was initialization, but all the conversions I experimented with were way cleaner using seq_buf. I even added a comment to strlcat()'s kern-doc to aim folks at seq_buf. :) /** * strlcat - Append a string to an existing string ... * Do not use this function. While FORTIFY_SOURCE tries to avoid * read and write overflows, this is only possible when the sizes * of @p and @q are known to the compiler. Prefer building the * string with formatting, via scnprintf(), seq_buf, or similar. Almost all of the remaining strncpy() usage is just string to string copying, but the corner cases that are being spun out that aren't strscpy() or strscpy_pad() are covered by strtomem(), kmemdup_nul(), and memcpy(). Each of these are a clear improvement since they remove the ambiguity of the intended behavior. Using seq_buf ends up being way more overhead than is needed. -Kees -- Kees Cook
Powered by blists - more mailing lists