mirrors/crosvm
1
0
Fork 0
mirror of https://chromium.googlesource.com/crosvm/crosvm synced 2024-11-24 12:34:31 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions

enable_raw_capability and kvm_enable_cap are unsafe

Browse source 
The args may be interpreted as pointers for some capabilities, so the
caller must ensure that any such pointers are allocated appropriately.

BUG=b:181564686
TEST=cargo test

Change-Id: I244f4d9417e588a6be5681f4718bb9ad7b262c3e
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/crosvm/+/2726709
Auto-Submit: Andrew Walbran <qwandor@google.com>
Commit-Queue: Dylan Reid <dgreid@chromium.org>
Tested-by: kokoro <noreply+kokoro@google.com>
Reviewed-by: Dylan Reid <dgreid@chromium.org>
This commit is contained in:
Andrew Walbran 2021-03-01 17:52:47 +00:00 committed by Commit Bot
parent 0a91c96437
commit ce10855e91
5 changed files with 33 additions and 19 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

17
hypervisor/src/kvm/mod.rs
View file

@ -386,6 +386,11 @@ impl KvmVm {
// Currently only used on aarch64, but works on any architecture.
#[allow(dead_code)]
/// Enables a KVM-specific capability for this VM, with the given arguments.
///
/// # Safety
/// This function is marked as unsafe because `args` may be interpreted as pointers for some
/// capabilities. The caller must ensure that any pointers passed in the `args` array are
/// allocated as the kernel expects, and that mutable pointers are owned.
unsafe fn enable_raw_capability(
&self,
capability: KvmCap,
@ -398,8 +403,8 @@ impl KvmVm {
flags,
..Default::default()
};
// Safe because we allocated the struct and we know the kernel will read
// exactly the size of the struct.
// Safe because we allocated the struct and we know the kernel will read exactly the size of
// the struct, and because we assume the caller has allocated the args appropriately.
let ret = ioctl_with_ref(self, KVM_ENABLE_CAP(), &kvm_cap);
if ret == 0 {
Ok(())
@ -842,15 +847,15 @@ impl Vcpu for KvmVcpu {
}
}
fn enable_raw_capability(&self, cap: u32, args: &[u64; 4]) -> Result<()> {
unsafe fn enable_raw_capability(&self, cap: u32, args: &[u64; 4]) -> Result<()> {
let kvm_cap = kvm_enable_cap {
cap,
args: *args,
..Default::default()
};
// Safe because we allocated the struct and we know the kernel will read
// exactly the size of the struct.
let ret = unsafe { ioctl_with_ref(self, KVM_ENABLE_CAP(), &kvm_cap) };
// Safe because we allocated the struct and we know the kernel will read exactly the size of
// the struct, and because we assume the caller has allocated the args appropriately.
let ret = ioctl_with_ref(self, KVM_ENABLE_CAP(), &kvm_cap);
if ret == 0 {
Ok(())
} else {

3
hypervisor/src/kvm/x86_64.rs
View file

@ -1479,8 +1479,7 @@ mod tests {
let vm = KvmVm::new(&kvm, gm).unwrap();
vm.create_irq_chip().unwrap();
let vcpu = vm.create_vcpu(0).unwrap();
vcpu.enable_raw_capability(kvm_sys::KVM_CAP_HYPERV_SYNIC, &[0; 4])
.unwrap();
unsafe { vcpu.enable_raw_capability(kvm_sys::KVM_CAP_HYPERV_SYNIC, &[0; 4]) }.unwrap();
}
#[test]

7
hypervisor/src/lib.rs
View file

@ -270,7 +270,12 @@ pub trait Vcpu: downcast_rs::DowncastSync {
/// Enables a hypervisor-specific extension on this Vcpu. `cap` is a constant defined by the
/// hypervisor API (e.g., kvm.h). `args` are the arguments for enabling the feature, if any.
fn enable_raw_capability(&self, cap: u32, args: &[u64; 4]) -> Result<()>;
///
/// # Safety
/// This function is marked as unsafe because `args` may be interpreted as pointers for some
/// capabilities. The caller must ensure that any pointers passed in the `args` array are
/// allocated as the kernel expects, and that mutable pointers are owned.
unsafe fn enable_raw_capability(&self, cap: u32, args: &[u64; 4]) -> Result<()>;
}
downcast_rs::impl_downcast!(sync Vcpu);

22
kvm/src/lib.rs
View file

@ -834,10 +834,12 @@ impl Vm {
/// Enable the specified capability.
/// See documentation for KVM_ENABLE_CAP.
pub fn kvm_enable_cap(&self, cap: &kvm_enable_cap) -> Result<()> {
// safe becuase we allocated the struct and we know the kernel will read
// exactly the size of the struct
let ret = unsafe { ioctl_with_ref(self, KVM_ENABLE_CAP(), cap) };
/// This function is marked as unsafe because `cap` may contain values which are interpreted as
/// pointers by the kernel.
pub unsafe fn kvm_enable_cap(&self, cap: &kvm_enable_cap) -> Result<()> {
// Safe because we allocated the struct and we know the kernel will read exactly the size of
// the struct.
let ret = ioctl_with_ref(self, KVM_ENABLE_CAP(), cap);
if ret < 0 {
errno_result()
} else {
@ -1403,10 +1405,12 @@ impl Vcpu {
/// Enable the specified capability.
/// See documentation for KVM_ENABLE_CAP.
pub fn kvm_enable_cap(&self, cap: &kvm_enable_cap) -> Result<()> {
// safe becuase we allocated the struct and we know the kernel will read
// exactly the size of the struct
let ret = unsafe { ioctl_with_ref(self, KVM_ENABLE_CAP(), cap) };
/// This function is marked as unsafe because `cap` may contain values which are interpreted as
/// pointers by the kernel.
pub unsafe fn kvm_enable_cap(&self, cap: &kvm_enable_cap) -> Result<()> {
// Safe because we allocated the struct and we know the kernel will read exactly the size of
// the struct.
let ret = ioctl_with_ref(self, KVM_ENABLE_CAP(), cap);
if ret < 0 {
return errno_result();
}
@ -2081,7 +2085,7 @@ mod tests {
let vcpu = Vcpu::new(0, &kvm, &vm).unwrap();
let mut cap: kvm_enable_cap = Default::default();
cap.cap = kvm_sys::KVM_CAP_HYPERV_SYNIC;
vcpu.kvm_enable_cap(&cap).unwrap();
unsafe { vcpu.kvm_enable_cap(&cap) }.unwrap();
}
#[test]

3
src/plugin/vcpu.rs
View file

@ -729,7 +729,8 @@ impl PluginVcpu {
} else {
let mut cap: kvm_enable_cap = Default::default();
cap.cap = capability;
vcpu.kvm_enable_cap(&cap)
// Safe because the allowed capabilities don't take pointer arguments.
unsafe { vcpu.kvm_enable_cap(&cap) }
}
} else if request.has_shutdown() {
return Err(SysError::new(EPIPE));