Re: [PATCH 1/1] arm64: syscall: Direct PRNG kstack randomization

From: "Arnd Bergmann" <arnd@arndb.de>
To: "Jeremy Linton" <jeremy.linton@arm.com>,
	"Kees Cook" <keescook@chromium.org>
Cc: linux-arm-kernel@lists.infradead.org,
	"Catalin Marinas" <catalin.marinas@arm.com>,
	"Will Deacon" <will@kernel.org>,
	"Jason A . Donenfeld" <Jason@zx2c4.com>,
	"Gustavo A. R. Silva" <gustavoars@kernel.org>,
	"Mark Rutland" <mark.rutland@arm.com>,
	"Steven Rostedt" <rostedt@goodmis.org>,
	"Mark Brown" <broonie@kernel.org>,
	"Guo Hui" <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>
Subject: Re: [PATCH 1/1] arm64: syscall: Direct PRNG kstack randomization
Date: Sat, 23 Mar 2024 13:47:29 +0100	[thread overview]
Message-ID: <a4a8e28d-ab9b-41cd-92e2-7ef111efd5a3@app.fastmail.com> (raw)
In-Reply-To: <2ebe2ea5-b107-4020-8e60-ff8cf43a3aab@arm.com>

On Sat, Mar 23, 2024, at 00:40, Jeremy Linton wrote:
> On 3/8/24 14:29, Arnd Bergmann wrote:
>> On Fri, Mar 8, 2024, at 17:49, Jeremy Linton wrote:
>>> On 3/7/24 05:10, Arnd Bergmann wrote:
>>>>
>>>> I'm not sure I understand the logic. Do you mean that accessing
>>>> CNTVCT itself is slow, or that reseeding based on CNTVCT is slow
>>>> because of the overhead of reseeding?
>>>
>>> Slow, as in, its running at a much lower frequency than a cycle counter.
>> 
>> Ok, I see. Would it be possible to use PMEVCNTR0 instead?
>
> So, I presume you actually mean PMCCNTR_EL0 because I don't see the 
> point of a dedicated event, although maybe...

Right, that would make more sense.

> So, the first and maybe largest problem is the PMxxx registers are all 
> optional because the PMU is optional! Although, they are strongly 
> recommended and most (AFAIK) machines do implement them. So, maybe if 
> its just using a cycle counter to dump some entropy into rnd_state it 
> becomes a statement that kstack randomization is slower or unsupported 
> if there isn't a PMU?

I think that sounds workable, especially as there is already
the randomize_kstack_offset=on/off conditional at boot time, it
could fall back to just not randomizing and print a warning
if the feature is enabled but unavailable at boot time.

> But then we have to basically enable the PMU cycle counter globally, 
> which requires reworking how it works, because the cycle counter is 
> enabled/disabled and reset on the fly depending on whether the user is 
> trying to profile something. So, I have hacked that up, and it appears 
> to be working, although i'm not sure what kind of interaction will 
> happen with KVM yet.
>
> But I guess the larger question is whether its worth changing the PMU 
> behavior for this?

I don't know too much about how the PMU works in detail, but I'm
also worried about two possible issues that end up preventing us
from using it in practice:

- if enabling PMCCNTR_EL0 takes away one of the limited number
  of available counters, we probably don't want to go there

- similarly, I would expect it to have a nonzero power
  consumption if the default is to have the clock disabled
  and non-counting. Probably not a big deal for server machines,
  but could be an issue on battery powered embedded devices.

     Arnd