2018-06-15 13:36-0700, Marc Orr:
> This patch extends the checks done prior to a nested VM entry.
> Specifically, it extends the check_vmentry_prereqs function with checks
> for fields relevant to the VM-entry event injection information, as
> described in the Intel SDM, volume 3.
>
> This patch is motivated by a syzkaller bug, where a bad VM-entry
> interruption information field is generated in the VMCS02, which causes
> the nested VM launch to fail. Then, KVM fails to resume L1.
>
> While KVM should be improved to correctly resume L1 execution after a
> failed nested launch, this change is justified because the existing code
> to resume L1 is flaky/ad-hoc and the test coverage for resuming L1 is
> sparse.
>
> Reported-by: syzbot <syzkaller@xxxxxxxxxxxxxxxx>
> Signed-off-by: Marc Orr <marcorr@xxxxxxxxxx>
> ---
> Changelog since v2:
> - Actually rename nr to vector
>
> Changelog since v1:
> - Renamed nr to vector
> - Colocate error code checks
> - Add Reported-by tag to commit message
>
> arch/x86/include/asm/vmx.h | 3 ++
> arch/x86/kvm/vmx.c | 67 ++++++++++++++++++++++++++++++++++++++
> arch/x86/kvm/x86.h | 14 ++++++++
> 3 files changed, 84 insertions(+)
>
> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
> @@ -1705,6 +1705,17 @@ static inline bool nested_cpu_has_vmwrite_any_field(struct kvm_vcpu *vcpu)
> MSR_IA32_VMX_MISC_VMWRITE_SHADOW_RO_FIELDS;
> }
>
> +static inline bool nested_cpu_has_zero_length_injection(struct kvm_vcpu *vcpu)
> +{
> + return to_vmx(vcpu)->nested.msrs.misc_low & VMX_MISC_ZERO_LEN_INS;
> +}
> +
> +static inline bool nested_cpu_has_monitor_trap_flag(struct kvm_vcpu *vcpu)
> +{
> + return to_vmx(vcpu)->nested.msrs.procbased_ctls_low &
We should be looking at the _high bits of the MSR.
(_low bits say if it must be enabled, _high if it can.)
> + CPU_BASED_MONITOR_TRAP_FLAG;
> +}
> +
> diff --git a/arch/x86/kvm/x86.h b/arch/x86/kvm/x86.h
> @@ -110,6 +110,20 @@ static inline bool is_la57_mode(struct kvm_vcpu *vcpu)
> #endif
> }
>
> +/*
> + * vector: x86 exception number; often called nr
> + * protected_mode: true if !unrestricted-guest || protected mode
> + */
> +static inline bool x86_exception_has_error_code(unsigned int vector,
> + bool protected_mode)
> +{
> + static u32 exception_has_error_code = BIT(DF_VECTOR) | BIT(TS_VECTOR) |
> + BIT(NP_VECTOR) | BIT(SS_VECTOR) | BIT(GP_VECTOR) |
> + BIT(PF_VECTOR) | BIT(AC_VECTOR);
> +
> + return protected_mode && ((1U << vector) & exception_has_error_code);
This implies that exceptions only have an error code in protected mode.
I think that moving protected_mode check to the condition in
check_vmentry_prereqs would be better.
I can change that while applying if you agree,
thanks.
