lkml.org 
[lkml]   [2006]   [Apr]   [17]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH] Remove unnecessary kmalloc/kfree calls in mtdchar
    On 4/14/06, Thiago Galesi <thiagogalesi@gmail.com> wrote:
    > This patch removes the use of repeated calls to kmalloc / kfree when
    > writing / reading from a MTD char device. Not the ideal solution
    > mentioned in the driver, but nonetheless better.

    NAK. This patch introduces a bug. See below.

    >
    > Index: linux-2.6.16.2/drivers/mtd/mtdchar.c
    > ===================================================================
    > --- linux-2.6.16.2.orig/drivers/mtd/mtdchar.c
    > +++ linux-2.6.16.2/drivers/mtd/mtdchar.c
    > @@ -170,15 +170,18 @@ static ssize_t mtd_read(struct file *fil
    >
    > /* FIXME: Use kiovec in 2.5 to lock down the user's buffers
    > and pass them directly to the MTD functions */
    > - while (count) {
    > - if (count > MAX_KMALLOC_SIZE)
    > - len = MAX_KMALLOC_SIZE;
    > - else
    > - len = count;
    >
    > - kbuf=kmalloc(len,GFP_KERNEL);
    > - if (!kbuf)
    > - return -ENOMEM;
    > + if (count > MAX_KMALLOC_SIZE)
    > + len = MAX_KMALLOC_SIZE;
    > + else
    > + len = count;

    Now that len is set outside of the loop, it is always the same size.
    If count is large enough to require more than a single read, the the
    original size will still be used and it could overflow the user's
    buffer.

    I agree that doing the kmallocs in a loop looks nasty. But we need to
    make sure moving out of the loop doesn't break things.

    josh
    -
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

    \
     
     \ /
      Last update: 2006-04-17 14:10    [W:0.021 / U:183.416 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site