Re: [PATCH v3 3/3] treewide: use get_random_u32_inclusive() when possible
From: Kees Cook <hidden>
Date: 2022-11-17 21:58:25
Also in:
linux-arm-kernel, linux-block, linux-crypto, linux-fsdevel, linux-media, linux-mips, linux-mmc, linux-patches, linuxppc-dev, lkml, loongarch
On Thu, Nov 17, 2022 at 09:29:06PM +0100, Jason A. Donenfeld wrote:
These cases were done with this Coccinelle: @@ expression H; expression L; @@ - (get_random_u32_below(H) + L) + get_random_u32_inclusive(L, H + L - 1) @@ expression H; expression L; expression E; @@ get_random_u32_inclusive(L, H - + E - - E ) @@ expression H; expression L; expression E; @@ get_random_u32_inclusive(L, H - - E - + E ) @@ expression H; expression L; expression E; expression F; @@ get_random_u32_inclusive(L, H - - E + F - + E ) @@ expression H; expression L; expression E; expression F; @@ get_random_u32_inclusive(L, H - + E + F - - E ) And then subsequently cleaned up by hand, with several automatic cases rejected if it didn't make sense contextually. Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> Reviewed-by: Jason Gunthorpe <jgg@nvidia.com> # for infiniband Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com> --- arch/x86/kernel/module.c | 2 +- crypto/rsa-pkcs1pad.c | 2 +- crypto/testmgr.c | 10 ++++---- drivers/bus/mhi/host/internal.h | 2 +- drivers/dma-buf/st-dma-fence-chain.c | 2 +- drivers/infiniband/core/cma.c | 2 +- drivers/infiniband/hw/hns/hns_roce_ah.c | 5 ++-- drivers/mtd/nand/raw/nandsim.c | 2 +- drivers/net/wireguard/selftest/allowedips.c | 8 +++--- .../broadcom/brcm80211/brcmfmac/p2p.c | 2 +- .../net/wireless/intel/iwlwifi/mvm/mac-ctxt.c | 2 +- fs/f2fs/segment.c | 6 ++--- kernel/kcsan/selftest.c | 2 +- lib/test_hexdump.c | 10 ++++---- lib/test_printf.c | 2 +- lib/test_vmalloc.c | 6 ++--- mm/kasan/kasan_test.c | 6 ++--- mm/kfence/kfence_test.c | 2 +- mm/swapfile.c | 5 ++-- net/bluetooth/mgmt.c | 5 ++-- net/core/pktgen.c | 25 ++++++++----------- net/ipv4/tcp_input.c | 2 +- net/ipv6/addrconf.c | 6 ++--- net/netfilter/nf_nat_helper.c | 2 +- net/xfrm/xfrm_state.c | 2 +- 25 files changed, 56 insertions(+), 64 deletions(-)
Even the diffstat agrees this is a nice clean-up. :) Reviewed-by: Kees Cook <redacted> The only comment I have is that maybe these cases can just be left as-is with _below()?
- size_t len = get_random_u32_below(rs) + gs; + size_t len = get_random_u32_inclusive(gs, rs + gs - 1);
It seems like writing it in the form of base plus [0, limit) is clearer? size_t len = gs + get_random_u32_below(rs); But there is only a handful, so *shrug* All the others are much cleaner rewritten as _inclusive(). -- Kees Cook