linux-kernel - Re: [PATCH] mm: Don't warn if memdup

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:	Thu, 12 Jan 2012 23:36:00 -0800
From:	Andrew Morton <akpm@...ux-foundation.org>
To:	Dan Carpenter <dan.carpenter@...cle.com>
Cc:	David Rientjes <rientjes@...gle.com>,
	Pekka Enberg <penberg@...nel.org>,
	Sasha Levin <levinsasha928@...il.com>, lizf@...fujitsu.com,
	linux-kernel@...r.kernel.org, linux-mm@...ck.org,
	Tyler Hicks <tyhicks@...onical.com>,
	Dustin Kirkland <kirkland@...onical.com>,
	ecryptfs@...r.kernel.org
Subject: Re: [PATCH] mm: Don't warn if memdup_user fails

On Fri, 13 Jan 2012 10:17:52 +0300 Dan Carpenter <dan.carpenter@...cle.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@...gle.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 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/