From: Dan Carpenter <dan.carpenter@oracle.com>
To: Andrew Morton <akpm@linux-foundation.org>
Cc: Andi Kleen <ak@linux.intel.com>,
"H. Peter Anvin" <hpa@linux.intel.com>,
linux-kernel@vger.kernel.org,
Vegard Nossum <vegard.nossum@gmail.com>
Subject: Re: [patch 2/2] lib/string.c: strlcpy() might read too far
Date: Tue, 15 Apr 2014 13:38:37 +0300 [thread overview]
Message-ID: <20140415103837.GV4963@mwanda> (raw)
In-Reply-To: <20140414144615.8fd678c78389c19df4343049@linux-foundation.org>
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@oracle.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
next prev parent reply other threads:[~2014-04-15 10:38 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-04-01 10:08 [PATCH] isdnloop: NUL-terminate strings from userspace Vegard Nossum
2014-04-01 10:30 ` Hannes Frederic Sowa
2014-04-01 10:46 ` Vegard Nossum
2014-04-01 11:02 ` Hannes Frederic Sowa
2014-04-01 12:35 ` Dan Carpenter
2014-04-01 20:18 ` David Miller
2014-04-02 3:48 ` [PATCH] isdnloop: Validate NUL-terminated strings from user YOSHIFUJI Hideaki
2014-04-03 15:27 ` David Miller
2014-04-02 8:47 ` [patch 1/2] lib/string.c: use the name "C-string" in comments Dan Carpenter
2014-04-02 8:47 ` [patch 2/2] lib/string.c: strlcpy() might read too far Dan Carpenter
2014-04-14 21:46 ` Andrew Morton
2014-04-15 10:38 ` Dan Carpenter [this message]
2014-04-14 22:21 ` Dave Jones
2014-04-01 11:23 ` [PATCH] isdnloop: NUL-terminate strings from userspace YOSHIFUJI Hideaki
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=20140415103837.GV4963@mwanda \
--to=dan.carpenter@oracle.com \
--cc=ak@linux.intel.com \
--cc=akpm@linux-foundation.org \
--cc=hpa@linux.intel.com \
--cc=linux-kernel@vger.kernel.org \
--cc=vegard.nossum@gmail.com \
/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