Re: [DISCUSSION] kstack offset randomization: bugs and performance

[Mark Rutland <mark.rutland@arm.com>]

On Mon, Nov 17, 2025 at 12:27:36PM -0800, Kees Cook wrote:
> On Mon, Nov 17, 2025 at 11:31:22AM +0000, Ryan Roberts wrote:
> > On 17/11/2025 11:30, Ryan Roberts wrote:
> > > I think we could just pass the random seed to add_random_kstack_offset() so that
> > > we consume the old and buffer the new atomically? We would still buffer it
> > > across syscalls to avoid the guessability issue that's documented. Then
> > > choose_random_kstack_offset() could be removed. Or we could store
> > > per-task_struct given it is only 32-bits?
> 
> I had wanted to avoid both growing task_struct and to avoid tying the
> randomness to a given task -- then unpredictability could be higher
> (barring the bug above), and could be only reduced to per-thread by
> pinning a thread exclusively to a single CPU.

I appreciate the rationale of not growing task struct, but I think the
"unpredictability could be higher" rationale doesn't hold any water.

If an adversary has userspace code execution, they can pin the thread to
a CPU. Regardless of whether they have that capability, in common cases
the thread isn't going to migrate for a number of syscalls if those are
non-blocking. The unpredictability gained by CPU migrations is slim to
none.

From a thread model perspective, we need this to be unpredictable
*regardless* of any pinning.

From a quick look at task struct, it appears that (on 64-bit platforms)
there are several opportunities for rearranging fields to pack things
together and save bytes. I think that if the state is sufficiently
small, we don't actually have to grow task struct. Practically speaking,
if this is small enough no-one will notice.

> > > Bug 2: add_random_kstack_offset() and choose_random_kstack_offset() both
> > > document their requirement to be called with interrupts and preemption disabled.
> > > They use raw_cpu_*(), which require this. But on arm64, they are called from
> > > invoke_syscall(), where interrupts and preemption are _enabled_. In practice, I
> > > don't think this will cause functional harm for arm64's implementations of
> > > raw_cpu_*(), but means that it's possible that the wrong per-cpu structure is
> > > being referred to. Perhaps there is a way for user code to exploit this to
> > > defeat the purpose of the feature.
> > > 
> > > This should be straightforward to fix; if we take the task_struct approach for
> > > bug 1, then that would also fix this issue too because the requirement to be in
> > > atomic context goes away. Otherwsise it can be moved earlier in the callchain,
> > > before interrupts are enabled.
> 
> I can't speak to these internals, just that I'd hope to avoid forcing
> the randomness down to the thread-level.

From my perspective this would be *much* nicer as a per-thread property,
since it wouldn't have any problematic interactions with preemption, and
wouldn't force awkward requirements on architecture code.

i.e. I completely disagree -- I think this would be much better per
thread.

Using siphash per thread sounds fine to me.

Mark.