Re: [PATCH] mtd: mtd_oobtest: fix verify errors due to incorrect useof prandom_bytes_state()

[Akinobu Mita]

2014/1/23 Lothar Waßmann <LW@xxxxxxxxxxxxxxxxxxx>:
> Hi,
>
> Akinobu Mita wrote:
>> 2014/1/22 Lothar Waßmann <LW@xxxxxxxxxxxxxxxxxxx>:
>> > Hi,
>> >
>> > Is anyone taking care of this?
>> >
>> > Lothar Waßmann wrote:
>> >> When using prandom_bytes_state() it is critical to use the same block
>> >> size in all invocations that are to produce the same random sequence.
>> >> Otherwise the state of the PRNG will be out of sync if the blocksize
>> >> is not divisible by 4.
>> >> This leads to bogus verification errors in several tests which use
>> >> different block sizes to initialize the buffer for writing and
>> >> comparison.
>> >>
>> >> Signed-off-by: Lothar Waßmann <LW@xxxxxxxxxxxxxxxxxxx>
>> >> ---
>> >>  drivers/mtd/tests/oobtest.c |   14 ++++++++++++--
>> >>  1 files changed, 12 insertions(+), 2 deletions(-)
>> >>
>> >> diff --git a/drivers/mtd/tests/oobtest.c b/drivers/mtd/tests/oobtest.c
>> >> index 2e9e2d1..72c7359 100644
>> >> --- a/drivers/mtd/tests/oobtest.c
>> >> +++ b/drivers/mtd/tests/oobtest.c
>> >> @@ -213,8 +213,15 @@ static int verify_eraseblock_in_one_go(int ebnum)
>> >>       int err = 0;
>> >>       loff_t addr = ebnum * mtd->erasesize;
>> >>       size_t len = mtd->ecclayout->oobavail * pgcnt;
>> >> +     int i;
>> >> +
>> >> +     for (i = 0; i < pgcnt; i++)
>> >> +             prandom_bytes_state(&rnd_state, &writebuf[i * use_len],
>> >> +                             use_len);
>> >> +     if (len % use_len)
>> >> +             prandom_bytes_state(&rnd_state, &writebuf[i * use_len],
>> >> +                             len % use_len);
>> >>
>> >> -     prandom_bytes_state(&rnd_state, writebuf, len);
>> >>       ops.mode      = MTD_OPS_AUTO_OOB;
>> >>       ops.len       = 0;
>> >>       ops.retlen    = 0;
>>
>> I would rather fix the use of prandom_bytes_state() in write_eraseblock()
>> than fix in verify_eraseblock_in_one_go().
>>
> Why and how?

I thought that it could reduce calls of prandom_bytes_state() and
it makes code simpler than increasing calls.

> write_whole_device() (which calls write_eraseblock()) is used multiple
> times with different verification methods (all blocks in one go or each
> block individually).
> If prandom_state_bytes() in write_eraseblock() would be changed, that
> function would have to know, how the block are going to be checked
> lateron to know how to set up the writebuffer.

Instead of calling prandom_bytes_state() in the for loop in
write_eraseblock(), call prandom_bytes_state() at once before going
into the loop and use correct offset in writebuf in the loop.
Although, we also need to fix verify_eraseblock() in the same way.

Doesn't that fix this problem?
--
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/