| Message ID | 20221007180107.216067-3-Jason@zx2c4.com |
|---|---|
| State | Not Applicable |
| Headers | show |
| Series | treewide cleanup of random integer usage | expand |
On Fri, Oct 07, 2022 at 12:01:03PM -0600, 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. > > Reviewed-by: Kees Cook <keescook@chromium.org> > Reviewed-by: KP Singh <kpsingh@kernel.org> > Reviewed-by: Christoph Böhmwalder <christoph.boehmwalder@linbit.com> # for drbd > Reviewed-by: Jan Kara <jack@suse.cz> # for ext2, ext4, and sbitmap > Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com> > --- <snip, skip to the xfs part> > diff --git a/fs/xfs/libxfs/xfs_alloc.c b/fs/xfs/libxfs/xfs_alloc.c > index e2bdf089c0a3..6261599bb389 100644 > --- a/fs/xfs/libxfs/xfs_alloc.c > +++ b/fs/xfs/libxfs/xfs_alloc.c > @@ -1520,7 +1520,7 @@ xfs_alloc_ag_vextent_lastblock( > > #ifdef DEBUG > /* Randomly don't execute the first algorithm. */ > - if (prandom_u32() & 1) > + if (prandom_u32_max(2)) I wonder if these usecases (picking 0 or 1 randomly) ought to have a trivial wrapper to make it more obvious that we want boolean semantics: static inline bool prandom_bool(void) { return prandom_u32_max(2); } if (prandom_bool()) use_crazy_algorithm(...); But this translation change looks correct to me, so for the XFS parts: Acked-by: Darrick J. Wong <djwong@kernel.org> --D > return 0; > #endif > > diff --git a/fs/xfs/libxfs/xfs_ialloc.c b/fs/xfs/libxfs/xfs_ialloc.c > index 6cdfd64bc56b..7838b31126e2 100644 > --- a/fs/xfs/libxfs/xfs_ialloc.c > +++ b/fs/xfs/libxfs/xfs_ialloc.c > @@ -636,7 +636,7 @@ xfs_ialloc_ag_alloc( > /* randomly do sparse inode allocations */ > if (xfs_has_sparseinodes(tp->t_mountp) && > igeo->ialloc_min_blks < igeo->ialloc_blks) > - do_sparse = prandom_u32() & 1; > + do_sparse = prandom_u32_max(2); > #endif > > /* > diff --git a/include/linux/nodemask.h b/include/linux/nodemask.h > index 4b71a96190a8..66ee9b4b7925 100644 > --- a/include/linux/nodemask.h > +++ b/include/linux/nodemask.h > @@ -509,7 +509,7 @@ static inline int node_random(const nodemask_t *maskp) > w = nodes_weight(*maskp); > if (w) > bit = bitmap_ord_to_pos(maskp->bits, > - get_random_int() % w, MAX_NUMNODES); > + prandom_u32_max(w), MAX_NUMNODES); > return bit; > #else > return 0; > diff --git a/lib/cmdline_kunit.c b/lib/cmdline_kunit.c > index e6a31c927b06..a72a2c16066e 100644 > --- a/lib/cmdline_kunit.c > +++ b/lib/cmdline_kunit.c > @@ -76,7 +76,7 @@ static void cmdline_test_lead_int(struct kunit *test) > int rc = cmdline_test_values[i]; > int offset; > > - sprintf(in, "%u%s", prandom_u32_max(256), str); > + sprintf(in, "%u%s", get_random_int() % 256, str); > /* Only first '-' after the number will advance the pointer */ > offset = strlen(in) - strlen(str) + !!(rc == 2); > cmdline_do_one_test(test, in, rc, offset); > @@ -94,7 +94,7 @@ static void cmdline_test_tail_int(struct kunit *test) > int rc = strcmp(str, "") ? (strcmp(str, "-") ? 0 : 1) : 1; > int offset; > > - sprintf(in, "%s%u", str, prandom_u32_max(256)); > + sprintf(in, "%s%u", str, get_random_int() % 256); > /* > * Only first and leading '-' not followed by integer > * will advance the pointer. > diff --git a/lib/kobject.c b/lib/kobject.c > index 5f0e71ab292c..a0b2dbfcfa23 100644 > --- a/lib/kobject.c > +++ b/lib/kobject.c > @@ -694,7 +694,7 @@ static void kobject_release(struct kref *kref) > { > struct kobject *kobj = container_of(kref, struct kobject, kref); > #ifdef CONFIG_DEBUG_KOBJECT_RELEASE > - unsigned long delay = HZ + HZ * (get_random_int() & 0x3); > + unsigned long delay = HZ + HZ * prandom_u32_max(4); > pr_info("kobject: '%s' (%p): %s, parent %p (delayed %ld)\n", > kobject_name(kobj), kobj, __func__, kobj->parent, delay); > INIT_DELAYED_WORK(&kobj->release, kobject_delayed_cleanup); > diff --git a/lib/reed_solomon/test_rslib.c b/lib/reed_solomon/test_rslib.c > index 6faf9c9a6215..4d241bdc88aa 100644 > --- a/lib/reed_solomon/test_rslib.c > +++ b/lib/reed_solomon/test_rslib.c > @@ -199,7 +199,7 @@ static int get_rcw_we(struct rs_control *rs, struct wspace *ws, > > derrlocs[i] = errloc; > > - if (ewsc && (prandom_u32() & 1)) { > + if (ewsc && prandom_u32_max(2)) { > /* Erasure with the symbol intact */ > errlocs[errloc] = 2; > } else { > diff --git a/lib/sbitmap.c b/lib/sbitmap.c > index c4f04edf3ee9..ef0661504561 100644 > --- a/lib/sbitmap.c > +++ b/lib/sbitmap.c > @@ -21,7 +21,7 @@ static int init_alloc_hint(struct sbitmap *sb, gfp_t flags) > int i; > > for_each_possible_cpu(i) > - *per_cpu_ptr(sb->alloc_hint, i) = prandom_u32() % depth; > + *per_cpu_ptr(sb->alloc_hint, i) = prandom_u32_max(depth); > } > return 0; > } > diff --git a/lib/test_hexdump.c b/lib/test_hexdump.c > index 0927f44cd478..41a0321f641a 100644 > --- a/lib/test_hexdump.c > +++ b/lib/test_hexdump.c > @@ -208,7 +208,7 @@ static void __init test_hexdump_overflow(size_t buflen, size_t len, > static void __init test_hexdump_overflow_set(size_t buflen, bool ascii) > { > unsigned int i = 0; > - int rs = (prandom_u32_max(2) + 1) * 16; > + int rs = prandom_u32_max(2) + 1 * 16; > > do { > int gs = 1 << i; > diff --git a/lib/test_vmalloc.c b/lib/test_vmalloc.c > index 4f2f2d1bac56..56ffaa8dd3f6 100644 > --- a/lib/test_vmalloc.c > +++ b/lib/test_vmalloc.c > @@ -151,9 +151,7 @@ static int random_size_alloc_test(void) > int i; > > for (i = 0; i < test_loop_count; i++) { > - n = prandom_u32(); > - n = (n % 100) + 1; > - > + n = prandom_u32_max(n % 100) + 1; > p = vmalloc(n * PAGE_SIZE); > > if (!p) > @@ -293,16 +291,12 @@ pcpu_alloc_test(void) > return -1; > > for (i = 0; i < 35000; i++) { > - unsigned int r; > - > - r = prandom_u32(); > - size = (r % (PAGE_SIZE / 4)) + 1; > + size = prandom_u32_max(PAGE_SIZE / 4) + 1; > > /* > * Maximum PAGE_SIZE > */ > - r = prandom_u32(); > - align = 1 << ((r % 11) + 1); > + align = 1 << (prandom_u32_max(11) + 1); > > pcpu[i] = __alloc_percpu(size, align); > if (!pcpu[i]) > @@ -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]); > diff --git a/net/core/pktgen.c b/net/core/pktgen.c > index a13ee452429e..5ca4f953034c 100644 > --- a/net/core/pktgen.c > +++ b/net/core/pktgen.c > @@ -2469,11 +2469,11 @@ static void mod_cur_headers(struct pktgen_dev *pkt_dev) > } > > if ((pkt_dev->flags & F_VID_RND) && (pkt_dev->vlan_id != 0xffff)) { > - pkt_dev->vlan_id = prandom_u32() & (4096 - 1); > + pkt_dev->vlan_id = prandom_u32_max(4096); > } > > if ((pkt_dev->flags & F_SVID_RND) && (pkt_dev->svlan_id != 0xffff)) { > - pkt_dev->svlan_id = prandom_u32() & (4096 - 1); > + pkt_dev->svlan_id = prandom_u32_max(4096); > } > > if (pkt_dev->udp_src_min < pkt_dev->udp_src_max) { > diff --git a/net/ipv4/inet_hashtables.c b/net/ipv4/inet_hashtables.c > index b9d995b5ce24..9dc070f2018e 100644 > --- a/net/ipv4/inet_hashtables.c > +++ b/net/ipv4/inet_hashtables.c > @@ -794,7 +794,7 @@ int __inet_hash_connect(struct inet_timewait_death_row *death_row, > * on low contention the randomness is maximal and on high contention > * it may be inexistent. > */ > - i = max_t(int, i, (prandom_u32() & 7) * 2); > + i = max_t(int, i, prandom_u32_max(8) * 2); > WRITE_ONCE(table_perturb[index], READ_ONCE(table_perturb[index]) + i + 2); > > /* Head lock still held and bh's disabled */ > diff --git a/net/sunrpc/cache.c b/net/sunrpc/cache.c > index c3c693b51c94..f075a9fb5ccc 100644 > --- a/net/sunrpc/cache.c > +++ b/net/sunrpc/cache.c > @@ -677,7 +677,7 @@ static void cache_limit_defers(void) > > /* Consider removing either the first or the last */ > if (cache_defer_cnt > DFR_MAX) { > - if (prandom_u32() & 1) > + if (prandom_u32_max(2)) > discard = list_entry(cache_defer_list.next, > struct cache_deferred_req, recent); > else > diff --git a/net/sunrpc/xprtsock.c b/net/sunrpc/xprtsock.c > index e976007f4fd0..c2caee703d2c 100644 > --- a/net/sunrpc/xprtsock.c > +++ b/net/sunrpc/xprtsock.c > @@ -1619,7 +1619,7 @@ static int xs_get_random_port(void) > if (max < min) > return -EADDRINUSE; > range = max - min + 1; > - rand = (unsigned short) prandom_u32() % range; > + rand = (unsigned short) prandom_u32_max(range); > return rand + min; > } > > -- > 2.37.3 >
On Fri, Oct 07, 2022 at 12:01:03PM -0600, 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. I actually meant splitting the by-hand stuff by subsystem, but nearly all of these can be done mechanically too, so it shouldn't be bad. Notes below... > > [...] > diff --git a/arch/arm64/kernel/process.c b/arch/arm64/kernel/process.c > index 92bcc1768f0b..87203429f802 100644 > --- a/arch/arm64/kernel/process.c > +++ b/arch/arm64/kernel/process.c > @@ -595,7 +595,7 @@ unsigned long __get_wchan(struct task_struct *p) > unsigned long arch_align_stack(unsigned long sp) > { > if (!(current->personality & ADDR_NO_RANDOMIZE) && randomize_va_space) > - sp -= get_random_int() & ~PAGE_MASK; > + sp -= prandom_u32_max(PAGE_SIZE); > return sp & ~0xf; > } > @mask@ expression MASK; @@ - (get_random_int() & ~(MASK)) + prandom_u32_max(MASK) > diff --git a/arch/loongarch/kernel/vdso.c b/arch/loongarch/kernel/vdso.c > index f32c38abd791..8c9826062652 100644 > --- a/arch/loongarch/kernel/vdso.c > +++ b/arch/loongarch/kernel/vdso.c > @@ -78,7 +78,7 @@ static unsigned long vdso_base(void) > unsigned long base = STACK_TOP; > > if (current->flags & PF_RANDOMIZE) { > - base += get_random_int() & (VDSO_RANDOMIZE_SIZE - 1); > + base += prandom_u32_max(VDSO_RANDOMIZE_SIZE); > base = PAGE_ALIGN(base); > } > @minus_one@ expression FULL; @@ - (get_random_int() & ((FULL) - 1) + prandom_u32_max(FULL) > diff --git a/arch/parisc/kernel/vdso.c b/arch/parisc/kernel/vdso.c > index 63dc44c4c246..47e5960a2f96 100644 > --- a/arch/parisc/kernel/vdso.c > +++ b/arch/parisc/kernel/vdso.c > @@ -75,7 +75,7 @@ int arch_setup_additional_pages(struct linux_binprm *bprm, > > map_base = mm->mmap_base; > if (current->flags & PF_RANDOMIZE) > - map_base -= (get_random_int() & 0x1f) * PAGE_SIZE; > + map_base -= prandom_u32_max(0x20) * PAGE_SIZE; > > vdso_text_start = get_unmapped_area(NULL, map_base, vdso_text_len, 0, 0); > These are more fun, but Coccinelle can still do them with a little Pythonic help: // Find a potential literal @literal_mask@ expression LITERAL; identifier randfunc =~ "get_random_int|prandom_u32|get_random_u32"; position p; @@ (randfunc()@p & (LITERAL)) // Add one to the literal. @script:python add_one@ literal << literal_mask.LITERAL; RESULT; @@ if literal.startswith('0x'): value = int(literal, 16) + 1 coccinelle.RESULT = cocci.make_expr("0x%x" % (value)) elif literal[0] in '123456789': value = int(literal, 10) + 1 coccinelle.RESULT = cocci.make_expr("%d" % (value)) else: print("I don't know how to handle: %s" % (literal)) // Replace the literal mask with the calculated result. @plus_one@ expression literal_mask.LITERAL; position literal_mask.p; expression add_one.RESULT; identifier FUNC; @@ - (FUNC()@p & (LITERAL)) + prandom_u32_max(RESULT) > diff --git a/drivers/mtd/tests/stresstest.c b/drivers/mtd/tests/stresstest.c > index cb29c8c1b370..d2faaca7f19d 100644 > --- a/drivers/mtd/tests/stresstest.c > +++ b/drivers/mtd/tests/stresstest.c > @@ -45,9 +45,8 @@ static int rand_eb(void) > unsigned int eb; > > again: > - eb = prandom_u32(); > /* Read or write up 2 eraseblocks at a time - hence 'ebcnt - 1' */ > - eb %= (ebcnt - 1); > + eb = prandom_u32_max(ebcnt - 1); > if (bbt[eb]) > goto again; > return eb; This can also be done mechanically: @multi_line@ identifier randfunc =~ "get_random_int|prandom_u32|get_random_u32"; identifier RAND; expression E; @@ - RAND = randfunc(); ... when != RAND - RAND %= (E); + RAND = prandom_u32_max(E); @collapse_ret@ type TYPE; identifier VAR; expression E; @@ { - TYPE VAR; - VAR = (E); - return VAR; + return E; } @drop_var@ type TYPE; identifier VAR; @@ { - TYPE VAR; ... when != VAR } > diff --git a/fs/ext2/ialloc.c b/fs/ext2/ialloc.c > index 998dd2ac8008..f4944c4dee60 100644 > --- a/fs/ext2/ialloc.c > +++ b/fs/ext2/ialloc.c > @@ -277,8 +277,7 @@ static int find_group_orlov(struct super_block *sb, struct inode *parent) > int best_ndir = inodes_per_group; > int best_group = -1; > > - group = prandom_u32(); > - parent_group = (unsigned)group % ngroups; > + parent_group = prandom_u32_max(ngroups); > for (i = 0; i < ngroups; i++) { > group = (parent_group + i) % ngroups; > desc = ext2_get_group_desc (sb, group, NULL); Okay, that one is too much for me -- checking that group is never used after the assignment removal is likely possible, but beyond my cocci know-how. :) > diff --git a/fs/ext4/ialloc.c b/fs/ext4/ialloc.c > index f73e5eb43eae..36d5bc595cc2 100644 > --- a/fs/ext4/ialloc.c > +++ b/fs/ext4/ialloc.c > @@ -463,10 +463,9 @@ static int find_group_orlov(struct super_block *sb, struct inode *parent, > hinfo.hash_version = DX_HASH_HALF_MD4; > hinfo.seed = sbi->s_hash_seed; > ext4fs_dirhash(parent, qstr->name, qstr->len, &hinfo); > - grp = hinfo.hash; > + parent_group = hinfo.hash % ngroups; > } else > - grp = prandom_u32(); > - parent_group = (unsigned)grp % ngroups; > + parent_group = prandom_u32_max(ngroups); > for (i = 0; i < ngroups; i++) { > g = (parent_group + i) % ngroups; > get_orlov_stats(sb, g, flex_size, &stats); Much less easy mechanically. :) > diff --git a/lib/test_hexdump.c b/lib/test_hexdump.c > index 0927f44cd478..41a0321f641a 100644 > --- a/lib/test_hexdump.c > +++ b/lib/test_hexdump.c > @@ -208,7 +208,7 @@ static void __init test_hexdump_overflow(size_t buflen, size_t len, > static void __init test_hexdump_overflow_set(size_t buflen, bool ascii) > { > unsigned int i = 0; > - int rs = (prandom_u32_max(2) + 1) * 16; > + int rs = prandom_u32_max(2) + 1 * 16; > > do { > int gs = 1 << i; This looks wrong. Cocci says: - int rs = (get_random_int() % 2 + 1) * 16; + int rs = (prandom_u32_max(2) + 1) * 16; > diff --git a/lib/test_vmalloc.c b/lib/test_vmalloc.c > index 4f2f2d1bac56..56ffaa8dd3f6 100644 > --- a/lib/test_vmalloc.c > +++ b/lib/test_vmalloc.c > @@ -151,9 +151,7 @@ static int random_size_alloc_test(void) > int i; > > for (i = 0; i < test_loop_count; i++) { > - n = prandom_u32(); > - n = (n % 100) + 1; > - > + n = prandom_u32_max(n % 100) + 1; > p = vmalloc(n * PAGE_SIZE); > > if (!p) This looks wrong. Cocci says: - n = prandom_u32(); - n = (n % 100) + 1; + n = prandom_u32_max(100) + 1; > @@ -293,16 +291,12 @@ pcpu_alloc_test(void) > return -1; > > for (i = 0; i < 35000; i++) { > - unsigned int r; > - > - r = prandom_u32(); > - size = (r % (PAGE_SIZE / 4)) + 1; > + size = prandom_u32_max(PAGE_SIZE / 4) + 1; > > /* > * Maximum PAGE_SIZE > */ > - r = prandom_u32(); > - align = 1 << ((r % 11) + 1); > + align = 1 << (prandom_u32_max(11) + 1); > > pcpu[i] = __alloc_percpu(size, align); > if (!pcpu[i]) > @@ -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]); Yup, agrees with Cocci on these.
On Fri, Oct 07, 2022 at 02:17:22PM -0700, Darrick J. Wong wrote: > On Fri, Oct 07, 2022 at 12:01:03PM -0600, 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. > > > > Reviewed-by: Kees Cook <keescook@chromium.org> > > Reviewed-by: KP Singh <kpsingh@kernel.org> > > Reviewed-by: Christoph Böhmwalder <christoph.boehmwalder@linbit.com> # for drbd > > Reviewed-by: Jan Kara <jack@suse.cz> # for ext2, ext4, and sbitmap > > Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com> > > --- > > <snip, skip to the xfs part> > > > diff --git a/fs/xfs/libxfs/xfs_alloc.c b/fs/xfs/libxfs/xfs_alloc.c > > index e2bdf089c0a3..6261599bb389 100644 > > --- a/fs/xfs/libxfs/xfs_alloc.c > > +++ b/fs/xfs/libxfs/xfs_alloc.c > > @@ -1520,7 +1520,7 @@ xfs_alloc_ag_vextent_lastblock( > > > > #ifdef DEBUG > > /* Randomly don't execute the first algorithm. */ > > - if (prandom_u32() & 1) > > + if (prandom_u32_max(2)) > > I wonder if these usecases (picking 0 or 1 randomly) ought to have a > trivial wrapper to make it more obvious that we want boolean semantics: > > static inline bool prandom_bool(void) > { > return prandom_u32_max(2); > } > > if (prandom_bool()) > use_crazy_algorithm(...); > Yea, I've had a lot of similar ideas there. Part of doing this (initial) patchset is to get an intuitive sense of what's actually used and how often. On my list for investigation are a get_random_u32_max() to return uniform numbers by rejection sampling (prandom_u32_max() doesn't do that uniformly) and adding a function for booleans or bits < 8. Possible ideas for the latter include: bool get_random_bool(void); bool get_random_bool(unsigned int probability); bool get_random_bits(u8 bits_less_than_eight); With the core of all of those involving the same batching as the current get_random_u{8,16,32,64}() functions, but also buffering the latest byte and managing how many bits are left in it that haven't been shifted out yet. So API-wise, there are a few ways to go, so hopefully this series will start to give a good picture of what's needed. One thing I've noticed is that most of the prandom_u32_max(2) invocations are in debug or test code, so that doesn't need to be optimized. But kfence does that too in its hot path, so a get_random_bool() function there would in theory lead to an 8x speed-up. But I guess I just have to try some things and see. Anyway, that is a long way to say, I share you curiosity on the matter and I'm looking into it. Jason
On Fri, Oct 07, 2022 at 03:47:44PM -0700, Kees Cook wrote: > On Fri, Oct 07, 2022 at 12:01:03PM -0600, 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. > > I actually meant splitting the by-hand stuff by subsystem, but nearly > all of these can be done mechanically too, so it shouldn't be bad. Notes > below... Oh, cool, more coccinelle. You're basically giving me a class on these recipes. Much appreciated. > > [...] > > diff --git a/arch/arm64/kernel/process.c b/arch/arm64/kernel/process.c > > index 92bcc1768f0b..87203429f802 100644 > > --- a/arch/arm64/kernel/process.c > > +++ b/arch/arm64/kernel/process.c > > @@ -595,7 +595,7 @@ unsigned long __get_wchan(struct task_struct *p) > > unsigned long arch_align_stack(unsigned long sp) > > { > > if (!(current->personality & ADDR_NO_RANDOMIZE) && randomize_va_space) > > - sp -= get_random_int() & ~PAGE_MASK; > > + sp -= prandom_u32_max(PAGE_SIZE); > > return sp & ~0xf; > > } > > > > @mask@ > expression MASK; > @@ > > - (get_random_int() & ~(MASK)) > + prandom_u32_max(MASK) Not quite! PAGE_MASK != PAGE_SIZE. In this case, things get a litttttle more complicated where you can do: get_random_int() & MASK == prandom_u32_max(MASK + 1) *only if all the top bits of MASK are set* That is, if MASK one less than a power of two. Or if MASK & (MASK + 1) == 0. (If those top bits aren't set, you can technically do prandom_u32_max(MASK >> n + 1) << n. That'd be a nice thing to work out. But yeesh, maybe a bit much for the time being and probably a bit beyond coccinelle.) This case here, though, is a bit more special, where we can just rely on an obvious given kernel identity. Namely, PAGE_MASK == ~(PAGE_SIZE - 1). So ~PAGE_MASK == PAGE_SIZE - 1. So get_random_int() & ~PAGE_MASK == prandom_u32_max(PAGE_SIZE - 1 + 1). So get_random_int() & ~PAGE_MASK == prandom_u32_max(PAGE_SIZE). And most importantly, this makes the code more readable, since everybody knows what bounding by PAGE_SIZE means, where as what on earth is happening with the &~PAGE_MASK thing. So it's a good change. I'll try to teach coccinelle about that special case. > > diff --git a/arch/loongarch/kernel/vdso.c b/arch/loongarch/kernel/vdso.c > > index f32c38abd791..8c9826062652 100644 > > --- a/arch/loongarch/kernel/vdso.c > > +++ b/arch/loongarch/kernel/vdso.c > > @@ -78,7 +78,7 @@ static unsigned long vdso_base(void) > > unsigned long base = STACK_TOP; > > > > if (current->flags & PF_RANDOMIZE) { > > - base += get_random_int() & (VDSO_RANDOMIZE_SIZE - 1); > > + base += prandom_u32_max(VDSO_RANDOMIZE_SIZE); > > base = PAGE_ALIGN(base); > > } > > > > @minus_one@ > expression FULL; > @@ > > - (get_random_int() & ((FULL) - 1) > + prandom_u32_max(FULL) Ahh, well, okay, this is the example I mentioned above. Only works if FULL is saturated. Any clever way to get coccinelle to prove that? Can it look at the value of constants? > > > diff --git a/arch/parisc/kernel/vdso.c b/arch/parisc/kernel/vdso.c > > index 63dc44c4c246..47e5960a2f96 100644 > > --- a/arch/parisc/kernel/vdso.c > > +++ b/arch/parisc/kernel/vdso.c > > @@ -75,7 +75,7 @@ int arch_setup_additional_pages(struct linux_binprm *bprm, > > > > map_base = mm->mmap_base; > > if (current->flags & PF_RANDOMIZE) > > - map_base -= (get_random_int() & 0x1f) * PAGE_SIZE; > > + map_base -= prandom_u32_max(0x20) * PAGE_SIZE; > > > > vdso_text_start = get_unmapped_area(NULL, map_base, vdso_text_len, 0, 0); > > > > These are more fun, but Coccinelle can still do them with a little > Pythonic help: > > // Find a potential literal > @literal_mask@ > expression LITERAL; > identifier randfunc =~ "get_random_int|prandom_u32|get_random_u32"; > position p; > @@ > > (randfunc()@p & (LITERAL)) > > // Add one to the literal. > @script:python add_one@ > literal << literal_mask.LITERAL; > RESULT; > @@ > > if literal.startswith('0x'): > value = int(literal, 16) + 1 > coccinelle.RESULT = cocci.make_expr("0x%x" % (value)) > elif literal[0] in '123456789': > value = int(literal, 10) + 1 > coccinelle.RESULT = cocci.make_expr("%d" % (value)) > else: > print("I don't know how to handle: %s" % (literal)) > > // Replace the literal mask with the calculated result. > @plus_one@ > expression literal_mask.LITERAL; > position literal_mask.p; > expression add_one.RESULT; > identifier FUNC; > @@ > > - (FUNC()@p & (LITERAL)) > + prandom_u32_max(RESULT) Oh that's pretty cool. I can do the saturation check in python, since `value` holds the parsed result. Neat. > > diff --git a/fs/ext2/ialloc.c b/fs/ext2/ialloc.c > > index 998dd2ac8008..f4944c4dee60 100644 > > --- a/fs/ext2/ialloc.c > > +++ b/fs/ext2/ialloc.c > > @@ -277,8 +277,7 @@ static int find_group_orlov(struct super_block *sb, struct inode *parent) > > int best_ndir = inodes_per_group; > > int best_group = -1; > > > > - group = prandom_u32(); > > - parent_group = (unsigned)group % ngroups; > > + parent_group = prandom_u32_max(ngroups); > > for (i = 0; i < ngroups; i++) { > > group = (parent_group + i) % ngroups; > > desc = ext2_get_group_desc (sb, group, NULL); > > Okay, that one is too much for me -- checking that group is never used > after the assignment removal is likely possible, but beyond my cocci > know-how. :) Yea this is a tricky one, which I initially didn't do by hand, but Jan seemed fine with it, and it's clear if you look at it. Trixy cocci indeed. > > diff --git a/lib/test_hexdump.c b/lib/test_hexdump.c > > index 0927f44cd478..41a0321f641a 100644 > > --- a/lib/test_hexdump.c > > +++ b/lib/test_hexdump.c > > @@ -208,7 +208,7 @@ static void __init test_hexdump_overflow(size_t buflen, size_t len, > > static void __init test_hexdump_overflow_set(size_t buflen, bool ascii) > > { > > unsigned int i = 0; > > - int rs = (prandom_u32_max(2) + 1) * 16; > > + int rs = prandom_u32_max(2) + 1 * 16; > > > > do { > > int gs = 1 << i; > > This looks wrong. Cocci says: > > - int rs = (get_random_int() % 2 + 1) * 16; > + int rs = (prandom_u32_max(2) + 1) * 16; !! Nice catch. Alright, I'll give this a try with more cocci. The big difficulty at the moment is the power of 2 constant checking thing. If you have any pointers on that, would be nice. Thanks a bunch for the guidance. Jason
On Fri, Oct 07, 2022 at 03:47:44PM -0700, Kees Cook wrote: > > diff --git a/lib/test_vmalloc.c b/lib/test_vmalloc.c > > index 4f2f2d1bac56..56ffaa8dd3f6 100644 > > --- a/lib/test_vmalloc.c > > +++ b/lib/test_vmalloc.c > > @@ -151,9 +151,7 @@ static int random_size_alloc_test(void) > > int i; > > > > for (i = 0; i < test_loop_count; i++) { > > - n = prandom_u32(); > > - n = (n % 100) + 1; > > - > > + n = prandom_u32_max(n % 100) + 1; > > p = vmalloc(n * PAGE_SIZE); > > > > if (!p) > > This looks wrong. Cocci says: > > - n = prandom_u32(); > - n = (n % 100) + 1; > + n = prandom_u32_max(100) + 1; I agree that's wrong, but what rule did you use to make Cocci generate that? Jason
From: Jason A. Donenfeld > Sent: 07 October 2022 19:01 > > 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. > ... > --- a/lib/cmdline_kunit.c > +++ b/lib/cmdline_kunit.c > @@ -76,7 +76,7 @@ static void cmdline_test_lead_int(struct kunit *test) > int rc = cmdline_test_values[i]; > int offset; > > - sprintf(in, "%u%s", prandom_u32_max(256), str); > + sprintf(in, "%u%s", get_random_int() % 256, str); > /* Only first '-' after the number will advance the pointer */ > offset = strlen(in) - strlen(str) + !!(rc == 2); > cmdline_do_one_test(test, in, rc, offset); > @@ -94,7 +94,7 @@ static void cmdline_test_tail_int(struct kunit *test) > int rc = strcmp(str, "") ? (strcmp(str, "-") ? 0 : 1) : 1; > int offset; > > - sprintf(in, "%s%u", str, prandom_u32_max(256)); > + sprintf(in, "%s%u", str, get_random_int() % 256); > /* > * Only first and leading '-' not followed by integer > * will advance the pointer. Something has gone backwards here.... And get_random_u8() looks a better fit. David - Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK Registration No: 1397386 (Wales)
On Sat, Oct 08, 2022 at 10:08:03PM +0000, David Laight wrote: > From: Jason A. Donenfeld > > Sent: 07 October 2022 19:01 > > > > 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. > > > ... > > --- a/lib/cmdline_kunit.c > > +++ b/lib/cmdline_kunit.c > > @@ -76,7 +76,7 @@ static void cmdline_test_lead_int(struct kunit *test) > > int rc = cmdline_test_values[i]; > > int offset; > > > > - sprintf(in, "%u%s", prandom_u32_max(256), str); > > + sprintf(in, "%u%s", get_random_int() % 256, str); > > /* Only first '-' after the number will advance the pointer */ > > offset = strlen(in) - strlen(str) + !!(rc == 2); > > cmdline_do_one_test(test, in, rc, offset); > > @@ -94,7 +94,7 @@ static void cmdline_test_tail_int(struct kunit *test) > > int rc = strcmp(str, "") ? (strcmp(str, "-") ? 0 : 1) : 1; > > int offset; > > > > - sprintf(in, "%s%u", str, prandom_u32_max(256)); > > + sprintf(in, "%s%u", str, get_random_int() % 256); > > /* > > * Only first and leading '-' not followed by integer > > * will advance the pointer. > > Something has gone backwards here.... > And get_random_u8() looks a better fit. Wrong patch version. > > David > > - > Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK > Registration No: 1397386 (Wales)
diff --git a/arch/arm64/kernel/process.c b/arch/arm64/kernel/process.c index 92bcc1768f0b..87203429f802 100644 --- a/arch/arm64/kernel/process.c +++ b/arch/arm64/kernel/process.c @@ -595,7 +595,7 @@ unsigned long __get_wchan(struct task_struct *p) unsigned long arch_align_stack(unsigned long sp) { if (!(current->personality & ADDR_NO_RANDOMIZE) && randomize_va_space) - sp -= get_random_int() & ~PAGE_MASK; + sp -= prandom_u32_max(PAGE_SIZE); return sp & ~0xf; } diff --git a/arch/loongarch/kernel/process.c b/arch/loongarch/kernel/process.c index 660492f064e7..1256e3582475 100644 --- a/arch/loongarch/kernel/process.c +++ b/arch/loongarch/kernel/process.c @@ -293,7 +293,7 @@ unsigned long stack_top(void) unsigned long arch_align_stack(unsigned long sp) { if (!(current->personality & ADDR_NO_RANDOMIZE) && randomize_va_space) - sp -= get_random_int() & ~PAGE_MASK; + sp -= prandom_u32_max(PAGE_SIZE); return sp & STACK_ALIGN; } diff --git a/arch/loongarch/kernel/vdso.c b/arch/loongarch/kernel/vdso.c index f32c38abd791..8c9826062652 100644 --- a/arch/loongarch/kernel/vdso.c +++ b/arch/loongarch/kernel/vdso.c @@ -78,7 +78,7 @@ static unsigned long vdso_base(void) unsigned long base = STACK_TOP; if (current->flags & PF_RANDOMIZE) { - base += get_random_int() & (VDSO_RANDOMIZE_SIZE - 1); + base += prandom_u32_max(VDSO_RANDOMIZE_SIZE); base = PAGE_ALIGN(base); } diff --git a/arch/mips/kernel/process.c b/arch/mips/kernel/process.c index 35b912bce429..bbe9ce471791 100644 --- a/arch/mips/kernel/process.c +++ b/arch/mips/kernel/process.c @@ -711,7 +711,7 @@ unsigned long mips_stack_top(void) unsigned long arch_align_stack(unsigned long sp) { if (!(current->personality & ADDR_NO_RANDOMIZE) && randomize_va_space) - sp -= get_random_int() & ~PAGE_MASK; + sp -= prandom_u32_max(PAGE_SIZE); return sp & ALMASK; } diff --git a/arch/mips/kernel/vdso.c b/arch/mips/kernel/vdso.c index b2cc2c2dd4bf..5fd9bf1d596c 100644 --- a/arch/mips/kernel/vdso.c +++ b/arch/mips/kernel/vdso.c @@ -79,7 +79,7 @@ static unsigned long vdso_base(void) } if (current->flags & PF_RANDOMIZE) { - base += get_random_int() & (VDSO_RANDOMIZE_SIZE - 1); + base += prandom_u32_max(VDSO_RANDOMIZE_SIZE); base = PAGE_ALIGN(base); } diff --git a/arch/parisc/kernel/vdso.c b/arch/parisc/kernel/vdso.c index 63dc44c4c246..47e5960a2f96 100644 --- a/arch/parisc/kernel/vdso.c +++ b/arch/parisc/kernel/vdso.c @@ -75,7 +75,7 @@ int arch_setup_additional_pages(struct linux_binprm *bprm, map_base = mm->mmap_base; if (current->flags & PF_RANDOMIZE) - map_base -= (get_random_int() & 0x1f) * PAGE_SIZE; + map_base -= prandom_u32_max(0x20) * PAGE_SIZE; vdso_text_start = get_unmapped_area(NULL, map_base, vdso_text_len, 0, 0); diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c index 0fbda89cd1bb..ff920f4d4317 100644 --- a/arch/powerpc/kernel/process.c +++ b/arch/powerpc/kernel/process.c @@ -2308,6 +2308,6 @@ void notrace __ppc64_runlatch_off(void) unsigned long arch_align_stack(unsigned long sp) { if (!(current->personality & ADDR_NO_RANDOMIZE) && randomize_va_space) - sp -= get_random_int() & ~PAGE_MASK; + sp -= prandom_u32_max(PAGE_SIZE); return sp & ~0xf; } diff --git a/arch/s390/kernel/process.c b/arch/s390/kernel/process.c index d5119e039d85..5ec78555dd2e 100644 --- a/arch/s390/kernel/process.c +++ b/arch/s390/kernel/process.c @@ -224,7 +224,7 @@ unsigned long __get_wchan(struct task_struct *p) unsigned long arch_align_stack(unsigned long sp) { if (!(current->personality & ADDR_NO_RANDOMIZE) && randomize_va_space) - sp -= get_random_int() & ~PAGE_MASK; + sp -= prandom_u32_max(PAGE_SIZE); return sp & ~0xf; } diff --git a/drivers/block/drbd/drbd_receiver.c b/drivers/block/drbd/drbd_receiver.c index af4c7d65490b..d8b1417dc503 100644 --- a/drivers/block/drbd/drbd_receiver.c +++ b/drivers/block/drbd/drbd_receiver.c @@ -781,7 +781,7 @@ static struct socket *drbd_wait_for_connect(struct drbd_connection *connection, timeo = connect_int * HZ; /* 28.5% random jitter */ - timeo += (prandom_u32() & 1) ? timeo / 7 : -timeo / 7; + timeo += prandom_u32_max(2) ? timeo / 7 : -timeo / 7; err = wait_for_completion_interruptible_timeout(&ad->door_bell, timeo); if (err <= 0) @@ -1004,7 +1004,7 @@ static int conn_connect(struct drbd_connection *connection) drbd_warn(connection, "Error receiving initial packet\n"); sock_release(s); randomize: - if (prandom_u32() & 1) + if (prandom_u32_max(2)) goto retry; } } diff --git a/drivers/md/bcache/request.c b/drivers/md/bcache/request.c index f2c5a7e06fa9..3427555b0cca 100644 --- a/drivers/md/bcache/request.c +++ b/drivers/md/bcache/request.c @@ -401,7 +401,7 @@ static bool check_should_bypass(struct cached_dev *dc, struct bio *bio) } if (bypass_torture_test(dc)) { - if ((get_random_int() & 3) == 3) + if (prandom_u32_max(4) == 3) goto skip; else goto rescale; diff --git a/drivers/mtd/tests/stresstest.c b/drivers/mtd/tests/stresstest.c index cb29c8c1b370..d2faaca7f19d 100644 --- a/drivers/mtd/tests/stresstest.c +++ b/drivers/mtd/tests/stresstest.c @@ -45,9 +45,8 @@ static int rand_eb(void) unsigned int eb; again: - eb = prandom_u32(); /* Read or write up 2 eraseblocks at a time - hence 'ebcnt - 1' */ - eb %= (ebcnt - 1); + eb = prandom_u32_max(ebcnt - 1); if (bbt[eb]) goto again; return eb; @@ -55,20 +54,12 @@ static int rand_eb(void) static int rand_offs(void) { - unsigned int offs; - - offs = prandom_u32(); - offs %= bufsize; - return offs; + return prandom_u32_max(bufsize); } static int rand_len(int offs) { - unsigned int len; - - len = prandom_u32(); - len %= (bufsize - offs); - return len; + return prandom_u32_max(bufsize - offs); } static int do_read(void) @@ -127,7 +118,7 @@ static int do_write(void) static int do_operation(void) { - if (prandom_u32() & 1) + if (prandom_u32_max(2)) return do_read(); else return do_write(); diff --git a/drivers/mtd/ubi/debug.h b/drivers/mtd/ubi/debug.h index 118248a5d7d4..dc8d8f83657a 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; } @@ -87,7 +87,7 @@ static inline int ubi_dbg_is_bitflip(const struct ubi_device *ubi) static inline int ubi_dbg_is_write_failure(const struct ubi_device *ubi) { if (ubi->dbg.emulate_io_failures) - return !(prandom_u32() % 500); + return !prandom_u32_max(500); return 0; } @@ -101,7 +101,7 @@ static inline int ubi_dbg_is_write_failure(const struct ubi_device *ubi) static inline int ubi_dbg_is_erase_failure(const struct ubi_device *ubi) { if (ubi->dbg.emulate_io_failures) - return !(prandom_u32() % 400); + return !prandom_u32_max(400); return 0; } diff --git a/drivers/net/ethernet/broadcom/cnic.c b/drivers/net/ethernet/broadcom/cnic.c index e86503d97f32..f597b313acaa 100644 --- a/drivers/net/ethernet/broadcom/cnic.c +++ b/drivers/net/ethernet/broadcom/cnic.c @@ -4105,8 +4105,7 @@ static int cnic_cm_alloc_mem(struct cnic_dev *dev) for (i = 0; i < MAX_CM_SK_TBL_SZ; i++) atomic_set(&cp->csk_tbl[i].ref_count, 0); - port_id = prandom_u32(); - port_id %= CNIC_LOCAL_PORT_RANGE; + port_id = prandom_u32_max(CNIC_LOCAL_PORT_RANGE); if (cnic_init_id_tbl(&cp->csk_port_tbl, CNIC_LOCAL_PORT_RANGE, CNIC_LOCAL_PORT_MIN, port_id)) { cnic_cm_free_mem(dev); diff --git a/fs/ext2/ialloc.c b/fs/ext2/ialloc.c index 998dd2ac8008..f4944c4dee60 100644 --- a/fs/ext2/ialloc.c +++ b/fs/ext2/ialloc.c @@ -277,8 +277,7 @@ static int find_group_orlov(struct super_block *sb, struct inode *parent) int best_ndir = inodes_per_group; int best_group = -1; - group = prandom_u32(); - parent_group = (unsigned)group % ngroups; + parent_group = prandom_u32_max(ngroups); for (i = 0; i < ngroups; i++) { group = (parent_group + i) % ngroups; desc = ext2_get_group_desc (sb, group, NULL); diff --git a/fs/ext4/ialloc.c b/fs/ext4/ialloc.c index f73e5eb43eae..36d5bc595cc2 100644 --- a/fs/ext4/ialloc.c +++ b/fs/ext4/ialloc.c @@ -463,10 +463,9 @@ static int find_group_orlov(struct super_block *sb, struct inode *parent, hinfo.hash_version = DX_HASH_HALF_MD4; hinfo.seed = sbi->s_hash_seed; ext4fs_dirhash(parent, qstr->name, qstr->len, &hinfo); - grp = hinfo.hash; + parent_group = hinfo.hash % ngroups; } else - grp = prandom_u32(); - parent_group = (unsigned)grp % ngroups; + parent_group = prandom_u32_max(ngroups); for (i = 0; i < ngroups; i++) { g = (parent_group + i) % ngroups; get_orlov_stats(sb, g, flex_size, &stats); diff --git a/fs/ubifs/lpt_commit.c b/fs/ubifs/lpt_commit.c index cd4d5726a78d..cfbc31f709f4 100644 --- a/fs/ubifs/lpt_commit.c +++ b/fs/ubifs/lpt_commit.c @@ -1970,7 +1970,7 @@ static int dbg_populate_lsave(struct ubifs_info *c) if (!dbg_is_chk_gen(c)) return 0; - if (prandom_u32() & 3) + if (prandom_u32_max(4)) return 0; for (i = 0; i < c->lsave_cnt; i++) diff --git a/fs/ubifs/tnc_commit.c b/fs/ubifs/tnc_commit.c index 58c92c96ecef..01362ad5f804 100644 --- a/fs/ubifs/tnc_commit.c +++ b/fs/ubifs/tnc_commit.c @@ -700,7 +700,7 @@ static int alloc_idx_lebs(struct ubifs_info *c, int cnt) c->ilebs[c->ileb_cnt++] = lnum; dbg_cmt("LEB %d", lnum); } - if (dbg_is_chk_index(c) && !(prandom_u32() & 7)) + if (dbg_is_chk_index(c) && !prandom_u32_max(8)) return -ENOSPC; return 0; } diff --git a/fs/xfs/libxfs/xfs_alloc.c b/fs/xfs/libxfs/xfs_alloc.c index e2bdf089c0a3..6261599bb389 100644 --- a/fs/xfs/libxfs/xfs_alloc.c +++ b/fs/xfs/libxfs/xfs_alloc.c @@ -1520,7 +1520,7 @@ xfs_alloc_ag_vextent_lastblock( #ifdef DEBUG /* Randomly don't execute the first algorithm. */ - if (prandom_u32() & 1) + if (prandom_u32_max(2)) return 0; #endif diff --git a/fs/xfs/libxfs/xfs_ialloc.c b/fs/xfs/libxfs/xfs_ialloc.c index 6cdfd64bc56b..7838b31126e2 100644 --- a/fs/xfs/libxfs/xfs_ialloc.c +++ b/fs/xfs/libxfs/xfs_ialloc.c @@ -636,7 +636,7 @@ xfs_ialloc_ag_alloc( /* randomly do sparse inode allocations */ if (xfs_has_sparseinodes(tp->t_mountp) && igeo->ialloc_min_blks < igeo->ialloc_blks) - do_sparse = prandom_u32() & 1; + do_sparse = prandom_u32_max(2); #endif /* diff --git a/include/linux/nodemask.h b/include/linux/nodemask.h index 4b71a96190a8..66ee9b4b7925 100644 --- a/include/linux/nodemask.h +++ b/include/linux/nodemask.h @@ -509,7 +509,7 @@ static inline int node_random(const nodemask_t *maskp) w = nodes_weight(*maskp); if (w) bit = bitmap_ord_to_pos(maskp->bits, - get_random_int() % w, MAX_NUMNODES); + prandom_u32_max(w), MAX_NUMNODES); return bit; #else return 0; diff --git a/lib/cmdline_kunit.c b/lib/cmdline_kunit.c index e6a31c927b06..a72a2c16066e 100644 --- a/lib/cmdline_kunit.c +++ b/lib/cmdline_kunit.c @@ -76,7 +76,7 @@ static void cmdline_test_lead_int(struct kunit *test) int rc = cmdline_test_values[i]; int offset; - sprintf(in, "%u%s", prandom_u32_max(256), str); + sprintf(in, "%u%s", get_random_int() % 256, str); /* Only first '-' after the number will advance the pointer */ offset = strlen(in) - strlen(str) + !!(rc == 2); cmdline_do_one_test(test, in, rc, offset); @@ -94,7 +94,7 @@ static void cmdline_test_tail_int(struct kunit *test) int rc = strcmp(str, "") ? (strcmp(str, "-") ? 0 : 1) : 1; int offset; - sprintf(in, "%s%u", str, prandom_u32_max(256)); + sprintf(in, "%s%u", str, get_random_int() % 256); /* * Only first and leading '-' not followed by integer * will advance the pointer. diff --git a/lib/kobject.c b/lib/kobject.c index 5f0e71ab292c..a0b2dbfcfa23 100644 --- a/lib/kobject.c +++ b/lib/kobject.c @@ -694,7 +694,7 @@ static void kobject_release(struct kref *kref) { struct kobject *kobj = container_of(kref, struct kobject, kref); #ifdef CONFIG_DEBUG_KOBJECT_RELEASE - unsigned long delay = HZ + HZ * (get_random_int() & 0x3); + unsigned long delay = HZ + HZ * prandom_u32_max(4); pr_info("kobject: '%s' (%p): %s, parent %p (delayed %ld)\n", kobject_name(kobj), kobj, __func__, kobj->parent, delay); INIT_DELAYED_WORK(&kobj->release, kobject_delayed_cleanup); diff --git a/lib/reed_solomon/test_rslib.c b/lib/reed_solomon/test_rslib.c index 6faf9c9a6215..4d241bdc88aa 100644 --- a/lib/reed_solomon/test_rslib.c +++ b/lib/reed_solomon/test_rslib.c @@ -199,7 +199,7 @@ static int get_rcw_we(struct rs_control *rs, struct wspace *ws, derrlocs[i] = errloc; - if (ewsc && (prandom_u32() & 1)) { + if (ewsc && prandom_u32_max(2)) { /* Erasure with the symbol intact */ errlocs[errloc] = 2; } else { diff --git a/lib/sbitmap.c b/lib/sbitmap.c index c4f04edf3ee9..ef0661504561 100644 --- a/lib/sbitmap.c +++ b/lib/sbitmap.c @@ -21,7 +21,7 @@ static int init_alloc_hint(struct sbitmap *sb, gfp_t flags) int i; for_each_possible_cpu(i) - *per_cpu_ptr(sb->alloc_hint, i) = prandom_u32() % depth; + *per_cpu_ptr(sb->alloc_hint, i) = prandom_u32_max(depth); } return 0; } diff --git a/lib/test_hexdump.c b/lib/test_hexdump.c index 0927f44cd478..41a0321f641a 100644 --- a/lib/test_hexdump.c +++ b/lib/test_hexdump.c @@ -208,7 +208,7 @@ static void __init test_hexdump_overflow(size_t buflen, size_t len, static void __init test_hexdump_overflow_set(size_t buflen, bool ascii) { unsigned int i = 0; - int rs = (prandom_u32_max(2) + 1) * 16; + int rs = prandom_u32_max(2) + 1 * 16; do { int gs = 1 << i; diff --git a/lib/test_vmalloc.c b/lib/test_vmalloc.c index 4f2f2d1bac56..56ffaa8dd3f6 100644 --- a/lib/test_vmalloc.c +++ b/lib/test_vmalloc.c @@ -151,9 +151,7 @@ static int random_size_alloc_test(void) int i; for (i = 0; i < test_loop_count; i++) { - n = prandom_u32(); - n = (n % 100) + 1; - + n = prandom_u32_max(n % 100) + 1; p = vmalloc(n * PAGE_SIZE); if (!p) @@ -293,16 +291,12 @@ pcpu_alloc_test(void) return -1; for (i = 0; i < 35000; i++) { - unsigned int r; - - r = prandom_u32(); - size = (r % (PAGE_SIZE / 4)) + 1; + size = prandom_u32_max(PAGE_SIZE / 4) + 1; /* * Maximum PAGE_SIZE */ - r = prandom_u32(); - align = 1 << ((r % 11) + 1); + align = 1 << (prandom_u32_max(11) + 1); pcpu[i] = __alloc_percpu(size, align); if (!pcpu[i]) @@ -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]); diff --git a/net/core/pktgen.c b/net/core/pktgen.c index a13ee452429e..5ca4f953034c 100644 --- a/net/core/pktgen.c +++ b/net/core/pktgen.c @@ -2469,11 +2469,11 @@ static void mod_cur_headers(struct pktgen_dev *pkt_dev) } if ((pkt_dev->flags & F_VID_RND) && (pkt_dev->vlan_id != 0xffff)) { - pkt_dev->vlan_id = prandom_u32() & (4096 - 1); + pkt_dev->vlan_id = prandom_u32_max(4096); } if ((pkt_dev->flags & F_SVID_RND) && (pkt_dev->svlan_id != 0xffff)) { - pkt_dev->svlan_id = prandom_u32() & (4096 - 1); + pkt_dev->svlan_id = prandom_u32_max(4096); } if (pkt_dev->udp_src_min < pkt_dev->udp_src_max) { diff --git a/net/ipv4/inet_hashtables.c b/net/ipv4/inet_hashtables.c index b9d995b5ce24..9dc070f2018e 100644 --- a/net/ipv4/inet_hashtables.c +++ b/net/ipv4/inet_hashtables.c @@ -794,7 +794,7 @@ int __inet_hash_connect(struct inet_timewait_death_row *death_row, * on low contention the randomness is maximal and on high contention * it may be inexistent. */ - i = max_t(int, i, (prandom_u32() & 7) * 2); + i = max_t(int, i, prandom_u32_max(8) * 2); WRITE_ONCE(table_perturb[index], READ_ONCE(table_perturb[index]) + i + 2); /* Head lock still held and bh's disabled */ diff --git a/net/sunrpc/cache.c b/net/sunrpc/cache.c index c3c693b51c94..f075a9fb5ccc 100644 --- a/net/sunrpc/cache.c +++ b/net/sunrpc/cache.c @@ -677,7 +677,7 @@ static void cache_limit_defers(void) /* Consider removing either the first or the last */ if (cache_defer_cnt > DFR_MAX) { - if (prandom_u32() & 1) + if (prandom_u32_max(2)) discard = list_entry(cache_defer_list.next, struct cache_deferred_req, recent); else diff --git a/net/sunrpc/xprtsock.c b/net/sunrpc/xprtsock.c index e976007f4fd0..c2caee703d2c 100644 --- a/net/sunrpc/xprtsock.c +++ b/net/sunrpc/xprtsock.c @@ -1619,7 +1619,7 @@ static int xs_get_random_port(void) if (max < min) return -EADDRINUSE; range = max - min + 1; - rand = (unsigned short) prandom_u32() % range; + rand = (unsigned short) prandom_u32_max(range); return rand + min; }