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 patch alters this sanitisation to have effect on the vcpu->arch registers directly before entering and after exiting the guest, leaving the structure that is copied back into L1 unchanged (except when we really want L1 to access the value, e.g the Cause bits of HFSCR). Signed-off-by: Fabiano Rosas <farosas@xxxxxxxxxxxxx> --- I'm taking another shot at fixing this locally without resorting to more complex things such as error handling and feature advertisement/negotiation. Changes since v1: - made the change more generic, not only applies to hfscr anymore; - sanitisation is now done directly on the vcpu struct, l2_hv is left unchanged; v1: https://lkml.kernel.org/r/20210305231055.2913892-1-farosas@xxxxxxxxxxxxx --- arch/powerpc/kvm/book3s_hv_nested.c | 33 +++++++++++++++++++++++------ 1 file changed, 26 insertions(+), 7 deletions(-) diff --git a/arch/powerpc/kvm/book3s_hv_nested.c b/arch/powerpc/kvm/book3s_hv_nested.c index 0cd0e7aad588..a60fccb2c4f2 100644 --- a/arch/powerpc/kvm/book3s_hv_nested.c +++ b/arch/powerpc/kvm/book3s_hv_nested.c @@ -132,21 +132,37 @@ static void save_hv_return_state(struct kvm_vcpu *vcpu, int trap, } } -static void sanitise_hv_regs(struct kvm_vcpu *vcpu, struct hv_guest_state *hr) +static void sanitise_vcpu_entry_state(struct kvm_vcpu *vcpu, + const struct hv_guest_state *l2_hv, + const struct hv_guest_state *l1_hv) { /* * Don't let L1 enable features for L2 which we've disabled for L1, * but preserve the interrupt cause field. */ - hr->hfscr &= (HFSCR_INTR_CAUSE | vcpu->arch.hfscr); + vcpu->arch.hfscr = l2_hv->hfscr & (HFSCR_INTR_CAUSE | l1_hv->hfscr); /* Don't let data address watchpoint match in hypervisor state */ - hr->dawrx0 &= ~DAWRX_HYP; - hr->dawrx1 &= ~DAWRX_HYP; + vcpu->arch.dawrx0 = l2_hv->dawrx0 & ~DAWRX_HYP; + vcpu->arch.dawrx1 = l2_hv->dawrx1 & ~DAWRX_HYP; /* Don't let completed instruction address breakpt match in HV state */ - if ((hr->ciabr & CIABR_PRIV) == CIABR_PRIV_HYPER) - hr->ciabr &= ~CIABR_PRIV; + if ((l2_hv->ciabr & CIABR_PRIV) == CIABR_PRIV_HYPER) + vcpu->arch.ciabr = l2_hv->ciabr & ~CIABR_PRIV; +} + + +/* + * During sanitise_vcpu_entry_state() we might have used bits from L1 + * state to restrict what the L2 state is allowed to be. Since L1 is + * not allowed to read the HV registers, do not include these + * modifications in the return state. + */ +static void sanitise_vcpu_return_state(struct kvm_vcpu *vcpu, + const struct hv_guest_state *l2_hv) +{ + vcpu->arch.hfscr = ((~HFSCR_INTR_CAUSE & l2_hv->hfscr) | + (HFSCR_INTR_CAUSE & vcpu->arch.hfscr)); } static void restore_hv_regs(struct kvm_vcpu *vcpu, struct hv_guest_state *hr) @@ -324,9 +340,10 @@ long kvmhv_enter_nested_guest(struct kvm_vcpu *vcpu) mask = LPCR_DPFD | LPCR_ILE | LPCR_TC | LPCR_AIL | LPCR_LD | LPCR_LPES | LPCR_MER; lpcr = (vc->lpcr & ~mask) | (l2_hv.lpcr & mask); - sanitise_hv_regs(vcpu, &l2_hv); restore_hv_regs(vcpu, &l2_hv); + sanitise_vcpu_entry_state(vcpu, &l2_hv, &saved_l1_hv); + vcpu->arch.ret = RESUME_GUEST; vcpu->arch.trap = 0; do { @@ -338,6 +355,8 @@ long kvmhv_enter_nested_guest(struct kvm_vcpu *vcpu) r = kvmhv_run_single_vcpu(vcpu, hdec_exp, lpcr); } while (is_kvmppc_resume_guest(r)); + sanitise_vcpu_return_state(vcpu, &l2_hv); + /* save L2 state for return */ l2_regs = vcpu->arch.regs; l2_regs.msr = vcpu->arch.shregs.msr; -- 2.29.2
