Received-SPF: pass (google.com: domain of linux-kernel+bounces-74712-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
Precedence: bulk
MIME-Version: 1.0
References: <20240221020258.1210148-1-jeremy.linton@arm.com> <202402202226.A6817927@keescook>
In-Reply-To: <202402202226.A6817927@keescook>
From: "Jason A. Donenfeld" <Jason@zx2c4.com>
Date: Wed, 21 Feb 2024 13:44:15 +0100
Message-ID: <CAHmME9pYS=YZM1b0ftrjj_qjz_U4V8PsiLzB15JmBRr39Av2Ng@mail.gmail.com>
Subject: Re: [RFC] arm64: syscall: Direct PRNG kstack randomization
To: Kees Cook <keescook@chromium.org>
Cc: Jeremy Linton <jeremy.linton@arm.com>, linux-arm-kernel@lists.infradead.org, 
	catalin.marinas@arm.com, will@kernel.org, gustavoars@kernel.org, 
	mark.rutland@arm.com, rostedt@goodmis.org, arnd@arndb.de, broonie@kernel.org, 
	guohui@uniontech.com, Manoj.Iyer@arm.com, linux-kernel@vger.kernel.org, 
	linux-hardening@vger.kernel.org, James Yang <james.yang@arm.com>, 
	Shiyou Huang <shiyou.huang@arm.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

Hi,

On Wed, Feb 21, 2024 at 7:33=E2=80=AFAM Kees Cook <keescook@chromium.org> w=
rote:
> > +#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 ^=3D state << 13;
> > +     state ^=3D state >> 17;
> > +     state ^=3D 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