Re: [PATCH] randomize_kstack: Remove non-functional per-arch entropy filtering

[Kees Cook <kees@kernel.org>]

On Thu, Jun 20, 2024 at 11:47:58AM +0800, liuyuntao (F) wrote:
> 
> 
> On 2024/6/20 5:47, Kees Cook wrote:
> > An unintended consequence of commit 9c573cd31343 ("randomize_kstack:
> > Improve entropy diffusion") was that the per-architecture entropy size
> > filtering reduced how many bits were being added to the mix, rather than
> > how many bits were being used during the offsetting. All architectures
> > fell back to the existing default of 0x3FF (10 bits), which will consume
> > at most 1KiB of stack space. It seems that this is working just fine,
> > so let's avoid the confusion and update everything to use the default.
> > 
> 
> My original intent was indeed to do this, but I regret that not being more
> explicit in the commit log..
> 
> Additionally, I've tested the stack entropy by applying the following patch,
> the result was `Bits of stack entropy: 7` on arm64, too. It does not seem to
> affect the entropy value, maybe removing it is OK, or there may be some
> nuances of your intentions that I've overlooked.
> 
> --- a/include/linux/randomize_kstack.h
> +++ b/include/linux/randomize_kstack.h
> @@ -79,9 +79,7 @@ DECLARE_PER_CPU(u32, kstack_offset);
>  #define choose_random_kstack_offset(rand) do {                         \
>         if (static_branch_maybe(CONFIG_RANDOMIZE_KSTACK_OFFSET_DEFAULT, \
>                                 &randomize_kstack_offset)) {            \
> -               u32 offset = raw_cpu_read(kstack_offset);               \
> -               offset = ror32(offset, 5) ^ (rand);                     \
> -               raw_cpu_write(kstack_offset, offset);                   \
> +               raw_cpu_write(kstack_offset, rand);                     \
>         }                                                               \
>  } while (0)
>  #else /* CONFIG_RANDOMIZE_KSTACK_OFFSET */

I blame the multiple applications of the word "entropy" in this feature. :)

So, there's both:

- "how many bits CAN be randomized?" (i.e. within what range can all
  possible stack offsets be?)

and

- "is the randomization predictable?" (i.e. is the distribution of
  selected positions with the above range evenly distributed?)

Commit 9c573cd31343 ("randomize_kstack: Improve entropy diffusion") was
trying to improve the latter, but accidentally also grew the former.
This patch is just trying to clean all this up now.

Thanks for testing! And I'm curious as to why arm64's stack offset
entropy is 7 for you when we're expecting it to be 6. Anyway, that's not
a problem I don't think. Just a greater offset range than expected.

-Kees

-- 
Kees Cook