lkml.org 
[lkml]   [2011]   [Jan]   [16]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
SubjectRe: [PATCH 1/1] mtd: nand: add check for out of page read
From
Date
On Thu, 2010-12-23 at 14:06 +0800, Jason Liu wrote:
> Hi, Artem,
>
> 2010/12/20 Artem Bityutskiy <dedekind1@gmail.com>:
> > On Wed, 2010-12-15 at 09:55 +0800, Jason Liu wrote:
> >> > /* Do not allow reads past end of device */
> >> > if (unlikely(from >= mtd->size ||
> >> > ops->ooboffs + readlen > ((mtd->size >> chip->page_shift) -
> >> > (from >> chip->page_shift)) * len)) {
> >> > DEBUG(MTD_DEBUG_LEVEL0, "%s: Attempt read beyond end "
> >> > "of device\n", __func__);
> >> > return -EINVAL;
> >> > }
> >>
> >> Here the mtd->size in nand_base.c should be the NAND flash chip size,
> >
> > I think this is partition size as well.
>
> I think you are wrong. This should be the NAND flash chip size not the
> partition size.

Yes, thanks for pointing this.

The patch below fixes the issue you found. I've also pushed this patch
to my l2-mtd-2.6 tree. If you can test it, please, do, but it fixes the
issue when nandsim is used.


From 2b6dc448005afdfe48ac2096d24694b8ea3ac88b Mon Sep 17 00:00:00 2001
From: Artem Bityutskiy <Artem.Bityutskiy@nokia.com>
Date: Sun, 16 Jan 2011 17:50:54 +0200
Subject: [PATCH] mtd: mtdpart: disallow reading OOB past the end of the partition

This patch fixes the mtdpart bug which allows users reading OOB past the
end of the partition. This happens because 'part_read_oob()' allows reading
multiple OOB areas in one go, and mtdparts does not validate the OOB
length in the request.

Although there is such check in 'nand_do_read_oob()' in nand_base.c, but
it checks that we do not read past the flash chip, not the partition,
because in nand_base.c we work with the whole chip (e.g., mtd->size
in nand_base.c is the size of the whole chip). So this check cannot
be done correctly in nand_base.c and should be instead done in mtdparts.c.

This problem was reported by Jason Liu <r64343@freescale.com> and reproduced
with nandsim:

$ modprobe nandsim first_id_byte=0x20 second_id_byte=0xaa third_id_byte=0x00 \
fourth_id_byte=0x15 parts=0x400,0x400
$ modprobe nandsim mtd_oobtest.ko dev=0
$ dmesg
= snip =
mtd_oobtest: attempting to read past end of device
mtd_oobtest: an error is expected...
mtd_oobtest: error: read past end of device
= snip =
mtd_oobtest: finished with 2 errors

Reported-by: Jason Liu <liu.h.jason@gmail.com>
Signed-off-by: Artem Bityutskiy <Artem.Bityutskiy@nokia.com>
---
drivers/mtd/mtdpart.c | 19 ++++++++++++++++++-
1 files changed, 18 insertions(+), 1 deletions(-)

diff --git a/drivers/mtd/mtdpart.c b/drivers/mtd/mtdpart.c
index 79e3689..d2b932f 100644
--- a/drivers/mtd/mtdpart.c
+++ b/drivers/mtd/mtdpart.c
@@ -120,8 +120,25 @@ static int part_read_oob(struct mtd_info *mtd, loff_t from,
return -EINVAL;
if (ops->datbuf && from + ops->len > mtd->size)
return -EINVAL;
- res = part->master->read_oob(part->master, from + part->offset, ops);

+ /*
+ * If OOB is also requested, make sure that we do not read past the end
+ * of this partition.
+ */
+ if (ops->oobbuf) {
+ size_t len, pages;
+
+ if (ops->mode == MTD_OOB_AUTO)
+ len = mtd->oobavail;
+ else
+ len = mtd->oobsize;
+ pages = mtd_div_by_ws(mtd->size, mtd);
+ pages -= mtd_div_by_ws(from, mtd);
+ if (ops->ooboffs + ops->ooblen > pages * len)
+ return -EINVAL;
+ }
+
+ res = part->master->read_oob(part->master, from + part->offset, ops);
if (unlikely(res)) {
if (res == -EUCLEAN)
mtd->ecc_stats.corrected++;
--
1.7.3.4
--
Best Regards,
Artem Bityutskiy (Битюцкий Артём)

--
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: 2011-01-16 17:01    [W:0.492 / U:0.020 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site