lkml.org 
[lkml]   [2010]   [Nov]   [23]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH v2] gpu: vga: limit kmalloc'ed memory size
On Tue, 23 Nov 2010 22:08:28 +0300
Vasiliy Kulikov <segoon@openwall.com> wrote:

> > And perhaps
> > strndup_user() needs __GFP_NOWARN treatment.
>
> strndup_user() silently return ERR_PTR(-EINVAL) if string is too long.
>
> > The code should be using strndup_user() anyway.
>
> Smth like this?
>
> diff --git a/drivers/gpu/vga/vgaarb.c b/drivers/gpu/vga/vgaarb.c
> index 09e3090..3afd249 100644
> --- a/drivers/gpu/vga/vgaarb.c
> +++ b/drivers/gpu/vga/vgaarb.c
> @@ -836,19 +836,10 @@ static ssize_t vga_arb_write(struct file *file, const char __user * buf,
> int ret_val;
> int i;
>
> - if (count > PAGE_SIZE)
> - count = PAGE_SIZE;
> -
> - kbuf = kmalloc(count + 1, GFP_KERNEL);
> - if (!kbuf)
> - return -ENOMEM;
> -
> - if (copy_from_user(kbuf, buf, count)) {
> - kfree(kbuf);
> - return -EFAULT;
> - }
> + kbuf = strndup_user(buf, PAGE_SIZE);
> + if (IS_ERR(kbuf))
> + return PTR_ERR(kbuf);
> curr_pos = kbuf;
> - kbuf[count] = '\0'; /* Just to make sure... */
>
> if (strncmp(curr_pos, "lock ", 5) == 0) {
> curr_pos += 5;

What I'm suggesting is that we simply do

kbuf = strndup_user(buf, count);

and make strndup_user() do the right thing if `count' turned out to be
crazy large. THis way we don't have to sprinkle decisions about "crazy
largeness" all over the kernel.

And the way in which I suggest that strndup_user() decides whether the
length is too great is to try to kmalloc that amount of memory.
If it succeeds then fine, proceed. If it fails then return an error,
probably ENOMEM. And that attempt to invoke kmalloc() shouldn't spew a
warning.




\
 
 \ /
  Last update: 2010-11-23 21:49    [W:0.047 / U:0.048 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site