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

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

On Tue, Dec 02, 2025 at 10:15:22AM +0100, 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:
> > >> diff --git a/include/linux/randomize_kstack.h b/include/linux/randomize_kstack.h
> > >> index 089b1432f7e6..83c7e6710f6d 100644
> > >> --- a/include/linux/randomize_kstack.h
> > >> +++ b/include/linux/randomize_kstack.h
> > >> @@ -6,6 +6,7 @@
> > >>  #include <linux/kernel.h>
> > >>  #include <linux/jump_label.h>
> > >>  #include <linux/percpu-defs.h>
> > >> +#include <linux/prandom.h>
> > >>
> > >>  DECLARE_STATIC_KEY_MAYBE(CONFIG_RANDOMIZE_KSTACK_OFFSET_DEFAULT,
> > >>                          randomize_kstack_offset);
> > >> @@ -45,9 +46,13 @@ DECLARE_STATIC_KEY_MAYBE(CONFIG_RANDOMIZE_KSTACK_OFFSET_DEFAULT,
> > >>  #define KSTACK_OFFSET_MAX(x)   ((x) & 0b1111111100)
> > >>  #endif
> > >>
> > >> +static __always_inline u32 get_update_kstack_offset(void)
> > >> +{
> > >> +       return prandom_u32_state(&current->kstack_rnd_state);
> >
> > I've got bot warnings because this is being called from noinstr code. I guess
> > the best option is to just move add_random_kstack_offset() to after
> > instrumentation is enabled for the affected arches.
> 
> Just put instrumentation_begin()/instrumentation_end() around the call
> to prandom_u32_state() - that seems to be the common approach for
> punching holes into the 'noinstr' validation.

That silences the warning, but isn't necessarily safe, so please DO NOT
do that blindly. The instrumentation_{begin,end}() annotations are only
supposed to be used when we know by construction that instrumentation is
safe.

Generally, if you can move this to after instrumentation is already
enabled, that should be safe, and so that'd be the better approach.

Ryan, can you share those warnings (e.g. link to those reports)?

IIUC only x86 has noinstr validation, and from a quick scan, I expect
you see warnings from:

* do_syscall_64()
* do_int80_syscall_32()
* __do_fast_syscall_32()

For all of these, it is not safe to call instrumentable code before the
calls to {syscall_,}enter_from_user_mode{,_prepare}(). You'll need to
move the stack rnadomization after the existing instrumentation_begin()
calls.

We'll need to go check the other architectures similarly.

Mark.