lkml.org 
[lkml]   [2010]   [Dec]   [14]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    SubjectRe: [PATCH 1/1] mtd: nand: add check for out of page read
    From
    Date
    On Fri, 2010-11-19 at 16:40 +0800, Jason Liu wrote:
    > When run mtd_oobtest case, there will be one error for step(4),
    > which turned out it need add one check for out of page read in
    > nand_do_read_oob just like mtd_do_write_oob did it already.
    > This commit also fix one typo error for comments in mtd_do_write_oob
    >
    > Signed-off-by: Jason Liu <r64343@freescale.com>
    > ---
    > drivers/mtd/nand/nand_base.c | 9 ++++++++-
    > 1 files changed, 8 insertions(+), 1 deletions(-)
    >
    > diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
    > index 1f75a1b..75d199e 100644
    > --- a/drivers/mtd/nand/nand_base.c
    > +++ b/drivers/mtd/nand/nand_base.c
    > @@ -1782,6 +1782,13 @@ static int nand_do_read_oob(struct mtd_info *mtd, loff_t from,
    > else
    > len = mtd->oobsize;
    >
    > + /* Do not allow read past end of page */
    > + if ((ops->ooboffs + readlen) > len) {
    > + DEBUG(MTD_DEBUG_LEVEL0, "%s: Attempt to read "
    > + "past end of page\n", __func__);
    > + return -EINVAL;
    > + }

    As you reported to me in a private e-mail (although I prefer to always
    have a public ML in CC when dealing with open source stuff) this patch
    is wrong. Indeed, it limits the maximum amount of bytes which can be
    read at one go to the OOB size, which is incorrect, because
    mtd->read_oob() allows reading multiple pages at a time, see comment
    near "struct mtd_oob_ops" at include/linux/mtd/mtd.h. So this patch
    breaks ABI and hence, has to be reverted.

    > if (unlikely(ops->ooboffs >= len)) {
    > DEBUG(MTD_DEBUG_LEVEL0, "%s: Attempt to start read "
    > "outside oob\n", __func__);

    Side note: nand_base.c has a bunch of senseless unlikely() hints, would
    be nice to clean that up some day.

    > - /* Do not allow reads past end of device */
    > + /* Do not allow write past end of device */

    Care to make this a separate clean-up patch meanwhile?

    Thank!

    --
    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: 2010-12-14 16:15    [W:3.245 / U:0.016 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site