linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: Andrew Morton <akpm@linux-foundation.org>
To: Dan Carpenter <dan.carpenter@oracle.com>
Cc: David Rientjes <rientjes@google.com>,
	Pekka Enberg <penberg@kernel.org>,
	Sasha Levin <levinsasha928@gmail.com>,
	lizf@cn.fujitsu.com, linux-kernel@vger.kernel.org,
	linux-mm@kvack.org, Tyler Hicks <tyhicks@canonical.com>,
	Dustin Kirkland <kirkland@canonical.com>,
	ecryptfs@vger.kernel.org
Subject: Re: [PATCH] mm: Don't warn if memdup_user fails
Date: Thu, 12 Jan 2012 23:36:00 -0800	[thread overview]
Message-ID: <20120112233600.33805bfc.akpm@linux-foundation.org> (raw)
In-Reply-To: <20120113071752.GA3802@mwanda>

On Fri, 13 Jan 2012 10:17:52 +0300 Dan Carpenter <dan.carpenter@oracle.com> wrote:

> On Thu, Jan 12, 2012 at 01:58:03PM -0800, Andrew Morton wrote:
> > On Thu, 12 Jan 2012 13:19:54 -0800 (PST)
> > David Rientjes <rientjes@google.com> wrote:
> > 
> > > On Thu, 12 Jan 2012, Pekka Enberg wrote:
> > > 
> > > > I think you missed Andrew's point. We absolutely want to issue a
> > > > kernel warning here because ecryptfs is misusing the memdup_user()
> > > > API. We must not let userspace processes allocate large amounts of
> > > > memory arbitrarily.
> > > > 
> > > 
> > > I think it's good to fix ecryptfs like Tyler is doing and, at the same 
> > > time, ensure that the len passed to memdup_user() makes sense prior to 
> > > kmallocing memory with GFP_KERNEL.  Perhaps something like
> > > 
> > > 	if (WARN_ON(len > PAGE_SIZE << PAGE_ALLOC_COSTLY_ORDER))
> > > 		return ERR_PTR(-ENOMEM);
> > > 
> > > in which case __GFP_NOWARN is irrelevant.
> > 
> > If someone is passing huge size_t's into kmalloc() and getting failures
> > then that's probably a bug.
> 
> It's pretty common to pass high values to kmalloc().  We've added
> a bunch of integer overflow checks recently where we do:
> 
> 	if (n > ULONG_MAX / size)
> 		return -EINVAL;

It would be cleaner to use kcalloc().  Except kcalloc() zeroes the memory
and we still don't have a non-zeroing kcalloc().

> The problem is that we didn't set a maximum bound before and we
> can't know which maximum will break compatibility.

Except for special cases (what are they?), code shouldn't be checking
for maximum kmalloc() size.  It should be checking the size against the
upper value which makes sense in the context of whatever it is doing at
the time.  This ecryptfs callsite is an example.

wrt any compatibility issues: the maximum amount of memory which can be
allocated by kmalloc() depends on the kernel config (see
kmalloc_sizes.h) so any code which is relying on any particular upper
bound is already busted.


--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

  reply	other threads:[~2012-01-13  7:31 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-01-11 16:50 [PATCH] mm: Don't warn if memdup_user fails Sasha Levin
2012-01-11 21:46 ` David Rientjes
2012-01-12  6:43   ` Pekka Enberg
2012-01-12  6:44     ` Pekka Enberg
2012-01-12  9:09       ` Li Zefan
2012-01-11 22:12 ` Andrew Morton
2012-01-12  7:12   ` Pekka Enberg
2012-01-12  8:06   ` Sasha Levin
2012-01-12  8:15     ` Pekka Enberg
2012-01-12 21:19       ` David Rientjes
2012-01-12 21:58         ` Andrew Morton
2012-01-12 22:29           ` David Rientjes
2012-01-13  7:17           ` Dan Carpenter
2012-01-13  7:36             ` Andrew Morton [this message]
2012-01-12 11:16     ` Tyler Hicks

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=20120112233600.33805bfc.akpm@linux-foundation.org \
    --to=akpm@linux-foundation.org \
    --cc=dan.carpenter@oracle.com \
    --cc=ecryptfs@vger.kernel.org \
    --cc=kirkland@canonical.com \
    --cc=levinsasha928@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=lizf@cn.fujitsu.com \
    --cc=penberg@kernel.org \
    --cc=rientjes@google.com \
    --cc=tyhicks@canonical.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;
as well as URLs for NNTP newsgroup(s).