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

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <553FE3F0.4060803@ezchip.com>
Date:	Tue, 28 Apr 2015 15:48:00 -0400
From:	Chris Metcalf <cmetcalf@...hip.com>
To:	Linus Torvalds <torvalds@...ux-foundation.org>,
	Al Viro <viro@...iv.linux.org.uk>
CC:	Fabian Frederick <fabf@...net.be>,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
	Randy Dunlap <rdunlap@...radead.org>,
	Rickard Strandqvist <rickard_strandqvist@...ctrumdigital.se>
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@...iv.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@...era.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@...radead.org>
     Reviewed-by: Rickard Strandqvist 
<rickard_strandqvist@...ctrumdigital.se>
     Signed-off-by: Chris Metcalf <cmetcalf@...era.com>


-- 
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@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/