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 <keescook@xxxxxxxxxxxx>
--
Kees Cook
