linux-kernel - Re: [PATCH] mtd: mtd_oobtest: fix verify errors due to incorrect use of prandom_bytes

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:	Thu, 23 Jan 2014 06:51:07 +0100
From:	Lothar Waßmann <LW@...O-electronics.de>
To:	Akinobu Mita <akinobu.mita@...il.com>
Cc:	"linux-mtd@...ts.infradead.org" <linux-mtd@...ts.infradead.org>,
	David Woodhouse <dwmw2@...radead.org>,
	Brian Norris <computersforpeace@...il.com>,
	Artem Bityutskiy <artem.bityutskiy@...ux.intel.com>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Huang Shijie <b32955@...escale.com>,
	LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] mtd: mtd_oobtest: fix verify errors due to incorrect
 use of prandom_bytes_state()

Hi,

Akinobu Mita wrote:
> 2014/1/23 Lothar Waßmann <LW@...o-electronics.de>:
> > Hi,
> >
> > Akinobu Mita wrote:
> >> 2014/1/22 Lothar Waßmann <LW@...o-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@...O-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
-- 
___________________________________________________________

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

www.karo-electronics.de | info@...o-electronics.de
___________________________________________________________
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/