linux-hardening - Re: [RFC] arm64: syscall: Direct PRNG kstack randomization

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]

Date: Wed, 21 Feb 2024 13:44:15 +0100
From: "Jason A. Donenfeld" <Jason@...c4.com>
To: Kees Cook <keescook@...omium.org>
Cc: Jeremy Linton <jeremy.linton@....com>, linux-arm-kernel@...ts.infradead.org, 
	catalin.marinas@....com, will@...nel.org, gustavoars@...nel.org, 
	mark.rutland@....com, rostedt@...dmis.org, arnd@...db.de, broonie@...nel.org, 
	guohui@...ontech.com, Manoj.Iyer@....com, linux-kernel@...r.kernel.org, 
	linux-hardening@...r.kernel.org, James Yang <james.yang@....com>, 
	Shiyou Huang <shiyou.huang@....com>
Subject: Re: [RFC] arm64: syscall: Direct PRNG kstack randomization

Hi,

On Wed, Feb 21, 2024 at 7:33 AM Kees Cook <keescook@...omium.org> 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.

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
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.

Jason