* [PATCH 0/3] KVM: nVMX: tighten some KVM_SET_NESTED_STATE validity checks
@ 2020-07-27 13:39 Paolo Bonzini
2020-07-27 13:39 ` [PATCH 1/3] selftests: kvm: do not set guest mode flag Paolo Bonzini
` (2 more replies)
0 siblings, 3 replies; 4+ messages in thread
From: Paolo Bonzini @ 2020-07-27 13:39 UTC (permalink / raw)
To: linux-kernel, kvm; +Cc: vkuznets, aaronlewis
Patch 1 is a fix to the tests so that we can cover the new cases properly.
Patches 2-3 check for more cases of invalid nested state data.
Paolo
Paolo Bonzini (3):
selftests: kvm: do not set guest mode flag
KVM: nVMX: check for required but missing VMCS12 in
KVM_SET_NESTED_STATE
KVM: nVMX: check for invalid hdr.vmx.flags
arch/x86/kvm/vmx/nested.c | 16 +++++--
arch/x86/kvm/vmx/nested.h | 5 +++
.../kvm/x86_64/vmx_set_nested_state_test.c | 42 +++++++++++++++----
3 files changed, 51 insertions(+), 12 deletions(-)
--
2.26.2
^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH 1/3] selftests: kvm: do not set guest mode flag
2020-07-27 13:39 [PATCH 0/3] KVM: nVMX: tighten some KVM_SET_NESTED_STATE validity checks Paolo Bonzini
@ 2020-07-27 13:39 ` Paolo Bonzini
2020-07-27 13:39 ` [PATCH 2/3] KVM: nVMX: check for required but missing VMCS12 in KVM_SET_NESTED_STATE Paolo Bonzini
2020-07-27 13:39 ` [PATCH 3/3] KVM: nVMX: check for invalid hdr.vmx.flags Paolo Bonzini
2 siblings, 0 replies; 4+ messages in thread
From: Paolo Bonzini @ 2020-07-27 13:39 UTC (permalink / raw)
To: linux-kernel, kvm; +Cc: vkuznets, aaronlewis
Setting KVM_STATE_NESTED_GUEST_MODE enables various consistency checks
on VMCS12 and therefore causes KVM_SET_NESTED_STATE to fail spuriously
with -EINVAL. Do not set the flag so that we're sure to cover the
conditions included by the test, and cover the case where VMCS12 is
set and KVM_SET_NESTED_STATE is called with invalid VMCS12 contents.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
.../kvm/x86_64/vmx_set_nested_state_test.c | 17 +++++++++--------
1 file changed, 9 insertions(+), 8 deletions(-)
diff --git a/tools/testing/selftests/kvm/x86_64/vmx_set_nested_state_test.c b/tools/testing/selftests/kvm/x86_64/vmx_set_nested_state_test.c
index 54cdefdfb49d..d14a34f1b018 100644
--- a/tools/testing/selftests/kvm/x86_64/vmx_set_nested_state_test.c
+++ b/tools/testing/selftests/kvm/x86_64/vmx_set_nested_state_test.c
@@ -76,10 +76,8 @@ void set_default_state(struct kvm_nested_state *state)
void set_default_vmx_state(struct kvm_nested_state *state, int size)
{
memset(state, 0, size);
- state->flags = KVM_STATE_NESTED_GUEST_MODE |
- KVM_STATE_NESTED_RUN_PENDING;
if (have_evmcs)
- state->flags |= KVM_STATE_NESTED_EVMCS;
+ state->flags = KVM_STATE_NESTED_EVMCS;
state->format = 0;
state->size = size;
state->hdr.vmx.vmxon_pa = 0x1000;
@@ -190,17 +188,20 @@ void test_vmx_nested_state(struct kvm_vm *vm)
state->size = sizeof(*state);
test_nested_state(vm, state);
+ /*
+ * KVM_SET_NESTED_STATE succeeds with invalid VMCS
+ * contents but L2 not running.
+ */
+ set_default_vmx_state(state, state_sz);
+ state->flags = 0;
+ test_nested_state(vm, state);
+
/* vmxon_pa cannot be the same address as vmcs_pa. */
set_default_vmx_state(state, state_sz);
state->hdr.vmx.vmxon_pa = 0;
state->hdr.vmx.vmcs12_pa = 0;
test_nested_state_expect_einval(vm, state);
- /* The revision id for vmcs12 must be VMCS12_REVISION. */
- set_default_vmx_state(state, state_sz);
- set_revision_id_for_vmcs12(state, 0);
- test_nested_state_expect_einval(vm, state);
-
/*
* Test that if we leave nesting the state reflects that when we get
* it again.
--
2.26.2
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH 2/3] KVM: nVMX: check for required but missing VMCS12 in KVM_SET_NESTED_STATE
2020-07-27 13:39 [PATCH 0/3] KVM: nVMX: tighten some KVM_SET_NESTED_STATE validity checks Paolo Bonzini
2020-07-27 13:39 ` [PATCH 1/3] selftests: kvm: do not set guest mode flag Paolo Bonzini
@ 2020-07-27 13:39 ` Paolo Bonzini
2020-07-27 13:39 ` [PATCH 3/3] KVM: nVMX: check for invalid hdr.vmx.flags Paolo Bonzini
2 siblings, 0 replies; 4+ messages in thread
From: Paolo Bonzini @ 2020-07-27 13:39 UTC (permalink / raw)
To: linux-kernel, kvm; +Cc: vkuznets, aaronlewis
A missing VMCS12 was not causing -EINVAL (it was just read with
copy_from_user, so it is not a security issue, but it is still
wrong). Test for VMCS12 validity and reject the nested state
if a VMCS12 is required but not present.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
arch/x86/kvm/vmx/nested.c | 13 ++++++++++---
arch/x86/kvm/vmx/nested.h | 5 +++++
.../kvm/x86_64/vmx_set_nested_state_test.c | 12 +++++++++++-
3 files changed, 26 insertions(+), 4 deletions(-)
diff --git a/arch/x86/kvm/vmx/nested.c b/arch/x86/kvm/vmx/nested.c
index d4a4cec034d0..6a0e32a7418c 100644
--- a/arch/x86/kvm/vmx/nested.c
+++ b/arch/x86/kvm/vmx/nested.c
@@ -6108,9 +6108,16 @@ static int vmx_set_nested_state(struct kvm_vcpu *vcpu,
if (ret)
return ret;
- /* Empty 'VMXON' state is permitted */
- if (kvm_state->size < sizeof(*kvm_state) + sizeof(*vmcs12))
- return 0;
+ /* Empty VMX data is permitted if no VMCS loaded */
+ if (kvm_state->size < sizeof(*kvm_state) + sizeof(*vmcs12)) {
+ /* See vmx_has_valid_vmcs12. */
+ if ((kvm_state->flags & KVM_STATE_NESTED_GUEST_MODE) ||
+ (kvm_state->flags & KVM_STATE_NESTED_EVMCS) ||
+ (kvm_state->hdr.vmx.vmcs12_pa != -1ull))
+ return -EINVAL;
+ else
+ return 0;
+ }
if (kvm_state->hdr.vmx.vmcs12_pa != -1ull) {
if (kvm_state->hdr.vmx.vmcs12_pa == kvm_state->hdr.vmx.vmxon_pa ||
diff --git a/arch/x86/kvm/vmx/nested.h b/arch/x86/kvm/vmx/nested.h
index 758bccc26cf9..197148d76b8f 100644
--- a/arch/x86/kvm/vmx/nested.h
+++ b/arch/x86/kvm/vmx/nested.h
@@ -47,6 +47,11 @@ static inline struct vmcs12 *get_shadow_vmcs12(struct kvm_vcpu *vcpu)
return to_vmx(vcpu)->nested.cached_shadow_vmcs12;
}
+/*
+ * Note: the same condition is checked against the state provided by userspace
+ * in vmx_set_nested_state; if it is satisfied, the nested state must include
+ * the VMCS12.
+ */
static inline int vmx_has_valid_vmcs12(struct kvm_vcpu *vcpu)
{
struct vcpu_vmx *vmx = to_vmx(vcpu);
diff --git a/tools/testing/selftests/kvm/x86_64/vmx_set_nested_state_test.c b/tools/testing/selftests/kvm/x86_64/vmx_set_nested_state_test.c
index d14a34f1b018..94f28a657569 100644
--- a/tools/testing/selftests/kvm/x86_64/vmx_set_nested_state_test.c
+++ b/tools/testing/selftests/kvm/x86_64/vmx_set_nested_state_test.c
@@ -183,9 +183,19 @@ void test_vmx_nested_state(struct kvm_vm *vm)
state->hdr.vmx.smm.flags = KVM_STATE_NESTED_SMM_GUEST_MODE;
test_nested_state_expect_einval(vm, state);
- /* Size must be large enough to fit kvm_nested_state and vmcs12. */
+ /*
+ * Size must be large enough to fit kvm_nested_state and vmcs12
+ * if VMCS12 physical address is set
+ */
+ set_default_vmx_state(state, state_sz);
+ state->size = sizeof(*state);
+ state->flags = 0;
+ test_nested_state_expect_einval(vm, state);
+
set_default_vmx_state(state, state_sz);
state->size = sizeof(*state);
+ state->flags = 0;
+ state->hdr.vmx.vmcs12_pa = -1;
test_nested_state(vm, state);
/*
--
2.26.2
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH 3/3] KVM: nVMX: check for invalid hdr.vmx.flags
2020-07-27 13:39 [PATCH 0/3] KVM: nVMX: tighten some KVM_SET_NESTED_STATE validity checks Paolo Bonzini
2020-07-27 13:39 ` [PATCH 1/3] selftests: kvm: do not set guest mode flag Paolo Bonzini
2020-07-27 13:39 ` [PATCH 2/3] KVM: nVMX: check for required but missing VMCS12 in KVM_SET_NESTED_STATE Paolo Bonzini
@ 2020-07-27 13:39 ` Paolo Bonzini
2 siblings, 0 replies; 4+ messages in thread
From: Paolo Bonzini @ 2020-07-27 13:39 UTC (permalink / raw)
To: linux-kernel, kvm; +Cc: vkuznets, aaronlewis
hdr.vmx.flags is meant for future extensions to the ABI, rejecting
invalid flags is necessary to avoid broken half-loads of the
nVMX state.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
arch/x86/kvm/vmx/nested.c | 3 +++
.../kvm/x86_64/vmx_set_nested_state_test.c | 13 +++++++++++++
2 files changed, 16 insertions(+)
diff --git a/arch/x86/kvm/vmx/nested.c b/arch/x86/kvm/vmx/nested.c
index 6a0e32a7418c..11e4df560018 100644
--- a/arch/x86/kvm/vmx/nested.c
+++ b/arch/x86/kvm/vmx/nested.c
@@ -6079,6 +6079,9 @@ static int vmx_set_nested_state(struct kvm_vcpu *vcpu,
~(KVM_STATE_NESTED_SMM_GUEST_MODE | KVM_STATE_NESTED_SMM_VMXON))
return -EINVAL;
+ if (kvm_state->hdr.vmx.flags & ~KVM_STATE_VMX_PREEMPTION_TIMER_DEADLINE)
+ return -EINVAL;
+
/*
* SMM temporarily disables VMX, so we cannot be in guest mode,
* nor can VMLAUNCH/VMRESUME be pending. Outside SMM, SMM flags
diff --git a/tools/testing/selftests/kvm/x86_64/vmx_set_nested_state_test.c b/tools/testing/selftests/kvm/x86_64/vmx_set_nested_state_test.c
index 94f28a657569..d59f3eb67c8f 100644
--- a/tools/testing/selftests/kvm/x86_64/vmx_set_nested_state_test.c
+++ b/tools/testing/selftests/kvm/x86_64/vmx_set_nested_state_test.c
@@ -146,6 +146,11 @@ void test_vmx_nested_state(struct kvm_vm *vm)
state->hdr.vmx.smm.flags = 1;
test_nested_state_expect_einval(vm, state);
+ /* Invalid flags are rejected. */
+ set_default_vmx_state(state, state_sz);
+ state->hdr.vmx.flags = ~0;
+ test_nested_state_expect_einval(vm, state);
+
/* It is invalid to have vmxon_pa == -1ull and vmcs_pa != -1ull. */
set_default_vmx_state(state, state_sz);
state->hdr.vmx.vmxon_pa = -1ull;
@@ -206,6 +211,14 @@ void test_vmx_nested_state(struct kvm_vm *vm)
state->flags = 0;
test_nested_state(vm, state);
+ /* Invalid flags are rejected, even if no VMCS loaded. */
+ set_default_vmx_state(state, state_sz);
+ state->size = sizeof(*state);
+ state->flags = 0;
+ state->hdr.vmx.vmcs12_pa = -1;
+ state->hdr.vmx.flags = ~0;
+ test_nested_state_expect_einval(vm, state);
+
/* vmxon_pa cannot be the same address as vmcs_pa. */
set_default_vmx_state(state, state_sz);
state->hdr.vmx.vmxon_pa = 0;
--
2.26.2
^ permalink raw reply related [flat|nested] 4+ messages in thread
end of thread, other threads:[~2020-07-27 13:39 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-27 13:39 [PATCH 0/3] KVM: nVMX: tighten some KVM_SET_NESTED_STATE validity checks Paolo Bonzini
2020-07-27 13:39 ` [PATCH 1/3] selftests: kvm: do not set guest mode flag Paolo Bonzini
2020-07-27 13:39 ` [PATCH 2/3] KVM: nVMX: check for required but missing VMCS12 in KVM_SET_NESTED_STATE Paolo Bonzini
2020-07-27 13:39 ` [PATCH 3/3] KVM: nVMX: check for invalid hdr.vmx.flags Paolo Bonzini
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).