Re: [RFC PATCH v1 2/2] randomize_kstack: Unify random source across arches

[Ryan Roberts <ryan.roberts@arm.com>]

On 02/12/2025 09:15, Ard Biesheuvel wrote:
> On Mon, 1 Dec 2025 at 19:20, Ryan Roberts <ryan.roberts@arm.com> wrote:
>>
>> On 28/11/2025 11:01, Ard Biesheuvel wrote:
>>> On Thu, 27 Nov 2025 at 12:00, Ryan Roberts <ryan.roberts@arm.com> wrote:
>>>>

[...]

>>>>  static inline void random_kstack_task_init(struct task_struct *tsk)
>>>>  {
>>>> -       current->kstack_offset = 0;
>>>> +       if (static_branch_maybe(CONFIG_RANDOMIZE_KSTACK_OFFSET_DEFAULT,
>>>> +                               &randomize_kstack_offset)) {
>>>> +               prandom_seed_state(&tsk->kstack_rnd_state, get_random_u64());
>>>
>>> We should either fix prandom_seed_state() not to truncate the u64 to
>>> u32, or even better, refactor prandom_seed_full_state() so we can
>>> reuse it here, and use a 128-bit seed directly.
>>
>> How about something like this:
>>
> 
> Looks good to me, but it does make we wonder why a full prandom state
> (i.e., one per cpu) isn't sufficient here, and why we are putting one
> in every task. Is it just to avoid the overhead of dis/enabling
> preemption on every syscall entry?

Good point, I'm not sure there is a good reason. It's just the shape I ended up
with after the first patch that fixes the bugs. But given we are removing the
second API call in this patch, the migraiton bug goes away. As you say, we could
disable preemption around the prandom_seed_state() call and it should all be
safe. I'll benchmark it.

> 
> 
> 
> 
> 
>>
>> ---8<---
>> diff --git a/include/linux/prandom.h b/include/linux/prandom.h
>> index f2ed5b72b3d6..9b651c9b3448 100644
>> --- a/include/linux/prandom.h
>> +++ b/include/linux/prandom.h
>> @@ -19,10 +19,11 @@ struct rnd_state {
>>
>>  u32 prandom_u32_state(struct rnd_state *state);
>>  void prandom_bytes_state(struct rnd_state *state, void *buf, size_t nbytes);
>> -void prandom_seed_full_state(struct rnd_state __percpu *pcpu_state);
>> +void prandom_seed_full_state_one(struct rnd_state *state);
>> +void prandom_seed_full_state_all(struct rnd_state __percpu *pcpu_state);
>>
>>  #define prandom_init_once(pcpu_state)                  \
>> -       DO_ONCE(prandom_seed_full_state, (pcpu_state))
>> +       DO_ONCE(prandom_seed_full_state_all, (pcpu_state))
>>
>>  /*
>>   * Handle minimum values for seeds
>> diff --git a/lib/random32.c b/lib/random32.c
>> index 24e7acd9343f..50d8f5f9fca7 100644
>> --- a/lib/random32.c
>> +++ b/lib/random32.c
>> @@ -107,24 +107,28 @@ static void prandom_warmup(struct rnd_state *state)
>>         prandom_u32_state(state);
>>  }
>>
>> -void prandom_seed_full_state(struct rnd_state __percpu *pcpu_state)
>> +void prandom_seed_full_state_one(struct rnd_state *state)
>>  {
>> -       int i;
>> +       u32 seeds[4];
>>
>> -       for_each_possible_cpu(i) {
>> -               struct rnd_state *state = per_cpu_ptr(pcpu_state, i);
>> -               u32 seeds[4];
>> +       get_random_bytes(&seeds, sizeof(seeds));
>> +       state->s1 = __seed(seeds[0],   2U);
>> +       state->s2 = __seed(seeds[1],   8U);
>> +       state->s3 = __seed(seeds[2],  16U);
>> +       state->s4 = __seed(seeds[3], 128U);
>>
>> -               get_random_bytes(&seeds, sizeof(seeds));
>> -               state->s1 = __seed(seeds[0],   2U);
>> -               state->s2 = __seed(seeds[1],   8U);
>> -               state->s3 = __seed(seeds[2],  16U);
>> -               state->s4 = __seed(seeds[3], 128U);
>> +       prandom_warmup(state);
>> +}
>> +EXPORT_SYMBOL(prandom_seed_full_state_one);
>>
>> -               prandom_warmup(state);
>> -       }
>> +void prandom_seed_full_state_all(struct rnd_state __percpu *pcpu_state)
>> +{
>> +       int i;
>> +
>> +       for_each_possible_cpu(i)
>> +               prandom_seed_full_state_one(per_cpu_ptr(pcpu_state, i));
>>  }
>> -EXPORT_SYMBOL(prandom_seed_full_state);
>> +EXPORT_SYMBOL(prandom_seed_full_state_all);
>>
>>  #ifdef CONFIG_RANDOM32_SELFTEST
>>  static struct prandom_test1 {
>> ---8<---
>>