Messages in this thread | | | Date | Tue, 28 Apr 2015 15:48:00 -0400 | From | Chris Metcalf <> | Subject | Re: revert "fs/befs/linuxvfs.c: replace strncpy by strlcpy" |
| |
On 04/28/2015 12:42 PM, Linus Torvalds wrote: > On Tue, Apr 28, 2015 at 9:05 AM, Al Viro <viro@zeniv.linux.org.uk> wrote: >> >> Unfortunately, we _can't_ make strlcpy() never look past src + size - 1 - >> not without changing its semantics. > > Yeah, strlcpy is actually nasty. I don't understand why people like it > so much. It's a crap interface, and completely unsuitable for > untrusted sources because of the overrun issue.
FWIW, I wanted to deal with some strncpy/strlcpy API issues last year and just put a "strscpy()" function in arch/tile/gxio/mpipe.c, with a comment saying it might be worth making it global at a later date, but at the time only the reviewers seemed interested in making it happen, so I let that possibility die on the vine.
I argue that truncated strings are often pretty nasty, so you don't want to just silently say "Sure, I made it fit!" but instead make that be a failure case. In principle you could keep the return code of my proposed strscpy() and still do the truncated-string copy, but I think it's a mistake in almost every case, and if you really want that semantics, you should be forced to use something harder (e.g. some combination of explicit strlen and memcpy) so it's clear you know what you're doing.
commit bceb7efa6a7e656bfaa67b6f54925e7db75bcd52 Author: Chris Metcalf <cmetcalf@tilera.com> Date: Tue Sep 2 16:25:22 2014 -0400
tile gxio: use better string copy primitive
Both strncpy and strlcpy suffer from the fact that they do partial copies of strings into the destination when the target buffer is too small. This is frequently pointless since an overflow of the target buffer may make the result invalid.
strncpy() makes it relatively hard to even detect the error condition, and with strlcpy() you have to duplicate the buffer size parameter to test to see if the result exceeds it. By returning zero in the failure case, we both make testing for it easy, and by simply not copying anything in that case, we make it mandatory for callers to test the error code.
To catch lazy programmers who don't check, we also place a NUL at the start of the destination buffer (if there is space) to ensure that the result is an invalid string.
At some point it may make sense to promote strscpy() to a global platform-independent function, but other than the reviewers, no one was interested on LKML, so for now leave the strscpy() function as file-static.
Reviewed-by: Randy Dunlap <rdunlap@infradead.org> Reviewed-by: Rickard Strandqvist <rickard_strandqvist@spectrumdigital.se> Signed-off-by: Chris Metcalf <cmetcalf@tilera.com>
-- Chris Metcalf, EZChip Semiconductor http://www.ezchip.com
| |