linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Chris Metcalf <cmetcalf@ezchip.com>
To: Linus Torvalds <torvalds@linux-foundation.org>,
	Al Viro <viro@zeniv.linux.org.uk>
Cc: Fabian Frederick <fabf@skynet.be>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	Randy Dunlap <rdunlap@infradead.org>,
	Rickard Strandqvist <rickard_strandqvist@spectrumdigital.se>
Subject: Re: revert "fs/befs/linuxvfs.c: replace strncpy by strlcpy"
Date: Tue, 28 Apr 2015 15:48:00 -0400	[thread overview]
Message-ID: <553FE3F0.4060803@ezchip.com> (raw)
In-Reply-To: <CA+55aFy94RF6tS9ZGxcrLuUtSaSs-SYUbVbVv2E=xhm82L1wnQ@mail.gmail.com>

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

  reply	other threads:[~2015-04-28 19:48 UTC|newest]

Thread overview: 35+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-04-28  3:48 revert "fs/befs/linuxvfs.c: replace strncpy by strlcpy" Al Viro
2015-04-28  5:35 ` Fabian Frederick
2015-04-28 16:05   ` Al Viro
2015-04-28 16:42     ` Fabian Frederick
2015-04-28 17:39       ` Al Viro
2015-04-28 20:16         ` Fabian Frederick
2015-04-28 16:42     ` Linus Torvalds
2015-04-28 19:48       ` Chris Metcalf [this message]
2015-04-28 20:51         ` Linus Torvalds
2015-04-28 21:38           ` Chris Metcalf
2015-04-28 21:48             ` Linus Torvalds
2015-04-29  0:35               ` Al Viro
2015-04-29  8:24                 ` Geert Uytterhoeven
2015-04-30 16:01               ` [PATCH 0/3] add new strscpy() API for string copy Chris Metcalf
2015-04-30 16:01                 ` [PATCH 1/3] Make asm/word-at-a-time.h available on all architectures Chris Metcalf
2015-04-30 16:01                 ` [PATCH 2/3] string: provide strscpy() and strscpy_truncate() Chris Metcalf
2015-05-06 15:01                   ` Dan Carpenter
2015-05-06 15:21                     ` Chris Metcalf
2015-05-06 15:59                       ` Dan Carpenter
2015-05-06 16:45                         ` Geert Uytterhoeven
2015-05-07  9:00                           ` Dan Carpenter
2015-05-07 15:10                             ` Chris Metcalf
2015-04-30 16:01                 ` [PATCH 3/3] tile: use global strscpy() rather than private copy Chris Metcalf
2015-05-11 15:37                 ` [PATCH 0/3] add new strscpy() API for string copy Chris Metcalf
2015-05-14 23:10                 ` Michael Ellerman
2015-05-15 15:15                   ` Chris Metcalf
2015-05-18  1:13                     ` Michael Ellerman
2015-05-26 19:33                       ` Chris Metcalf
  -- strict thread matches above, loose matches on Subject: below --
2015-06-30 18:01 [GIT PULL] strscpy string copy function Chris Metcalf
2015-07-01 16:11 ` Linus Torvalds
2015-07-08 20:20   ` [PATCH v2 0/3] add new strscpy() API for string copy Chris Metcalf
2015-07-08 20:20     ` [PATCH v2 1/3] Make asm/word-at-a-time.h available on all architectures Chris Metcalf
2015-07-08 20:20     ` [PATCH v2 2/3] string: provide strscpy() Chris Metcalf
2015-07-08 20:54       ` Geert Uytterhoeven
2015-07-08 20:20     ` [PATCH v2 3/3] tile: use global strscpy() rather than private copy Chris Metcalf

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=553FE3F0.4060803@ezchip.com \
    --to=cmetcalf@ezchip.com \
    --cc=fabf@skynet.be \
    --cc=linux-kernel@vger.kernel.org \
    --cc=rdunlap@infradead.org \
    --cc=rickard_strandqvist@spectrumdigital.se \
    --cc=torvalds@linux-foundation.org \
    --cc=viro@zeniv.linux.org.uk \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).