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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <202406201127.17CE526F0@keescook>
Date: Thu, 20 Jun 2024 11:34:22 -0700
From: Kees Cook <kees@...nel.org>
To: "liuyuntao (F)" <liuyuntao12@...wei.com>
Cc: Arnd Bergmann <arnd@...db.de>, Mark Rutland <mark.rutland@....com>,
	Catalin Marinas <catalin.marinas@....com>,
	Will Deacon <will@...nel.org>, Heiko Carstens <hca@...ux.ibm.com>,
	Vasily Gorbik <gor@...ux.ibm.com>,
	Alexander Gordeev <agordeev@...ux.ibm.com>,
	Christian Borntraeger <borntraeger@...ux.ibm.com>,
	Sven Schnelle <svens@...ux.ibm.com>,
	Thomas Gleixner <tglx@...utronix.de>,
	Ingo Molnar <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>,
	Dave Hansen <dave.hansen@...ux.intel.com>,
	"H. Peter Anvin" <hpa@...or.com>,
	"Gustavo A. R. Silva" <gustavoars@...nel.org>,
	Paul Walmsley <paul.walmsley@...ive.com>,
	Palmer Dabbelt <palmer@...belt.com>,
	Albert Ou <aou@...s.berkeley.edu>,
	Leonardo Bras <leobras@...hat.com>,
	Claudio Imbrenda <imbrenda@...ux.ibm.com>,
	Pawan Gupta <pawan.kumar.gupta@...ux.intel.com>,
	linux-kernel@...r.kernel.org, x86@...nel.org,
	linux-arm-kernel@...ts.infradead.org, linux-s390@...r.kernel.org,
	linux-hardening@...r.kernel.org, linux-riscv@...ts.infradead.org
Subject: Re: [PATCH] randomize_kstack: Remove non-functional per-arch entropy
 filtering

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