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 <[email protected]>
---
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;
@@ -594,7 +601,10 @@ static int __init mtd_oobtest_init(void)
if (bbt[i] || bbt[i + 1])
continue;
prandom_bytes_state(&rnd_state, writebuf,
- mtd->ecclayout->oobavail * 2);
+ mtd->ecclayout->oobavail);
+ prandom_bytes_state(&rnd_state,
+ writebuf + mtd->ecclayout->oobavail,
+ mtd->ecclayout->oobavail);
addr = (i + 1) * mtd->erasesize - mtd->writesize;
ops.mode = MTD_OPS_AUTO_OOB;
ops.len = 0;
--
1.7.2.5
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 <[email protected]>
> ---
> 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;
> @@ -594,7 +601,10 @@ static int __init mtd_oobtest_init(void)
> if (bbt[i] || bbt[i + 1])
> continue;
> prandom_bytes_state(&rnd_state, writebuf,
> - mtd->ecclayout->oobavail * 2);
> + mtd->ecclayout->oobavail);
> + prandom_bytes_state(&rnd_state,
> + writebuf + mtd->ecclayout->oobavail,
> + mtd->ecclayout->oobavail);
> addr = (i + 1) * mtd->erasesize - mtd->writesize;
> ops.mode = MTD_OPS_AUTO_OOB;
> ops.len = 0;
--
___________________________________________________________
Ka-Ro electronics GmbH | Pascalstraße 22 | D - 52076 Aachen
Phone: +49 2408 1402-0 | Fax: +49 2408 1402-10
Geschäftsführer: Matthias Kaussen
Handelsregistereintrag: Amtsgericht Aachen, HRB 4996
http://www.karo-electronics.de | [email protected]
___________________________________________________________
2014/1/22 Lothar Wa?mann <[email protected]>:
> 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 <[email protected]>
>> ---
>> 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().
>> @@ -594,7 +601,10 @@ static int __init mtd_oobtest_init(void)
>> if (bbt[i] || bbt[i + 1])
>> continue;
>> prandom_bytes_state(&rnd_state, writebuf,
>> - mtd->ecclayout->oobavail * 2);
>> + mtd->ecclayout->oobavail);
>> + prandom_bytes_state(&rnd_state,
>> + writebuf + mtd->ecclayout->oobavail,
>> + mtd->ecclayout->oobavail);
>> addr = (i + 1) * mtd->erasesize - mtd->writesize;
>> ops.mode = MTD_OPS_AUTO_OOB;
>> ops.len = 0;
The same goes for this. I would rather fix prandom_bytes_state() when
writing OOBs.
Hi,
Akinobu Mita wrote:
> 2014/1/22 Lothar Waßmann <[email protected]>:
> > 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 <[email protected]>
> >> ---
> >> 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?
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.
Dto. for mtd_write_oob()
Lothar Waßmann
--
___________________________________________________________
Ka-Ro electronics GmbH | Pascalstraße 22 | D - 52076 Aachen
Phone: +49 2408 1402-0 | Fax: +49 2408 1402-10
Geschäftsführer: Matthias Kaussen
Handelsregistereintrag: Amtsgericht Aachen, HRB 4996
http://www.karo-electronics.de | [email protected]
___________________________________________________________
2014/1/23 Lothar Wa?mann <[email protected]>:
> Hi,
>
> Akinobu Mita wrote:
>> 2014/1/22 Lothar Wa?mann <[email protected]>:
>> > 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 <[email protected]>
>> >> ---
>> >> 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?
Hi,
Akinobu Mita wrote:
> 2014/1/23 Lothar Waßmann <[email protected]>:
> > Hi,
> >
> > Akinobu Mita wrote:
> >> 2014/1/22 Lothar Waßmann <[email protected]>:
> >> > 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 <[email protected]>
> >> >> ---
> >> >> 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?
>
Of course one could fix it that way, but that would be a much more
invasive change that also needs more testing.
Lothar Waßmann
--
___________________________________________________________
Ka-Ro electronics GmbH | Pascalstraße 22 | D - 52076 Aachen
Phone: +49 2408 1402-0 | Fax: +49 2408 1402-10
Geschäftsführer: Matthias Kaussen
Handelsregistereintrag: Amtsgericht Aachen, HRB 4996
http://www.karo-electronics.de | [email protected]
___________________________________________________________