From mboxrd@z Thu Jan 1 00:00:00 1970 From: Kees Cook Subject: Re: [PATCH v1 1/5] treewide: use prandom_u32_max() when possible Date: Wed, 5 Oct 2022 21:16:50 -0700 Message-ID: <202210052035.A1020E3@keescook> References: <20221005214844.2699-1-Jason@zx2c4.com> <20221005214844.2699-2-Jason@zx2c4.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: DKIM-Filter: OpenDKIM Filter v2.11.0 smtp2.osuosl.org 8A8EE409A5 DKIM-Filter: OpenDKIM Filter v2.11.0 smtp2.osuosl.org 5E49840147 DKIM-Filter: OpenDKIM Filter v2.11.0 smtp1.osuosl.org BCACE81AA7 DKIM-Filter: OpenDKIM Filter v2.11.0 smtp1.osuosl.org 52A9881763 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org; s=google; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:from:to:cc:subject:date; bh=WbAjddv7dJAWgX74vxMdFtfHq8KnIvmUd6IHydPVZWw=; b=WgCBVOxTCY3r6hnjb6DkPXMWKhTpJd2tJ8grQVy4FqddmqNDeeFgajag3lajOwT2v4 a87IfTIAA5TCPftGBNiy4cY7xnOoxUEfMiQjvrPT/rt/xjKxauu3MOYhjJEXssFW35Ee /z+9ItQjm7QhlvSxza54MmNIb/ZuYs/f028rM= Content-Disposition: inline In-Reply-To: <20221005214844.2699-2-Jason-OnJsPKxuuEcAvxtiuMwx3w@public.gmane.org> List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: ovs-dev-bounces-yBygre7rU0TnMu66kgdUjQ@public.gmane.org Sender: "dev" To: "Jason A. Donenfeld" Cc: Andrew Lunn , "Darrick J . Wong" , Ulf Hansson , dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org, Andrii Nakryiko , Hans Verkuil , linux-sctp-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, "Md . Haris Iqbal" , Miquel Raynal , Christoph Hellwig , Andy Gospodarek , Sergey Matyukevich , Rohit Maheshwari , Michael Ellerman , ceph-devel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Christophe Leroy , Jozsef Kadlecsik , Nilesh Javali , Jean-Paul Roubelat , Dick Kennedy , Jay Vosburgh , Potnuri Bharat Teja , Vinay Kumar Yadav , linux-nfs@vg 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 -- Kees Cook