| Message ID | 1389608739-10945-1-git-send-email-LW@KARO-electronics.de |
|---|---|
| State | Not Applicable |
| Headers | show |
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@KARO-electronics.de> > --- > 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;
2014/1/22 Lothar Waßmann <LW@karo-electronics.de>: > 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@KARO-electronics.de> >> --- >> 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 <LW@karo-electronics.de>: > > 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@KARO-electronics.de> > >> --- > >> 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
2014/1/23 Lothar Waßmann <LW@karo-electronics.de>: > Hi, > > Akinobu Mita wrote: >> 2014/1/22 Lothar Waßmann <LW@karo-electronics.de>: >> > 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@KARO-electronics.de> >> >> --- >> >> 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 <LW@karo-electronics.de>: > > Hi, > > > > Akinobu Mita wrote: > >> 2014/1/22 Lothar Waßmann <LW@karo-electronics.de>: > >> > 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@KARO-electronics.de> > >> >> --- > >> >> 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
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;
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@KARO-electronics.de> --- drivers/mtd/tests/oobtest.c | 14 ++++++++++++-- 1 files changed, 12 insertions(+), 2 deletions(-)