[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <31187502-2a11-4ef3-82b4-927a271d8b44@arm.com>
Date: Mon, 19 Jan 2026 16:51:32 +0000
From: Ryan Roberts <ryan.roberts@....com>
To: Dave Hansen <dave.hansen@...el.com>,
Catalin Marinas <catalin.marinas@....com>, Will Deacon <will@...nel.org>,
Huacai Chen <chenhuacai@...nel.org>,
Madhavan Srinivasan <maddy@...ux.ibm.com>,
Michael Ellerman <mpe@...erman.id.au>, Paul Walmsley <pjw@...nel.org>,
Palmer Dabbelt <palmer@...belt.com>, Albert Ou <aou@...s.berkeley.edu>,
Heiko Carstens <hca@...ux.ibm.com>, Vasily Gorbik <gor@...ux.ibm.com>,
Alexander Gordeev <agordeev@...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>,
Kees Cook <kees@...nel.org>, "Gustavo A. R. Silva" <gustavoars@...nel.org>,
Arnd Bergmann <arnd@...db.de>, Mark Rutland <mark.rutland@....com>,
"Jason A. Donenfeld" <Jason@...c4.com>, Ard Biesheuvel <ardb@...nel.org>,
Jeremy Linton <jeremy.linton@....com>,
David Laight <david.laight.linux@...il.com>
Cc: linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
loongarch@...ts.linux.dev, linuxppc-dev@...ts.ozlabs.org,
linux-riscv@...ts.infradead.org, linux-s390@...r.kernel.org,
linux-hardening@...r.kernel.org, stable@...r.kernel.org
Subject: Re: [PATCH v4 1/3] randomize_kstack: Maintain kstack_offset per task
Thanks for the review!
On 19/01/2026 16:10, Dave Hansen wrote:
> On 1/19/26 05:01, Ryan Roberts wrote:
> ...
>> Cc: stable@...r.kernel.org
>
> Since this doesn't fix any known functional issues, if it were me, I'd
> leave stable@ alone. It isn't clear that this is stable material.
I listed 2 issues in the commit log; I agree that issue 1 falls into the
category of "don't really care", but issue 2 means that kstack randomization is
currently trivial to defeat. That's the reason I thought it would valuable in
stable.
But if you're saying don't bother and others agree, then this whole patch can be
dropped; this is just intended to be the backportable fix. Patch 3 reimplements
this entirely for upstream.
I'll wait and see if others have opinions if that's ok?
>
>> --- a/include/linux/sched.h
>> +++ b/include/linux/sched.h
>> @@ -1591,6 +1591,10 @@ struct task_struct {
>> unsigned long prev_lowest_stack;
>> #endif
>>
>> +#ifdef CONFIG_RANDOMIZE_KSTACK_OFFSET
>> + u32 kstack_offset;
>> +#endif
>> +
>> #ifdef CONFIG_X86_MCE
>> void __user *mce_vaddr;
>
> Nit: This seems to be throwing a u32 potentially in between a couple of
> void*/ulong sized objects.
Yeah, I spent a bit of time with pahole but eventually concluded that it was
difficult to find somewhere to nestle it that would work reliably cross arch.
Eventually I just decided to group it with other stack meta data.
>
> It probably doesn't matter with struct randomization and it's really
> hard to get right among the web of task_struct #ifdefs. But, it would be
> nice to at _least_ nestle this next to another int-sized thing.
>
> Does it really even need to be 32 bits? x86 has this comment:
>
>> /*
>> * This value will get limited by KSTACK_OFFSET_MAX(), which is 10
>> * bits. The actual entropy will be further reduced by the compiler
>> * when applying stack alignment constraints (see cc_stack_align4/8 in
>> * arch/x86/Makefile), which will remove the 3 (x86_64) or 2 (ia32)
>> * low bits from any entropy chosen here.
>> *
>> * Therefore, final stack offset entropy will be 7 (x86_64) or
>> * 8 (ia32) bits.
>> */
For more recent kernels it's 6 bits shifted by 4 for 64-bit kernels or 8 bits
shifted by 2 for 32-bit kernels regardless of arch. So could probably make it
work with 8 bits of storage. Although I was deliberately trying to keep the
change simple, since it was intended for backporting. Patch 3 rips it out.
Overall I'd prefer to leave it all as is. But if people don't think we should
backport, then let's just drop the whole patch.
Thanks,
Ryan
Powered by blists - more mailing lists