Skip to content

Commit 3a9c45e

Browse files
zulinx86ShadowCurse
authored andcommitted
Allocate Xsave with an appropriate FAM size
Although `KVM_CHECK_EXTENSION(KVM_CAP_XSAVE2)` returns the size in byte, FAM entry is `__u32` type. So overly large memory space was allocated. Note that the previous code also works since it's large enough. Signed-off-by: Takahiro Itazuri <[email protected]>
1 parent f28ca49 commit 3a9c45e

File tree

1 file changed

+20
-11
lines changed

1 file changed

+20
-11
lines changed

kvm-ioctls/src/ioctls/vcpu.rs

Lines changed: 20 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -876,9 +876,9 @@ impl VcpuFd {
876876
/// This function is unsafe because there is no guarantee `xsave` is allocated with enough space
877877
/// to hold the entire xsave state.
878878
///
879-
/// The required size can be retrieved via `KVM_CHECK_EXTENSION(KVM_CAP_XSAVE2)` and can vary
880-
/// depending on features that have been dynamically enabled by `arch_prctl()`. Thus, any
881-
/// features must not be enabled dynamically after the required size has been confirmed.
879+
/// The required size in bytes can be retrieved via `KVM_CHECK_EXTENSION(KVM_CAP_XSAVE2)` and
880+
/// can vary depending on features that have been dynamically enabled by `arch_prctl()`. Thus,
881+
/// any features must not be enabled dynamically after the required size has been confirmed.
882882
///
883883
/// If `xsave` is not large enough, `KVM_GET_XSAVE2` copies data beyond the allocated area,
884884
/// possibly causing undefined behavior.
@@ -891,14 +891,17 @@ impl VcpuFd {
891891
/// ```rust
892892
/// # extern crate kvm_ioctls;
893893
/// # extern crate kvm_bindings;
894+
/// # extern crate vmm_sys_util;
894895
/// # use kvm_ioctls::{Kvm, Cap};
895-
/// # use kvm_bindings::{Xsave, kvm_xsave};
896+
/// # use kvm_bindings::{Xsave, kvm_xsave, kvm_xsave2};
897+
/// # use vmm_sys_util::fam::FamStruct;
896898
/// let kvm = Kvm::new().unwrap();
897899
/// let vm = kvm.create_vm().unwrap();
898900
/// let vcpu = vm.create_vcpu(0).unwrap();
899901
/// let xsave_size = vm.check_extension_int(Cap::Xsave2);
900902
/// if xsave_size > 0 {
901-
/// let fam_size = xsave_size as usize - std::mem::size_of::<kvm_xsave>();
903+
/// let fam_size = (xsave_size as usize - std::mem::size_of::<kvm_xsave>())
904+
/// .div_ceil(std::mem::size_of::<<kvm_xsave2 as FamStruct>::Entry>());
902905
/// let mut xsave = Xsave::new(fam_size).unwrap();
903906
/// unsafe { vcpu.get_xsave2(&mut xsave).unwrap() };
904907
/// }
@@ -981,9 +984,9 @@ impl VcpuFd {
981984
/// This function is unsafe because there is no guarantee `xsave` is properly allocated with
982985
/// the size that KVM assumes.
983986
///
984-
/// The required size can be retrieved via `KVM_CHECK_EXTENSION(KVM_CAP_XSAVE2)` and can vary
985-
/// depending on features that have been dynamically enabled by `arch_prctl()`. Thus, any
986-
/// features must not be enabled after the required size has been confirmed.
987+
/// The required size in bytes can be retrieved via `KVM_CHECK_EXTENSION(KVM_CAP_XSAVE2)` and
988+
/// can vary depending on features that have been dynamically enabled by `arch_prctl()`. Thus,
989+
/// any features must not be enabled after the required size has been confirmed.
987990
///
988991
/// If `xsave` is not large enough, `KVM_SET_XSAVE` copies data beyond the allocated area to
989992
/// the kernel, possibly causing undefined behavior.
@@ -996,14 +999,17 @@ impl VcpuFd {
996999
/// ```rust
9971000
/// # extern crate kvm_ioctls;
9981001
/// # extern crate kvm_bindings;
1002+
/// # extern crate vmm_sys_util;
9991003
/// # use kvm_ioctls::{Kvm, Cap};
1000-
/// # use kvm_bindings::{Xsave, kvm_xsave};
1004+
/// # use kvm_bindings::{Xsave, kvm_xsave, kvm_xsave2};
1005+
/// # use vmm_sys_util::fam::FamStruct;
10011006
/// let kvm = Kvm::new().unwrap();
10021007
/// let vm = kvm.create_vm().unwrap();
10031008
/// let vcpu = vm.create_vcpu(0).unwrap();
10041009
/// let xsave_size = vm.check_extension_int(Cap::Xsave2);
10051010
/// if xsave_size > 0 {
1006-
/// let fam_size = xsave_size as usize - std::mem::size_of::<kvm_xsave>();
1011+
/// let fam_size = (xsave_size as usize - std::mem::size_of::<kvm_xsave>())
1012+
/// .div_ceil(std::mem::size_of::<<kvm_xsave2 as FamStruct>::Entry>());
10071013
/// let xsave = Xsave::new(fam_size).unwrap();
10081014
/// // Your `xsave` manipulation here.
10091015
/// unsafe { vcpu.set_xsave2(&xsave).unwrap() };
@@ -2343,6 +2349,8 @@ mod tests {
23432349
#[cfg(target_arch = "x86_64")]
23442350
#[test]
23452351
fn xsave_test() {
2352+
use vmm_sys_util::fam::FamStruct;
2353+
23462354
let kvm = Kvm::new().unwrap();
23472355
let vm = kvm.create_vm().unwrap();
23482356
let vcpu = vm.create_vcpu(0).unwrap();
@@ -2355,7 +2363,8 @@ mod tests {
23552363
let xsave_size = vm.check_extension_int(Cap::Xsave2);
23562364
// only if KVM_CAP_XSAVE2 is supported
23572365
if xsave_size > 0 {
2358-
let fam_size = xsave_size as usize - std::mem::size_of::<kvm_xsave>();
2366+
let fam_size = (xsave_size as usize - std::mem::size_of::<kvm_xsave>())
2367+
.div_ceil(std::mem::size_of::<<kvm_xsave2 as FamStruct>::Entry>());
23592368
let mut xsave2 = Xsave::new(fam_size).unwrap();
23602369
// SAFETY: Safe because `xsave2` is allocated with enough space.
23612370
unsafe { vcpu.get_xsave2(&mut xsave2).unwrap() };

0 commit comments

Comments
 (0)