On Mon, Dec 07, 2020, Krish Sadhukhan wrote:
> According to sections "Canonicalization and Consistency Checks" and "Event
> Injection" in APM vol 2
>
> VMRUN exits with VMEXIT_INVALID error code if either:
> - Reserved values of TYPE have been specified, or
> - TYPE = 3 (exception) has been specified with a vector that does not
> correspond to an exception (this includes vector 2, which is an NMI,
> not an exception).
>
> Signed-off-by: Krish Sadhukhan <krish.sadhukhan@xxxxxxxxxx>
> ---
> arch/x86/include/asm/svm.h | 4 ++++
> arch/x86/kvm/svm/nested.c | 14 ++++++++++++++
> 2 files changed, 18 insertions(+)
>
> diff --git a/arch/x86/include/asm/svm.h b/arch/x86/include/asm/svm.h
> index 71d630bb5e08..d676f140cd19 100644
> --- a/arch/x86/include/asm/svm.h
> +++ b/arch/x86/include/asm/svm.h
> @@ -341,9 +341,13 @@ struct vmcb {
> #define SVM_EVTINJ_TYPE_MASK (7 << SVM_EVTINJ_TYPE_SHIFT)
>
> #define SVM_EVTINJ_TYPE_INTR (0 << SVM_EVTINJ_TYPE_SHIFT)
> +#define SVM_EVTINJ_TYPE_RESV1 (1 << SVM_EVTINJ_TYPE_SHIFT)
> #define SVM_EVTINJ_TYPE_NMI (2 << SVM_EVTINJ_TYPE_SHIFT)
> #define SVM_EVTINJ_TYPE_EXEPT (3 << SVM_EVTINJ_TYPE_SHIFT)
> #define SVM_EVTINJ_TYPE_SOFT (4 << SVM_EVTINJ_TYPE_SHIFT)
> +#define SVM_EVTINJ_TYPE_RESV5 (5 << SVM_EVTINJ_TYPE_SHIFT)
> +#define SVM_EVTINJ_TYPE_RESV6 (6 << SVM_EVTINJ_TYPE_SHIFT)
> +#define SVM_EVTINJ_TYPE_RESV7 (7 << SVM_EVTINJ_TYPE_SHIFT)
>
> #define SVM_EVTINJ_VALID (1 << 31)
> #define SVM_EVTINJ_VALID_ERR (1 << 11)
> diff --git a/arch/x86/kvm/svm/nested.c b/arch/x86/kvm/svm/nested.c
> index 9e4c226dbf7d..fa51231c1f24 100644
> --- a/arch/x86/kvm/svm/nested.c
> +++ b/arch/x86/kvm/svm/nested.c
> @@ -212,6 +212,9 @@ static bool svm_get_nested_state_pages(struct kvm_vcpu *vcpu)
>
> static bool nested_vmcb_check_controls(struct vmcb_control_area *control)
> {
> + u8 type, vector;
> + bool valid;
> +
> if ((vmcb_is_intercept(control, INTERCEPT_VMRUN)) == 0)
> return false;
>
> @@ -222,6 +225,17 @@ static bool nested_vmcb_check_controls(struct vmcb_control_area *control)
> !npt_enabled)
> return false;
>
> + valid = control->event_inj & SVM_EVTINJ_VALID;
> + type = control->event_inj & SVM_EVTINJ_TYPE_MASK;
The mask is shifted by 8, which means type is guaranteed to be 0 since the value
will be truncated when casting to a u8. I'm somewhat surprised neither
checkpatch nor checkpatch warns. The types are also shifted, so the easiest
thing is probably to store it as a u32, same as event_inj. I suspect your test
passes because type==0 is INTR and the test always injects #DE, which is likely
an illegal vector.
> + if (valid && (type == SVM_EVTINJ_TYPE_RESV1 ||
> + type >= SVM_EVTINJ_TYPE_RESV5))
> + return false;
> +
> + vector = control->event_inj & SVM_EVTINJ_VEC_MASK;
> + if (valid && (type == SVM_EVTINJ_TYPE_EXEPT))
> + if (vector == NMI_VECTOR || vector > 31)
Preferred style is to combine these into a single statement.
> + return false;
> +
> return true;
> }
>
> --
> 2.27.0
>
