On 2012-01-12 10:06:34, Sasha Levin wrote: > 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. I just wrote up a patch for the eCryptfs portion. I'll send it out a little later after I get a chance to test it. Tyler > > 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. >
Attachment:
signature.asc
Description: Digital signature