Re: [RFC] arm64: syscall: Direct PRNG kstack randomization

[Jeremy Linton]

Hi,

Thanks for looking at this.

On 2/21/24 06:44, Jason A. Donenfeld wrote:
> Hi,
>
> On Wed, Feb 21, 2024 at 7:33 AM Kees Cook <keescook@xxxxxxxxxxxx> wrote:
> > > +#ifdef CONFIG_RANDOMIZE_KSTACK_OFFSET
> > > +DEFINE_PER_CPU(u32, kstackrng);
> > > +static u32 xorshift32(u32 state)
> > > +{
> > > +     /*
> > > +      * From top of page 4 of Marsaglia, "Xorshift RNGs"
> > > +      * This algorithm is intended to have a period 2^32 -1
> > > +      * And should not be used anywhere else outside of this
> > > +      * code path.
> > > +      */
> > > +     state ^= state << 13;
> > > +     state ^= state >> 17;
> > > +     state ^= state << 5;
> > > +     return state;
> > > +}
>
> Can we please *not* introduce yet another RNG? You can't just sprinkle
> this stuff all over the place with no rhyme or reason.
>
> If you need repeatable randomness, use prandom_u32_state() or similar.
> If you need non-repeatable randomness, use get_random_bytes() or
> similar.

Sure prandom_u32_state() should have a similar effect being a bit 
slower, and a bit better due to the extra hidden state.

> If you think prandom_u32_state() is insufficient for some reason or
> doesn't have some property or performance that you want, submit a
> patch to make it better.
>
> Looking at the actual intention here, of using repeatable randomness,
> I find the intent pretty weird. Isn't the whole point of kstack
> randomization that you can't predict it? If so, get_random_u*() is
> what you want. If performance isn't sufficient, let's figure out some

There isn't anything wrong with get_random_u16 from a kstack 
randomization standpoint, except for the latency spikes of course.

> way to improve performance. And as Kees said, if the point of this is
> to have some repeatable benchmarks, maybe just don't enable the
> security-intended code whose purpose is non-determinism? Both exploits
> and now apparently benchmarks like determinism.

As I mentioned in the other email, benchmark is probably the wrong word. 
Its a better QoS response time distributions for a given workload. And 
its not strictly in RT kernel latency test types of things, but normal 
memcached style workloads on !RT kernels as well.