Re: [PATCH 1/1] mtd: nand: add check for out of page read

[Jason Liu]

Hi, Artem,

2010/12/14 Artem Bityutskiy <dedekind1@xxxxxxxxx>:
> 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@xxxxxxxxxxxxx>
>
> I cannot reproduce this with nandsim, can you?
>
> I tried:
>
> 1. sudo modprobe nandsim
>   sudo modprobe mtd_oobtest dev=0
>
> 2. sudo ./load_nandsim.sh 128 128 2048
>   sudo modprobe mtd_oobtest dev=0
>
> In both cases the test passes: "mtd_oobtest: finished with 0 errors"
>
> And in 'nand_do_read_oob()' I see the following piece of code:
>
>        /* 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,
while in nand_oobtest,

              /* Attempt to read off end of device */
                ops.mode      = MTD_OOB_AUTO;
                ops.len       = 0;
                ops.retlen    = 0;
                ops.ooblen    = mtd->ecclayout->oobavail;
                ops.oobretlen = 0;
                ops.ooboffs   = 1;
                ops.datbuf    = NULL;
                ops.oobbuf    = readbuf;
                printk(PRINT_PREF "attempting to read past end of device\n");
                printk(PRINT_PREF "an error is expected...\n");
                err = mtd->read_oob(mtd, mtd->size - mtd->writesize, &ops);
                if (err) {
                        printk(PRINT_PREF "error occurred as expected\n");
                        err = 0;
                } else {
                        printk(PRINT_PREF "error: read past end of device\n");
                        errcnt += 1;
                }
        }

here, mtd->size is mtd partition size right? when the mtd partition is
not the last MTD partition, the error will happen.

I have tested on real NAND, while not on nandsim.

>
> which should handle the case AFAICS.
>
> So, although I did think hard about this, it looks like there is no
> problem. What is the kernel version you are looking at? I checked it
> with my l2-mtd-2.6.git, which is the latest mtd-2.6.git plus a revert
> commit of your patch:
>
> http://git.infradead.org/users/dedekind/l2-mtd-2.6.git

Yes, as I have written the email to you before in a private email to
tell that this patch has issue and please drop it.

Thanks,

>
> --
> Best Regards,
> Artem Bityutskiy (Артём Битюцкий)
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/