[hypervisor][x86] Check CPL when handling a VMCALL
Only allow guest CPL 0 to make a hypercall. If anything other than the
guest kernel attempts a VMCALL, return NOT_PERMITTED.
Bug: b/154791063
Change-Id: I08d7f0d00df002ef90bb9be14c52697027df03bb
Reviewed-on: https://fuchsia-review.googlesource.com/c/fuchsia/+/383975
Testability-Review: Abdulla Kamar <abdulla@google.com>
Reviewed-by: Venkatesh Srinivas <venkateshs@google.com>
Commit-Queue: Abdulla Kamar <abdulla@google.com>
diff --git a/zircon/kernel/arch/x86/hypervisor/vcpu_priv.h b/zircon/kernel/arch/x86/hypervisor/vcpu_priv.h
index 10efb14..3c29d35 100644
--- a/zircon/kernel/arch/x86/hypervisor/vcpu_priv.h
+++ b/zircon/kernel/arch/x86/hypervisor/vcpu_priv.h
@@ -71,6 +71,7 @@
static const uint32_t kGuestXxAccessRightsTypeCode = 1u << 3;
// See Volume 3, Section 3.4.5.1 for valid non-system selector types.
static const uint32_t kGuestXxAccessRightsS = 1u << 4;
+static const uint32_t kGuestXxAccessRightsDplUser = 3u << 5;
static const uint32_t kGuestXxAccessRightsP = 1u << 7;
static const uint32_t kGuestXxAccessRightsL = 1u << 13;
static const uint32_t kGuestXxAccessRightsD = 1u << 14;
diff --git a/zircon/kernel/arch/x86/hypervisor/vmexit.cc b/zircon/kernel/arch/x86/hypervisor/vmexit.cc
index a8d130f..2707456 100644
--- a/zircon/kernel/arch/x86/hypervisor/vmexit.cc
+++ b/zircon/kernel/arch/x86/hypervisor/vmexit.cc
@@ -1066,14 +1066,21 @@
hypervisor::GuestPhysicalAddressSpace* gpas,
GuestState* guest_state) {
next_rip(exit_info, vmcs);
- vmcs->Invalidate();
+ const uint32_t access_rights = vmcs->Read(VmcsField32::GUEST_SS_ACCESS_RIGHTS);
+ if ((access_rights & kGuestXxAccessRightsDplUser) != 0) {
+ // We only accept a VMCALL if CPL is 0.
+ guest_state->rax = VmCallStatus::NOT_PERMITTED;
+ return ZX_OK;
+ }
+
+ vmcs->Invalidate();
VmCallInfo info(guest_state);
switch (info.type) {
case VmCallType::CLOCK_PAIRING: {
if (info.arg[1] != 0) {
dprintf(INFO, "CLOCK_PAIRING hypercall doesn't support clock type %lu\n", info.arg[1]);
- guest_state->rax = VmCallStatus::OP_NOT_SUPPORTED;
+ guest_state->rax = VmCallStatus::NOT_SUPPORTED;
break;
}
zx_status_t status = pv_clock_populate_offset(gpas, info.arg[0]);
@@ -1086,10 +1093,10 @@
break;
}
default:
- dprintf(INFO, "Unknown VMCALL(%lu) (arg0=%#lx, arg1=%#lx, arg2=%#lx, arg3=%#lx)\n",
+ dprintf(INFO, "Unknown hypercall %lu (arg0=%#lx, arg1=%#lx, arg2=%#lx, arg3=%#lx)\n",
static_cast<unsigned long>(info.type), info.arg[0], info.arg[1], info.arg[2],
info.arg[3]);
- guest_state->rax = VmCallStatus::NO_SYS;
+ guest_state->rax = VmCallStatus::UNKNOWN_HYPERCALL;
break;
}
// We never fail in case of hypercalls, we just return/propagate errors to the caller.
diff --git a/zircon/kernel/arch/x86/hypervisor/vmexit_priv.h b/zircon/kernel/arch/x86/hypervisor/vmexit_priv.h
index b8eab72..e2fdef4 100644
--- a/zircon/kernel/arch/x86/hypervisor/vmexit_priv.h
+++ b/zircon/kernel/arch/x86/hypervisor/vmexit_priv.h
@@ -233,9 +233,10 @@
enum VmCallStatus : int64_t {
OK = 0,
+ NOT_PERMITTED = -1,
FAULT = -14,
- OP_NOT_SUPPORTED = -95,
- NO_SYS = -1000,
+ NOT_SUPPORTED = -95,
+ UNKNOWN_HYPERCALL = -1000,
};
enum class VmCallType : uint64_t {
diff --git a/zircon/system/utest/hypervisor/guest.cc b/zircon/system/utest/hypervisor/guest.cc
index be935bb..bda8f3c 100644
--- a/zircon/system/utest/hypervisor/guest.cc
+++ b/zircon/system/utest/hypervisor/guest.cc
@@ -65,7 +65,8 @@
DECLARE_TEST_FUNCTION(vcpu_syscall)
DECLARE_TEST_FUNCTION(vcpu_sysenter)
DECLARE_TEST_FUNCTION(vcpu_sysenter_compat)
-DECLARE_TEST_FUNCTION(vcpu_vmcall)
+DECLARE_TEST_FUNCTION(vcpu_vmcall_invalid_number)
+DECLARE_TEST_FUNCTION(vcpu_vmcall_invalid_cpl)
DECLARE_TEST_FUNCTION(vcpu_extended_registers)
DECLARE_TEST_FUNCTION(guest_set_trap_with_io)
#endif
@@ -114,7 +115,8 @@
#ifdef __aarch64__
-static zx_status_t get_interrupt_controller_info(fuchsia::sysinfo::InterruptControllerInfoPtr* info) {
+static zx_status_t get_interrupt_controller_info(
+ fuchsia::sysinfo::InterruptControllerInfoPtr* info) {
fuchsia::sysinfo::SysInfoSyncPtr sysinfo = get_sysinfo();
zx_status_t fidl_status;
@@ -948,11 +950,11 @@
END_TEST;
}
-static bool vcpu_vmcall() {
+static bool vcpu_vmcall_invalid_number() {
BEGIN_TEST;
test_t test;
- ASSERT_TRUE(setup(&test, vcpu_vmcall_start, vcpu_vmcall_end));
+ ASSERT_TRUE(setup(&test, vcpu_vmcall_invalid_number_start, vcpu_vmcall_invalid_number_end));
if (!test.supported) {
// The hypervisor isn't supported, so don't run the test.
return true;
@@ -963,8 +965,29 @@
zx_vcpu_state_t vcpu_state;
ASSERT_EQ(test.vcpu.read_state(ZX_VCPU_STATE, &vcpu_state, sizeof(vcpu_state)), ZX_OK);
- const uint64_t kVmCallNoSys = -1000;
- EXPECT_EQ(vcpu_state.rax, kVmCallNoSys);
+ const uint64_t kUnknownHypercall = -1000;
+ EXPECT_EQ(vcpu_state.rax, kUnknownHypercall);
+
+ END_TEST;
+}
+
+static bool vcpu_vmcall_invalid_cpl() {
+ BEGIN_TEST;
+
+ test_t test;
+ ASSERT_TRUE(setup(&test, vcpu_vmcall_invalid_cpl_start, vcpu_vmcall_invalid_cpl_end));
+ if (!test.supported) {
+ // The hypervisor isn't supported, so don't run the test.
+ return true;
+ }
+
+ ASSERT_TRUE(resume_and_clean_exit(&test));
+
+ zx_vcpu_state_t vcpu_state;
+ ASSERT_EQ(test.vcpu.read_state(ZX_VCPU_STATE, &vcpu_state, sizeof(vcpu_state)), ZX_OK);
+
+ const uint64_t kNotPermitted = -1;
+ EXPECT_EQ(vcpu_state.rax, kNotPermitted);
END_TEST;
}
@@ -1099,7 +1122,8 @@
RUN_TEST(vcpu_syscall)
RUN_TEST(vcpu_sysenter)
RUN_TEST(vcpu_sysenter_compat)
-RUN_TEST(vcpu_vmcall)
+RUN_TEST(vcpu_vmcall_invalid_number)
+RUN_TEST(vcpu_vmcall_invalid_cpl)
RUN_TEST(vcpu_extended_registers)
RUN_TEST(vcpu_write_state_io_invalid_size)
RUN_TEST(guest_set_trap_with_io)
diff --git a/zircon/system/utest/hypervisor/x64.S b/zircon/system/utest/hypervisor/x64.S
index 91b06b6..2dcdf50 100644
--- a/zircon/system/utest/hypervisor/x64.S
+++ b/zircon/system/utest/hypervisor/x64.S
@@ -405,11 +405,29 @@
movq $0, (EXIT_TEST_ADDR)
FUNCTION vcpu_sysenter_compat_end
-FUNCTION vcpu_vmcall_start
+FUNCTION vcpu_vmcall_invalid_number_start
movq $(INVALID_HYPERCALL), %rax
vmcall
movq $0, (EXIT_TEST_ADDR)
-FUNCTION vcpu_vmcall_end
+FUNCTION vcpu_vmcall_invalid_number_end
+
+FUNCTION vcpu_vmcall_invalid_cpl_start
+ init_interrupt_handling vcpu_vmcall_invalid_cpl_start
+
+ xor %eax, %eax
+ mov $((USER_CODE_64_SELECTOR << 16) | CODE_64_SELECTOR), %edx
+ mov $X86_MSR_IA32_STAR, %ecx
+ wrmsr
+
+ pushfq
+ mov $ABSOLUTE_ADDR(vcpu_vmcall_invalid_cpl_start, vcpu_vmcall_invalid_cpl_ring3), %rcx
+ sysretq
+
+vcpu_vmcall_invalid_cpl_ring3:
+ movq $(INVALID_HYPERCALL), %rax
+ vmcall
+ movq $0, (EXIT_TEST_ADDR)
+FUNCTION vcpu_vmcall_invalid_cpl_end
FUNCTION vcpu_extended_registers_start
// Enable SSE instructions.
