On Mon, Nov 30, 2020, Krish Sadhukhan wrote:
> diff --git a/arch/x86/kvm/svm/nested.c b/arch/x86/kvm/svm/nested.c
> index 05d564c8d034..2599b32ea7be 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)
> {
> + u32 type, vector;
Making vector a u8 would be more intuitive, since its mask is 0xff.
> + 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;
> + if (valid && type == SVM_EVTINJ_TYPE_RESV1 ||
> + type >= SVM_EVTINJ_TYPE_RESV5)
You were a little too aggressive in removing parantheses. :-) This breaks the
build with CONFIG_KVM_WERROR=y due to not wrapping the chain of ANDs that are
separated by ORs.
/arch/x86/kvm/svm/nested.c: In function ‘nested_vmcb_check_controls’:
/arch/x86/kvm/svm/nested.c:230:12: error: suggest parentheses around ‘&&’ within ‘||’ [-Werror=parentheses]
230 | if (valid && type == SVM_EVTINJ_TYPE_RESV1 ||
/arch/x86/kvm/svm/nested.c:235:45: error: suggest parentheses around ‘&&’ within ‘||’ [-Werror=parentheses]
235 | if (valid && type == SVM_EVTINJ_TYPE_EXEPT &&
> + return false;
> +
> + vector = control->event_inj & SVM_EVTINJ_VEC_MASK;
> + if (valid && type == SVM_EVTINJ_TYPE_EXEPT &&
> + vector == NMI_VECTOR || (vector > 31 && vector < 256))
I'm pretty sure this is straight up wrong due to removing paranetheses. I
assume the check is supposed to reject valid exceptions with (vector == 2 ||
vector > 31). As is, it rejects valid exceptions with vector ==2, OR any vector
greater than 31. If for some reason this should reject any vector > 31, it'd be
better to through that into a separate if statement.
Also, the "< 256" check is pointless, it will always be true since vector is
an 8-bit value.
> + return false;
> +
> return true;
> }
>
> --
> 2.27.0
>
