Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

move encrypt_op{_sev} into system ioctls #259

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@
- [[#255](https://github.com/rust-vmm/kvm-ioctls/issues/255)]: Fixed a
soundness issue when accessing the `kvm_run` struct. `VcpuFd::run()` and
`VcpuFd::set_kvm_immediate_exit()` now take `&mut self` as a consequence.
- Changed `encrypt_op` and `encrypt_op_sev` to system ioctls to be in line
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you add the PR number here?

Suggested change
- Changed `encrypt_op` and `encrypt_op_sev` to system ioctls to be in line
- [[#259](https://github.com/rust-vmm/kvm-ioctls/pull/259)] Changed `encrypt_op` and `encrypt_op_sev` to system ioctls to be in line

with the KVM API

# v0.16.0

Expand Down
102 changes: 102 additions & 0 deletions src/ioctls/system.rs
Original file line number Diff line number Diff line change
Expand Up @@ -682,6 +682,97 @@ impl Kvm {
let run_mmap_size = self.get_vcpu_mmap_size()?;
Ok(new_vmfd(File::from_raw_fd(fd), run_mmap_size))
}

/// Issues platform-specific memory encryption commands to manage encrypted VMs if
/// the platform supports creating those encrypted VMs.
///
/// Currently, this ioctl is used for issuing Secure Encrypted Virtualization
/// (SEV) commands on AMD Processors.
///
/// See the documentation for `KVM_MEMORY_ENCRYPT_OP` in the
/// [KVM API doc](https://www.kernel.org/doc/Documentation/virtual/kvm/api.txt).
///
/// For SEV-specific functionality, prefer safe wrapper:
/// - [`encrypt_op_sev`](Self::encrypt_op_sev)
///
/// # Safety
///
/// This function is unsafe because there is no guarantee `T` is valid in this context, how
/// much data kernel will read from memory and where it will write data on error.
///
/// # Arguments
///
/// * `fd` - the RawFd to be operated on. (`VmFd`, `VcpuFd`, etc.)
/// * `op` - an opaque platform specific structure.
///
/// # Example
#[cfg_attr(has_sev, doc = "```rust")]
#[cfg_attr(not(has_sev), doc = "```rust,no_run")]
/// # extern crate kvm_ioctls;
/// # extern crate kvm_bindings;
/// use kvm_bindings::bindings::kvm_sev_cmd;
/// # use kvm_ioctls::Kvm;
///
/// let kvm = Kvm::new().unwrap();
/// let vm = kvm.create_vm().unwrap();
///
/// // Initialize the SEV platform context.
/// let mut init: kvm_sev_cmd = Default::default();
/// unsafe { kvm.encrypt_op(&vm, &mut init).unwrap() };
/// ```
#[cfg(any(target_arch = "x86", target_arch = "x86_64"))]
pub unsafe fn encrypt_op<T>(&self, fd: &impl AsRawFd, op: *mut T) -> Result<()> {
let ret = ioctl_with_mut_ptr(fd, KVM_MEMORY_ENCRYPT_OP(), op);
Comment on lines +724 to +725
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not familiar with AMD-SEV, but if this is a system ioctl, shouldn't it be issued on the KVM fd? e.g.

Suggested change
pub unsafe fn encrypt_op<T>(&self, fd: &impl AsRawFd, op: *mut T) -> Result<()> {
let ret = ioctl_with_mut_ptr(fd, KVM_MEMORY_ENCRYPT_OP(), op);
pub unsafe fn encrypt_op<T>(&self, op: *mut T) -> Result<()> {
let ret = ioctl_with_mut_ptr(&self.kvm, KVM_MEMORY_ENCRYPT_OP(), op);

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I believe you're right. It was brought to my attention that I was referencing stale documentation. However, the changes to the ioctl should be so that it is both a vm ioctl and a vcpu ioctl, so that requires an update on my end.

if ret == 0 {
Ok(())
} else {
Err(errno::Error::last())
}
}

/// Issue common lifecycle events of SEV guests, such as launching, running, snapshotting,
/// migrating and decommissioning via `KVM_MEMORY_ENCRYPT_OP` ioctl.
///
/// Kernel documentation states that this ioctl can be used for testing whether SEV is enabled
/// by sending `NULL`. To do that, pass [`std::ptr::null_mut`](std::ptr::null_mut) to [`encrypt_op`](Self::encrypt_op).
///
/// See the documentation for Secure Encrypted Virtualization (SEV).
///
/// # Arguments
///
/// * `fd` - the RawFd to be operated on. (`VmFd`, `VcpuFd`, etc.)
/// * `op` - SEV-specific structure. For details check the
/// [Secure Encrypted Virtualization (SEV) doc](https://www.kernel.org/doc/Documentation/virtual/kvm/amd-memory-encryption.rst).
///
/// # Example
#[cfg_attr(has_sev, doc = "```rust")]
#[cfg_attr(not(has_sev), doc = "```rust,no_run")]
/// # extern crate kvm_ioctls;
/// # extern crate kvm_bindings;
/// # use std::{os::raw::c_void, ptr::null_mut};
/// use kvm_bindings::bindings::kvm_sev_cmd;
/// # use kvm_ioctls::Kvm;
///
/// let kvm = Kvm::new().unwrap();
/// let vm = kvm.create_vm().unwrap();
///
/// // Check whether SEV is enabled, optional.
/// assert!(unsafe { kvm.encrypt_op(&vm, null_mut() as *mut c_void) }.is_ok());
///
/// // Initialize the SEV platform context.
/// let mut init: kvm_sev_cmd = Default::default();
/// kvm.encrypt_op_sev(&vm, &mut init).unwrap();
/// ```
#[cfg(any(target_arch = "x86", target_arch = "x86_64"))]
pub fn encrypt_op_sev(
&self,
fd: &impl AsRawFd,
op: &mut kvm_bindings::kvm_sev_cmd,
) -> Result<()> {
// SAFETY: Safe because we know that kernel will only read the correct amount of memory
// from our pointer and we know where it will write it (op.error).
unsafe { self.encrypt_op(fd, op) }
}
}

impl AsRawFd for Kvm {
Expand Down Expand Up @@ -979,4 +1070,15 @@ mod tests {
}
assert_eq!(faulty_kvm.create_vm().err().unwrap().errno(), badf_errno);
}

#[test]
#[cfg(any(target_arch = "x86", target_arch = "x86_64"))]
#[cfg_attr(not(has_sev), ignore)]
fn test_encrypt_op_sev() {
let kvm = Kvm::new().unwrap();
let vm = kvm.create_vm().unwrap();

let mut init: kvm_bindings::kvm_sev_cmd = Default::default();
assert!(kvm.encrypt_op_sev(&vm, &mut init).is_ok());
}
}
110 changes: 6 additions & 104 deletions src/ioctls/vm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,6 @@ use crate::ioctls::{KvmRunWrapper, Result};
use crate::kvm_ioctls::*;
use vmm_sys_util::errno;
use vmm_sys_util::eventfd::EventFd;
#[cfg(any(target_arch = "x86", target_arch = "x86_64"))]
use vmm_sys_util::ioctl::ioctl_with_mut_ptr;
use vmm_sys_util::ioctl::{ioctl, ioctl_with_mut_ref, ioctl_with_ref, ioctl_with_val};

/// An address either in programmable I/O space or in memory mapped I/O space.
Expand Down Expand Up @@ -1326,91 +1324,6 @@ impl VmFd {
self.check_extension_int(c) > 0
}

/// Issues platform-specific memory encryption commands to manage encrypted VMs if
/// the platform supports creating those encrypted VMs.
///
/// Currently, this ioctl is used for issuing Secure Encrypted Virtualization
/// (SEV) commands on AMD Processors.
///
/// See the documentation for `KVM_MEMORY_ENCRYPT_OP` in the
/// [KVM API doc](https://www.kernel.org/doc/Documentation/virtual/kvm/api.txt).
///
/// For SEV-specific functionality, prefer safe wrapper:
/// - [`encrypt_op_sev`](Self::encrypt_op_sev)
///
/// # Safety
///
/// This function is unsafe because there is no guarantee `T` is valid in this context, how
/// much data kernel will read from memory and where it will write data on error.
///
/// # Arguments
///
/// * `op` - an opaque platform specific structure.
///
/// # Example
#[cfg_attr(has_sev, doc = "```rust")]
#[cfg_attr(not(has_sev), doc = "```rust,no_run")]
/// # extern crate kvm_ioctls;
/// # extern crate kvm_bindings;
/// use kvm_bindings::bindings::kvm_sev_cmd;
/// # use kvm_ioctls::Kvm;
///
/// let kvm = Kvm::new().unwrap();
/// let vm = kvm.create_vm().unwrap();
///
/// // Initialize the SEV platform context.
/// let mut init: kvm_sev_cmd = Default::default();
/// unsafe { vm.encrypt_op(&mut init).unwrap() };
/// ```
#[cfg(any(target_arch = "x86", target_arch = "x86_64"))]
pub unsafe fn encrypt_op<T>(&self, op: *mut T) -> Result<()> {
let ret = ioctl_with_mut_ptr(self, KVM_MEMORY_ENCRYPT_OP(), op);
if ret == 0 {
Ok(())
} else {
Err(errno::Error::last())
}
}

/// Issue common lifecycle events of SEV guests, such as launching, running, snapshotting,
/// migrating and decommissioning via `KVM_MEMORY_ENCRYPT_OP` ioctl.
///
/// Kernel documentation states that this ioctl can be used for testing whether SEV is enabled
/// by sending `NULL`. To do that, pass [`std::ptr::null_mut`](std::ptr::null_mut) to [`encrypt_op`](Self::encrypt_op).
///
/// See the documentation for Secure Encrypted Virtualization (SEV).
///
/// # Arguments
///
/// * `op` - SEV-specific structure. For details check the
/// [Secure Encrypted Virtualization (SEV) doc](https://www.kernel.org/doc/Documentation/virtual/kvm/amd-memory-encryption.rst).
///
/// # Example
#[cfg_attr(has_sev, doc = "```rust")]
#[cfg_attr(not(has_sev), doc = "```rust,no_run")]
/// # extern crate kvm_ioctls;
/// # extern crate kvm_bindings;
/// # use std::{os::raw::c_void, ptr::null_mut};
/// use kvm_bindings::bindings::kvm_sev_cmd;
/// # use kvm_ioctls::Kvm;
///
/// let kvm = Kvm::new().unwrap();
/// let vm = kvm.create_vm().unwrap();
///
/// // Check whether SEV is enabled, optional.
/// assert!(unsafe { vm.encrypt_op(null_mut() as *mut c_void) }.is_ok());
///
/// // Initialize the SEV platform context.
/// let mut init: kvm_sev_cmd = Default::default();
/// vm.encrypt_op_sev(&mut init).unwrap();
/// ```
#[cfg(any(target_arch = "x86", target_arch = "x86_64"))]
pub fn encrypt_op_sev(&self, op: &mut kvm_sev_cmd) -> Result<()> {
// SAFETY: Safe because we know that kernel will only read the correct amount of memory
// from our pointer and we know where it will write it (op.error).
unsafe { self.encrypt_op(op) }
}

/// Register a guest memory region which may contain encrypted data.
///
/// It is used in the SEV-enabled guest.
Expand Down Expand Up @@ -1444,7 +1357,7 @@ impl VmFd {
///
/// // Initialize the SEV platform context.
/// let mut init: kvm_sev_cmd = Default::default();
/// assert!(vm.encrypt_op_sev(&mut init).is_ok());
/// assert!(kvm.encrypt_op_sev(&vm, &mut init).is_ok());
///
/// // Create the memory encryption context.
/// let start_data: kvm_sev_launch_start = Default::default();
Expand All @@ -1454,7 +1367,7 @@ impl VmFd {
/// sev_fd: sev.as_raw_fd() as _,
/// ..Default::default()
/// };
/// assert!(vm.encrypt_op_sev(&mut start).is_ok());
/// assert!(kvm.encrypt_op_sev(&vm, &mut start).is_ok());
///
/// let addr = unsafe {
/// libc::mmap(
Expand Down Expand Up @@ -1520,7 +1433,7 @@ impl VmFd {
///
/// // Initialize the SEV platform context.
/// let mut init: kvm_sev_cmd = Default::default();
/// assert!(vm.encrypt_op_sev(&mut init).is_ok());
/// assert!(kvm.encrypt_op_sev(&vm, &mut init).is_ok());
///
/// // Create the memory encryption context.
/// let start_data: kvm_sev_launch_start = Default::default();
Expand All @@ -1530,7 +1443,7 @@ impl VmFd {
/// sev_fd: sev.as_raw_fd() as _,
/// ..Default::default()
/// };
/// assert!(vm.encrypt_op_sev(&mut start).is_ok());
/// assert!(kvm.encrypt_op_sev(&vm, &mut start).is_ok());
///
/// let addr = unsafe {
/// libc::mmap(
Expand Down Expand Up @@ -2242,17 +2155,6 @@ mod tests {
assert!(vm.check_extension(Cap::MpState));
}

#[test]
#[cfg(any(target_arch = "x86", target_arch = "x86_64"))]
#[cfg_attr(not(has_sev), ignore)]
fn test_encrypt_op_sev() {
let kvm = Kvm::new().unwrap();
let vm = kvm.create_vm().unwrap();

let mut init: kvm_sev_cmd = Default::default();
assert!(vm.encrypt_op_sev(&mut init).is_ok());
}

#[test]
#[cfg(any(target_arch = "x86", target_arch = "x86_64"))]
#[cfg_attr(not(has_sev), ignore)]
Expand All @@ -2270,7 +2172,7 @@ mod tests {
// https://www.kernel.org/doc/Documentation/virtual/kvm/amd-memory-encryption.rst

let mut init: kvm_sev_cmd = Default::default();
assert!(vm.encrypt_op_sev(&mut init).is_ok());
assert!(kvm.encrypt_op_sev(&vm, &mut init).is_ok());

let start_data: kvm_sev_launch_start = Default::default();
let mut start = kvm_sev_cmd {
Expand All @@ -2279,7 +2181,7 @@ mod tests {
sev_fd: sev.as_raw_fd() as _,
..Default::default()
};
assert!(vm.encrypt_op_sev(&mut start).is_ok());
assert!(kvm.encrypt_op_sev(&vm, &mut start).is_ok());

let addr = unsafe {
libc::mmap(
Expand Down