[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.