From dadb7625ea382b0372ec8be9fcae8f0aa3f0b6fd Mon Sep 17 00:00:00 2001 From: Zach Reizner Date: Tue, 6 Feb 2018 22:28:36 -0800 Subject: [PATCH] allow plugin to query for KVM extensions The guest may need to check for KVM extensions before blindly using them. TEST=cargo test --features plugin; cargo test -p kvm; ./build_test BUG=chromium:800626 Change-Id: If87b928753cd71adeabac4fc7732c3fce7265834 Reviewed-on: https://chromium-review.googlesource.com/906008 Commit-Ready: Zach Reizner Tested-by: Zach Reizner Reviewed-by: Dylan Reid --- Cargo.lock | 10 +++--- crosvm_plugin/Cargo.toml | 2 +- crosvm_plugin/crosvm.h | 8 ++++- crosvm_plugin/src/lib.rs | 25 +++++++++++++++ kvm/src/lib.rs | 22 +++++++++++++ plugin_proto/Cargo.toml | 2 +- plugin_proto/protos/plugin.proto | 37 +++++++++++++--------- src/plugin/process.rs | 9 ++++++ tests/plugin_extensions.c | 53 ++++++++++++++++++++++++++++++++ tests/plugins.rs | 5 +++ 10 files changed, 151 insertions(+), 22 deletions(-) create mode 100644 tests/plugin_extensions.c diff --git a/Cargo.lock b/Cargo.lock index b2bc83bb5d..7be1203bb0 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -18,7 +18,7 @@ name = "crosvm" version = "0.1.0" dependencies = [ "byteorder 1.1.0 (registry+https://github.com/rust-lang/crates.io-index)", - "crosvm_plugin 0.9.0", + "crosvm_plugin 0.10.0", "data_model 0.1.0", "devices 0.1.0", "io_jail 0.1.0", @@ -28,7 +28,7 @@ dependencies = [ "kvm_sys 0.1.0", "libc 0.2.34 (registry+https://github.com/rust-lang/crates.io-index)", "net_util 0.1.0", - "plugin_proto 0.9.0", + "plugin_proto 0.10.0", "protobuf 1.4.3 (registry+https://github.com/rust-lang/crates.io-index)", "qcow 0.1.0", "qcow_utils 0.1.0", @@ -41,12 +41,12 @@ dependencies = [ [[package]] name = "crosvm_plugin" -version = "0.9.0" +version = "0.10.0" dependencies = [ "kvm 0.1.0", "kvm_sys 0.1.0", "libc 0.2.34 (registry+https://github.com/rust-lang/crates.io-index)", - "plugin_proto 0.9.0", + "plugin_proto 0.10.0", "protobuf 1.4.3 (registry+https://github.com/rust-lang/crates.io-index)", "sys_util 0.1.0", ] @@ -160,7 +160,7 @@ dependencies = [ [[package]] name = "plugin_proto" -version = "0.9.0" +version = "0.10.0" dependencies = [ "gcc 0.3.54 (registry+https://github.com/rust-lang/crates.io-index)", "protobuf 1.4.3 (registry+https://github.com/rust-lang/crates.io-index)", diff --git a/crosvm_plugin/Cargo.toml b/crosvm_plugin/Cargo.toml index f6aeb2459f..4020f3a375 100644 --- a/crosvm_plugin/Cargo.toml +++ b/crosvm_plugin/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "crosvm_plugin" -version = "0.9.0" +version = "0.10.0" authors = ["The Chromium OS Authors"] [lib] diff --git a/crosvm_plugin/crosvm.h b/crosvm_plugin/crosvm.h index 1bdcec690a..454ba34c33 100644 --- a/crosvm_plugin/crosvm.h +++ b/crosvm_plugin/crosvm.h @@ -47,7 +47,7 @@ extern "C" { * do not indicate anything about what version of crosvm is running. */ #define CROSVM_API_MAJOR 0 -#define CROSVM_API_MINOR 9 +#define CROSVM_API_MINOR 10 #define CROSVM_API_PATCH 0 enum crosvm_address_space { @@ -108,6 +108,12 @@ int crosvm_destroy_connection(struct crosvm**); */ int crosvm_get_shutdown_eventfd(struct crosvm*); +/* + * Gets a bool indicating if a KVM_CAP_* enum is supported on this VM + */ +int crosvm_check_extension(struct crosvm*, uint32_t __extension, + bool *__has_extension); + /* * Registers a range in the given address space that, when accessed, will block * and wait for a crosvm_vcpu_resume call. diff --git a/crosvm_plugin/src/lib.rs b/crosvm_plugin/src/lib.rs index 102d1291d2..bfb928734d 100644 --- a/crosvm_plugin/src/lib.rs +++ b/crosvm_plugin/src/lib.rs @@ -233,6 +233,16 @@ impl crosvm { } } + fn check_extension(&mut self, extension: u32) -> result::Result { + let mut r = MainRequest::new(); + r.mut_check_extension().extension = extension; + let (response, _) = self.main_transaction(&r, &[])?; + if !response.has_check_extension() { + return Err(-EPROTO); + } + Ok(response.get_check_extension().has_extension) + } + fn reserve_range(&mut self, space: u32, start: u64, length: u64) -> result::Result<(), c_int> { let mut r = MainRequest::new(); { @@ -809,6 +819,21 @@ pub unsafe extern "C" fn crosvm_get_shutdown_eventfd(self_: *mut crosvm) -> c_in } } +#[no_mangle] +pub unsafe extern "C" fn crosvm_check_extension(self_: *mut crosvm, + extension: u32, + has_extension: *mut bool) + -> c_int { + let self_ = &mut (*self_); + match self_.check_extension(extension) { + Ok(supported) => { + *has_extension = supported; + 0 + } + Err(e) => e, + } +} + #[no_mangle] pub unsafe extern "C" fn crosvm_reserve_range(self_: *mut crosvm, space: u32, diff --git a/kvm/src/lib.rs b/kvm/src/lib.rs index 1c1ce2eb26..7eb290ef28 100644 --- a/kvm/src/lib.rs +++ b/kvm/src/lib.rs @@ -223,6 +223,18 @@ impl Vm { } } + /// Checks if a particular `Cap` is available. + /// + /// This is distinct from the `Kvm` version of this method because the some extensions depend on + /// the particular `Vm` existence. This method is encouraged by the kernel because it more + /// accurately reflects the usable capabilities. + pub fn check_extension(&self, c: Cap) -> bool { + // Safe because we know that our file is a KVM fd and that the extension is one of the ones + // defined by kernel. + unsafe { ioctl_with_val(self, KVM_CHECK_EXTENSION(), c as c_ulong) == 1 } + } + + /// Inserts the given `MemoryMapping` into the VM's address space at `guest_addr`. /// /// The slot that was assigned the device memory mapping is returned on success. The slot can be @@ -1022,6 +1034,16 @@ mod tests { assert!(!kvm.check_extension(Cap::S390UserSigp)); } + #[test] + fn check_vm_extension() { + let kvm = Kvm::new().unwrap(); + let gm = GuestMemory::new(&vec![(GuestAddress(0), 0x1000)]).unwrap(); + let vm = Vm::new(&kvm, gm).unwrap(); + assert!(vm.check_extension(Cap::UserMemory)); + // I assume nobody is testing this on s390 + assert!(!vm.check_extension(Cap::S390UserSigp)); + } + #[test] fn add_memory() { let kvm = Kvm::new().unwrap(); diff --git a/plugin_proto/Cargo.toml b/plugin_proto/Cargo.toml index 0e5ead4d6b..cb6c7fd981 100644 --- a/plugin_proto/Cargo.toml +++ b/plugin_proto/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "plugin_proto" -version = "0.9.0" +version = "0.10.0" authors = ["The Chromium OS Authors"] build = "build.rs" diff --git a/plugin_proto/protos/plugin.proto b/plugin_proto/protos/plugin.proto index 22f7be6b10..194b315ce9 100644 --- a/plugin_proto/protos/plugin.proto +++ b/plugin_proto/protos/plugin.proto @@ -68,6 +68,10 @@ message MainRequest { message GetShutdownEventfd {} + message CheckExtension { + uint32 extension = 1; + } + message ReserveRange { AddressSpace space = 1; uint64 start = 2; @@ -126,13 +130,14 @@ message MainRequest { Destroy destroy = 2; NewConnection new_connection = 3; GetShutdownEventfd get_shutdown_eventfd = 4; - ReserveRange reserve_range = 5; - SetIrq set_irq = 6; - SetIrqRouting set_irq_routing = 7; - SetIdentityMapAddr set_identity_map_addr = 8; - PauseVcpus pause_vcpus = 9; - GetVcpus get_vcpus = 10; - Start start = 11; + CheckExtension check_extension = 5; + ReserveRange reserve_range = 6; + SetIrq set_irq = 7; + SetIrqRouting set_irq_routing = 8; + SetIdentityMapAddr set_identity_map_addr = 9; + PauseVcpus pause_vcpus = 10; + GetVcpus get_vcpus = 11; + Start start = 12; // Method for a Memory type object for retrieving the dirty bitmap. Only valid if the memory // object was created with dirty_log set. MemoryDirtyLog dirty_log = 101; @@ -148,6 +153,9 @@ message MainResponse { // requests and responses independent of the other sockets. message NewConnection {} message GetShutdownEventfd {} + message CheckExtension { + bool has_extension = 1; + } message ReserveRange {} message SetIrq {} message SetIrqRouting {} @@ -170,13 +178,14 @@ message MainResponse { Destroy destroy = 3; NewConnection new_connection = 4; GetShutdownEventfd get_shutdown_eventfd = 5; - ReserveRange reserve_range = 6; - SetIrq set_irq = 7; - SetIrqRouting set_irq_routing = 8; - SetIdentityMapAddr set_identity_map_addr = 9; - PauseVcpus pause_vcpus = 10; - GetVcpus get_vcpus = 11; - Start start = 12; + CheckExtension check_extension = 6; + ReserveRange reserve_range = 7; + SetIrq set_irq = 8; + SetIrqRouting set_irq_routing = 9; + SetIdentityMapAddr set_identity_map_addr = 10; + PauseVcpus pause_vcpus = 11; + GetVcpus get_vcpus = 12; + Start start = 13; MemoryDirtyLog dirty_log = 101; } } diff --git a/src/plugin/process.rs b/src/plugin/process.rs index 38acf6cc11..99bdc110c7 100644 --- a/src/plugin/process.rs +++ b/src/plugin/process.rs @@ -5,6 +5,7 @@ use std::collections::hash_map::{HashMap, Entry, VacantEntry}; use std::env::set_var; use std::fs::File; +use std::mem::transmute; use std::net::Shutdown; use std::os::unix::io::{AsRawFd, RawFd}; use std::os::unix::net::UnixDatagram; @@ -483,6 +484,14 @@ impl Process { response.mut_get_shutdown_eventfd(); response_fds.push(self.kill_evt.as_raw_fd()); Ok(()) + } else if request.has_check_extension() { + // Safe because the Cap enum is not read by the check_extension method. In that method, + // cap is cast back to an integer and fed to an ioctl. If the extension name is actually + // invalid, the kernel will safely reject the extension under the assumption that the + // capability is legitimately unsupported. + let cap = unsafe { transmute(request.get_check_extension().extension) }; + response.mut_check_extension().has_extension = vm.check_extension(cap); + Ok(()) } else if request.has_reserve_range() { response.mut_reserve_range(); self.handle_reserve_range(request.get_reserve_range()) diff --git a/tests/plugin_extensions.c b/tests/plugin_extensions.c new file mode 100644 index 0000000000..4e3ea87624 --- /dev/null +++ b/tests/plugin_extensions.c @@ -0,0 +1,53 @@ +/* + * Copyright 2018 The Chromium OS Authors. All rights reserved. + * Use of this source code is governed by a BSD-style license that can be + * found in the LICENSE file. + */ + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include "crosvm.h" + +int main(int argc, char** argv) { + struct crosvm *crosvm; + int ret = crosvm_connect(&crosvm); + if (ret) { + fprintf(stderr, "failed to connect to crosvm: %d\n", ret); + return 1; + } + + bool supported; + ret = crosvm_check_extension(crosvm, KVM_CAP_IRQCHIP, &supported); + if (ret) { + fprintf(stderr, "failed to check for KVM extension: %d\n", ret); + return 1; + } + if (!supported) { + fprintf(stderr, "expected KVM extension to be supported\n"); + return 1; + } + + // Assume s390 extensions aren't supported because we shouldn't be running on one. + ret = crosvm_check_extension(crosvm, KVM_CAP_S390_PSW, &supported); + if (ret) { + fprintf(stderr, "failed to check for KVM extension: %d\n", ret); + return 1; + } + if (supported) { + fprintf(stderr, "unexpected KVM extension is supported\n"); + return 1; + } + + return 0; +} diff --git a/tests/plugins.rs b/tests/plugins.rs index 54c4b7de14..67383fc5f2 100644 --- a/tests/plugins.rs +++ b/tests/plugins.rs @@ -225,6 +225,11 @@ fn test_irqfd() { test_plugin(include_str!("plugin_irqfd.c")); } +#[test] +fn test_extensions() { + test_plugin(include_str!("plugin_extensions.c")); +} + #[test] fn test_debugregs() { let mini_plugin = MiniPlugin {