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

[Al Viro <viro@ZenIV.linux.org.uk>]

On Tue, Apr 28, 2015 at 06:42:10PM +0200, Fabian Frederick wrote:
> 
> 
> > On 28 April 2015 at 18:05 Al Viro <viro@ZenIV.linux.org.uk> wrote:
> >
> >
> > On Tue, Apr 28, 2015 at 07:35:10AM +0200, Fabian Frederick wrote:
> >
> > > > Al, very unhappy about the prospect of looking through ~2000 calls of
> > > > strlcpy()
> > > > we have in the tree...
> > >
> > > Sorry Al, I thought it was more secure.
> >
> > It's not just you, unfortunately, and dumping all that annoyance on you
> > as a proxy for everyone who does that kind of thing had been unfair.
> > My apologies...
> 
> No problem Al :) but why can't we harden strlcpy at first with
> something like a strlen limited to max char.
> (I don't know if it's already in kernel libs).
> 
> size_t strlenl(const char *s, size_t maxlen)

aka strnlen()

>         const char *sc = s;
>         size_t i = 0;
> 
>         while (*sc != '\0' && (i < maxlen)) {
>                 i++;
>                 sc++;
>         }
>         return sc - s;
> }
> 
> Then we could solve problems downstream ...

Can't.  Seriously, look what strlcpy() is supposed to return; it's pretty
much a microoptimized snprintf(dst, size, "%s", src).  It's certainly
been patterned after snprintf(3) - "don't exceed that size, NUL-terminate
unless the size is zero, return the number of characters (excluding NUL)
that would've been written if the size had been large enough".

The following is a legitimate use of strlcpy():

int foo(char *);	/* modifies string */

int const_foo(const char *s)
{
	int res;
	char buf[32], *p = buf;
	size_t wanted = strlcpy(buf, sizeof(buf), s);
	if (wanted >= sizeof(buf)) {
		p = malloc(wanted + 1);
		if (!p)
			return -ENOMEM;
		memcpy(p, s, wanted + 1);
	}
	res = foo(p);
	if (p != buf)
		free(p);
	return res;
}

None of the kernel callers are of exactly that form (and most ignore the
return value completely), but if we make that sucker return something
different from what strlcpy(3) would return, we'd damn better _not_ keep
the name; there's enough confusion in that area as it is.