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

[Arnd Bergmann]

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