Re: revert "fs/befs/linuxvfs.c: replace strncpy by strlcpy"

[Chris Metcalf]

On 04/28/2015 12:42 PM, Linus Torvalds wrote:
> On Tue, Apr 28, 2015 at 9:05 AM, Al Viro <viro@xxxxxxxxxxxxxxxxxx> 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@xxxxxxxxxx>
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@xxxxxxxxxxxxx>
    Reviewed-by: Rickard Strandqvist 
<rickard_strandqvist@xxxxxxxxxxxxxxxxxx>
    Signed-off-by: Chris Metcalf <cmetcalf@xxxxxxxxxx>


--
Chris Metcalf, EZChip Semiconductor
http://www.ezchip.com
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/