On Wed, Oct 5, 2022 at 9:16 PM Kees Cook <keesc...@chromium.org> wrote:
>
> On Wed, Oct 05, 2022 at 11:48:40PM +0200, Jason A. Donenfeld wrote:
> > Rather than incurring a division or requesting too many random bytes for
> > the given range, use the prandom_u32_max() function, which only takes
> > the minimum required bytes from the RNG and avoids divisions.
>
> Yes please!
>
> Since this is a treewide patch, it's helpful for (me at least) doing
> reviews to detail the mechanism of the transformation.
>
> e.g. I imagine this could be done with something like Coccinelle and
>
> @no_modulo@
> expression E;
> @@
>
> - (prandom_u32() % (E))
> + prandom_u32_max(E)
>
> > diff --git a/drivers/mtd/ubi/debug.h b/drivers/mtd/ubi/debug.h
> > index 118248a5d7d4..4236c799a47c 100644
> > --- a/drivers/mtd/ubi/debug.h
> > +++ b/drivers/mtd/ubi/debug.h
> > @@ -73,7 +73,7 @@ static inline int ubi_dbg_is_bgt_disabled(const struct
> > ubi_device *ubi)
> > static inline int ubi_dbg_is_bitflip(const struct ubi_device *ubi)
> > {
> > if (ubi->dbg.emulate_bitflips)
> > - return !(prandom_u32() % 200);
> > + return !(prandom_u32_max(200));
> > return 0;
> > }
> >
>
> Because some looks automated (why the parens?)
>
> > @@ -393,14 +387,11 @@ static struct test_driver {
> >
> > static void shuffle_array(int *arr, int n)
> > {
> > - unsigned int rnd;
> > int i, j;
> >
> > for (i = n - 1; i > 0; i--) {
> > - rnd = prandom_u32();
> > -
> > /* Cut the range. */
> > - j = rnd % i;
> > + j = prandom_u32_max(i);
> >
> > /* Swap indexes. */
> > swap(arr[i], arr[j]);
>
> And some by hand. :)
>
> Reviewed-by: Kees Cook <keesc...@chromium.org>
Thanks!
Reviewed-by: KP Singh <kpsi...@kernel.org>
>
> --
> Kees Cook
