linux-kernel - Re: [patch 2/2] lib/string.c: strlcpy() might read too far

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] [day] [month] [year] [list]

Message-ID: <20140415103837.GV4963@mwanda>
Date:	Tue, 15 Apr 2014 13:38:37 +0300
From:	Dan Carpenter <dan.carpenter@...cle.com>
To:	Andrew Morton <akpm@...ux-foundation.org>
Cc:	Andi Kleen <ak@...ux.intel.com>,
	"H. Peter Anvin" <hpa@...ux.intel.com>,
	linux-kernel@...r.kernel.org,
	Vegard Nossum <vegard.nossum@...il.com>
Subject: Re: [patch 2/2] lib/string.c: strlcpy() might read too far

On Mon, Apr 14, 2014 at 02:46:15PM -0700, Andrew Morton wrote:
> On Wed, 2 Apr 2014 11:47:31 +0300 Dan Carpenter <dan.carpenter@...cle.com> wrote:
> 
> > Imagine you have a user controlled variable at the end of a struct which
> > is allocated at the end of a page.  The strlen() could read beyond the
> > mapped memory and cause an oops.
> 
> Well, it's hard to conceive of a situation where anything like this
> could happen.  Code which copies a string from userspace should
> immediately ensure that the kernel copy is null-terminated.  But...  I
> guess it's defense in depth.


The code which prompted this is in isdnloop_start().  I did a:

-	strcpy(card->s0num[0], sdef.num[0]);
+	strlcpy(card->s0num[0], sdef.num[0], sizeof(card->s0num[0]));

So that we wouldn't corrupt memory.  But the debate was whether that was
enough to solve the problem or it was better to null-terminate
sdef.num[0] before doing the strlcpy().  In the end, we decided to
reject input if it wasn't null terminated.

In this case sdef is declared on the stack so there lots NUL characters
so reading too far in the strlen() is not a problem.

regards,
dan carpenter

--
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/