Fabiano Rosas <farosas@xxxxxxxxxxxxx> writes: > Nicholas Piggin <npiggin@xxxxxxxxx> writes: > >> Excerpts from Fabiano Rosas's message of March 6, 2021 9:10 am: >>> 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
