[PATCH v2 1/3] Replace invocation of weak PRNG in mm/slab.c

david.keisarschm@mail.huji.ac.il posted 3 patches 2 years, 9 months ago
[PATCH v2 1/3] Replace invocation of weak PRNG in mm/slab.c
Posted by david.keisarschm@mail.huji.ac.il 2 years, 9 months ago
From: David <david.keisarschm@mail.huji.ac.il>

We changed the invocation
 of prandom_u32_state to get_random_u32.
 We also changed the freelist_init_state
 to struct instead of a union,
 since the rnd_state is not needed anymore
- get_random_u32 maintains its own state.
This change it important since it
is make the slab allocator randomization
stronger.

Signed-off-by: David <david.keisarschm@mail.huji.ac.il>
---
 mm/slab.c | 20 ++++++++------------
 1 file changed, 8 insertions(+), 12 deletions(-)

diff --git a/mm/slab.c b/mm/slab.c
index 92d6b1d48..1476104f4 100644
--- a/mm/slab.c
+++ b/mm/slab.c
@@ -2360,20 +2360,17 @@ static void cache_init_objs_debug(struct kmem_cache *cachep, struct slab *slab)
 
 #ifdef CONFIG_SLAB_FREELIST_RANDOM
 /* Hold information during a freelist initialization */
-union freelist_init_state {
-	struct {
-		unsigned int pos;
-		unsigned int *list;
-		unsigned int count;
-	};
-	struct rnd_state rnd_state;
+struct freelist_init_state {
+	unsigned int pos;
+	unsigned int *list;
+	unsigned int count;
 };
 
 /*
  * Initialize the state based on the randomization method available.
  * return true if the pre-computed list is available, false otherwise.
  */
-static bool freelist_state_initialize(union freelist_init_state *state,
+static bool freelist_state_initialize(struct freelist_init_state *state,
 				struct kmem_cache *cachep,
 				unsigned int count)
 {
@@ -2385,7 +2382,6 @@ static bool freelist_state_initialize(union freelist_init_state *state,
 
 	/* Use a random state if the pre-computed list is not available */
 	if (!cachep->random_seq) {
-		prandom_seed_state(&state->rnd_state, rand);
 		ret = false;
 	} else {
 		state->list = cachep->random_seq;
@@ -2397,7 +2393,7 @@ static bool freelist_state_initialize(union freelist_init_state *state,
 }
 
 /* Get the next entry on the list and randomize it using a random shift */
-static freelist_idx_t next_random_slot(union freelist_init_state *state)
+static freelist_idx_t next_random_slot(struct freelist_init_state *state)
 {
 	if (state->pos >= state->count)
 		state->pos = 0;
@@ -2418,7 +2414,7 @@ static void swap_free_obj(struct slab *slab, unsigned int a, unsigned int b)
 static bool shuffle_freelist(struct kmem_cache *cachep, struct slab *slab)
 {
 	unsigned int objfreelist = 0, i, rand, count = cachep->num;
-	union freelist_init_state state;
+	struct freelist_init_state state;
 	bool precomputed;
 
 	if (count < 2)
@@ -2447,7 +2443,7 @@ static bool shuffle_freelist(struct kmem_cache *cachep, struct slab *slab)
 
 		/* Fisher-Yates shuffle */
 		for (i = count - 1; i > 0; i--) {
-			rand = prandom_u32_state(&state.rnd_state);
+			rand = get_random_u32();
 			rand %= (i + 1);
 			swap_free_obj(slab, i, rand);
 		}
-- 
2.38.0
Re: [PATCH v2 1/3] Replace invocation of weak PRNG in mm/slab.c
Posted by Matthew Wilcox 2 years, 9 months ago
On Tue, Dec 13, 2022 at 12:34:57PM +0200, david.keisarschm@mail.huji.ac.il wrote:
> From: David <david.keisarschm@mail.huji.ac.il>

It's normal to include the surname in your sign-off, fwiw.

> @@ -2447,7 +2443,7 @@ static bool shuffle_freelist(struct kmem_cache *cachep, struct slab *slab)
>  
>  		/* Fisher-Yates shuffle */
>  		for (i = count - 1; i > 0; i--) {
> -			rand = prandom_u32_state(&state.rnd_state);
> +			rand = get_random_u32();
>  			rand %= (i + 1);

Shouldn't this be "rand = get_random_u32_below(i + 1)"?

>  			swap_free_obj(slab, i, rand);
>  		}
> -- 
> 2.38.0
> 
>