Re: [PATCH] KVM: PPC: Book3S HV: Do not expose HFSCR sanitisation to nested hypervisor

Fabiano Rosas [off-list ref] writes:

Nicholas Piggin [off-list ref] writes:

quoted

Excerpts from Fabiano Rosas's message of March 6, 2021 9:10 am:

quoted

As one of the arguments of the H_ENTER_NESTED hypercall, the nested
hypervisor (L1) prepares a structure containing the values of various
hypervisor-privileged registers with which it wants the nested guest
(L2) to run. Since the nested HV runs in supervisor mode it needs the
host to write to these registers.

To stop a nested HV manipulating this mechanism and using a nested
guest as a proxy to access a facility that has been made unavailable
to it, we have a routine that sanitises the values of the HV registers
before copying them into the nested guest's vcpu struct.

However, when coming out of the guest the values are copied as they
were back into L1 memory, which means that any sanitisation we did
during guest entry will be exposed to L1 after H_ENTER_NESTED returns.

This is not a problem by itself, but in the case of the Hypervisor
Facility Status and Control Register (HFSCR), we use the intersection
between L2 hfscr bits and L1 hfscr bits. That means that L1 could use
this to indirectly read the (hv-privileged) value from its vcpu
struct.

This patch fixes this by making sure that L1 only gets back the bits
that are necessary for regular functioning.

The general idea of restricting exposure of HV privileged bits, but
for the case of HFSCR a guest can probe the HFCR anyway by testing which 
facilities are available (and presumably an HV may need some way to know
what features are available for it to advertise to its own guests), so
is this necessary? Perhaps a comment would be sufficient.

Well, I'd be happy to force them through the arduous path then =); and
there are features that are emulated by the HV which L1 would not be
able to probe.

I think we should implement a mechanism that stops all leaks now, rather
than having to ponder about this every time we touch an hv_reg in that
structure. I'm not too worried about HFSCR specifically.

Let me think about this some more and see if I can make it more generic,
I realise that sticking the saved_hfscr on the side is not the most
elegant approach.

Yeah that would be good.

I don't really like the patch as it is, ie. having to pass *saved_hfscr
and so on.

But in general I agree that we should avoid leaking details across
boundaries, even if we don't think they are particularly sensitive.

cheers