Re: [PATCH 0/6] add system vulnerability sysfs entries

[Jeremy Linton]

Hi Dave,

Thanks for looking at this!

On 12/13/2018 06:07 AM, Dave Martin wrote:
> On Thu, Dec 06, 2018 at 05:44:02PM -0600, Jeremy Linton wrote:
> > Part of this series was originally by Mian Yousaf Kaukab.
> >
> > Arm64 machines should be displaying a human readable
> > vulnerability status to speculative execution attacks in
> > /sys/devices/system/cpu/vulnerabilities
>
> Is there any agreement on the strings that will be returned in there?
>
> A quick search didn't find anything obvious upstream.  There is
> documentation proposed in [1], but I don't know what happened to it and
> it doesn't define the mitigation strings at all.  (I didn't follow the
> discussion, so there is likely background here I'm not aware of.)
>
> If the mitigation strings are meaningful at all, they really ought to be
> documented somewhere since this is ABI.

I think they are in testing? But that documentation is missing the 
"Unknown" state which tends to be our default case for "we don't have a 
complete white/black list", or "mitigation disabled, we don't know if 
your vulnerable", etc.

I'm not sure I like the "Unknown" state, but we can try to add it to the 
documentation.

> > This series enables that behavior by providing the expected
> > functions. Those functions expose the cpu errata and feature
> > states, as well as whether firmware is responding appropriately
> > to display the overall machine status. This means that in a
> > heterogeneous machine we will only claim the machine is mitigated
> > or safe if we are confident all booted cores are safe or
> > mitigated. Otherwise, we will display unknown or unsafe
> > depending on how much of the machine configuration can
> > be assured.
>
> Can the vulnerability status change once we enter userspace?

Generally no, for heterogeneous machines I think the answer here is yes, 
a user could check the state, and have it read "Not affected" then bring 
another core online which causes the state to change at which point if 
they re-read /sys it may reflect another state. OTOH, given that we tend 
to default to mitigated usually this shouldn't be an issue unless 
someone has disabled the mitigation.


> I see no locking or other concurrency protections, and various global
> variables that could be __ro_after_init if nothing will change them
> after boot.

I think the state changes are all protected due to the fact the bringing 
a core online/offline is serialized.

> If they can change after boot, userspace has no way to be notified,

Is checking on hotplug notification sufficient?


> (I haven't grokked the patches fully, so the answer to this question may
> be reasonably straightforward...)
>
>
> Cheers
> ---Dave
>
> [1] https://lkml.org/lkml/2018/1/8/145