Patchwork [BUG,MTD] number of bytes returned in mtd_oob_buf.start instead of length in MEMREADOOB.

login
register
mail settings
Submitter Shigeru Moriwake
Date Jan. 8, 2009, 12:09 a.m.
Message ID <20090108000931.DFF4449D1B8@mta03.sharp.co.jp>
Download mbox | patch
Permalink /patch/17264/
State New
Headers show

Comments

Shigeru Moriwake - Jan. 8, 2009, 12:09 a.m.
Hi there,

I found what looks like a bug in MEMREADOOB ioctl introduced in 2.6.8.  Can
anybody confirm this?

The API reads the OOB, and return the data and number of bytes read, in
struct mtd_oob_buf.  However, the number of bytes goes into
mtd_oob_buf.start instead of mtd_oob_buf.length in the current code.

This bug is not in MEMWRITEOOB, or kernels prior to 2.6.8-rc1-bk8.  I was
not able to locate the specific mail which introduced this bug.

The argument to put_user() points to the head of mtd_oob_buf, which is
member start.  The member length is at an offset, so the fix is below.

----

Thanks,
Shigeru Moriwake

Patch

--- drivers-mtd-mtdchar.c.orig  2009-01-07 17:35:18.000000000 +0900
+++ drivers-mtd-mtdchar.c       2009-01-07 17:42:36.000000000 +0900
@@ -579,17 +579,17 @@ 

                ops.oobbuf = kmalloc(buf.length, GFP_KERNEL);
                if (!ops.oobbuf)
                        return -ENOMEM;

                buf.start &= ~(mtd->oobsize - 1);
                ret = mtd->read_oob(mtd, buf.start, &ops);

-               if (put_user(ops.oobretlen, (uint32_t __user *)argp))
+               if (put_user(ops.oobretlen, (uint32_t __user *)(argp + 
+ offsetof(mtd_oob_buf, length))))
                        ret = -EFAULT;
                else if (ops.oobretlen && copy_to_user(buf.ptr, ops.oobbuf,
                                                    ops.oobretlen))
                        ret = -EFAULT;

                kfree(ops.oobbuf);
                break;
        }