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

Message-ID: <1326355594.1999.7.camel@lappy>
Date:	Thu, 12 Jan 2012 10:06:34 +0200
From:	Sasha Levin <levinsasha928@...il.com>
To:	Andrew Morton <akpm@...ux-foundation.org>
Cc:	lizf@...fujitsu.com, penberg@...nel.org,
	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 Wed, 2012-01-11 at 14:12 -0800, Andrew Morton wrote:
> There's nothing particularly special about memdup_user(): there are
> many ways in which userspace can trigger GFP_KERNEL allocations.
> 
> The problem here (one which your patch carefully covers up) is that
> ecryptfs_miscdev_write() is passing an unchecked userspace-provided
> `count' direct into kmalloc().  This is a bit problematic for other
> reasons: it gives userspace a way to trigger heavy reclaim activity and
> perhaps even to trigger the oom-killer.
> 
> A better fix here would be to validate the incoming arg before using
> it.  Preferably by running ecryptfs_parse_packet_length() before taking
> a copy of the data.  That would require adding a small copy_from_user()
> to peek at the message header. 

Let's split it to two parts: the specific ecryptfs issue I've given as
an example here, and a general view about memdup_user().

I fully agree that in the case of ecryptfs there's a missing validity
check, and just calling memdup_user() with whatever the user has passed
to it is wrong and dangerous. This should be fixed in the ecryptfs code
and I'll send a patch to do that.

The other part, is memdup_user() itself. Kernel warnings are usually
reserved (AFAIK) to cases where it would be difficult to notify the user
since it happens in a flow which the user isn't directly responsible
for.

memdup_user() is always located in path which the user has triggered,
and is usually almost the first thing we try doing in response to the
trigger. In those code flows it doesn't make sense to print a kernel
warnings and taint the kernel, instead we can simply notify the user
about that error and let him deal with it any way he wants.

There are more reasons kalloc() can show warnings besides just trying to
allocate too much, and theres no reason to dump kernel warnings when
it's easier to notify the user.

-- 

Sasha.

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