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: <20150428173957.GK889@ZenIV.linux.org.uk>
Date:	Tue, 28 Apr 2015 18:39:57 +0100
From:	Al Viro <viro@...IV.linux.org.uk>
To:	Fabian Frederick <fabf@...net.be>
Cc:	linux-kernel@...r.kernel.org,
	Linus Torvalds <torvalds@...ux-foundation.org>
Subject: Re: revert "fs/befs/linuxvfs.c: replace strncpy by strlcpy"

On Tue, Apr 28, 2015 at 06:42:10PM +0200, Fabian Frederick wrote:
> 
> 
> > On 28 April 2015 at 18:05 Al Viro <viro@...IV.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.
--
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/