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.

Sign up for GitHub

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

Jump to bottom

fix(snapshot): prevent an integer overflow when calculating TSC scaling #4606

Merged
merged 1 commit into from
May 7, 2024
Merged

fix(snapshot): prevent an integer overflow when calculating TSC scaling #4606

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
25 changes: 19 additions & 6 deletions src/vmm/src/vstate/vcpu/x86_64.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,8 @@ use crate::vstate::vm::Vm;
// The value of 250 parts per million is based on
// the QEMU approach, more details here:
// https://bugzilla.redhat.com/show_bug.cgi?id=1839095
const TSC_KHZ_TOL_NUMERATOR: u32 = 250;
const TSC_KHZ_TOL_DENOMINATOR: u32 = 1_000_000;
const TSC_KHZ_TOL_NUMERATOR: i64 = 250;
const TSC_KHZ_TOL_DENOMINATOR: i64 = 1_000_000;

/// Errors associated with the wrappers over KVM ioctls.
#[derive(Debug, PartialEq, Eq, thiserror::Error, displaydoc::Display)]
Expand Down Expand Up @@ -433,7 +433,8 @@ impl KvmVcpu {
// per million beacuse it is common for TSC frequency
// to differ due to calibration at boot time.
let diff = (i64::from(self.get_tsc_khz()?) - i64::from(state_tsc_freq)).abs();
Ok(diff > i64::from(state_tsc_freq * TSC_KHZ_TOL_NUMERATOR / TSC_KHZ_TOL_DENOMINATOR))
// Cannot overflow since u32::MAX * 250 < i64::MAX
Ok(diff > i64::from(state_tsc_freq) * TSC_KHZ_TOL_NUMERATOR / TSC_KHZ_TOL_DENOMINATOR)
roypat marked this conversation as resolved.
Show resolved Hide resolved
}

/// Scale the TSC frequency of this vCPU to the one provided as a parameter.
Expand Down Expand Up @@ -860,7 +861,9 @@ mod tests {
let mut state = vcpu.save_state().unwrap();
state.tsc_khz = Some(
state.tsc_khz.unwrap()
+ state.tsc_khz.unwrap() * TSC_KHZ_TOL_NUMERATOR / TSC_KHZ_TOL_DENOMINATOR / 2,
+ state.tsc_khz.unwrap() * u32::try_from(TSC_KHZ_TOL_NUMERATOR).unwrap()
/ u32::try_from(TSC_KHZ_TOL_DENOMINATOR).unwrap()
/ 2,
);
assert!(!vcpu
.is_tsc_scaling_required(state.tsc_khz.unwrap())
Expand All @@ -872,12 +875,20 @@ mod tests {
let mut state = vcpu.save_state().unwrap();
state.tsc_khz = Some(
state.tsc_khz.unwrap()
+ state.tsc_khz.unwrap() * TSC_KHZ_TOL_NUMERATOR / TSC_KHZ_TOL_DENOMINATOR * 2,
+ state.tsc_khz.unwrap() * u32::try_from(TSC_KHZ_TOL_NUMERATOR).unwrap()
/ u32::try_from(TSC_KHZ_TOL_DENOMINATOR).unwrap()
* 2,
);
assert!(vcpu
.is_tsc_scaling_required(state.tsc_khz.unwrap())
.unwrap());
}

{
// Try a large frequency (30GHz) in the state and check it doesn't
// overflow
assert!(vcpu.is_tsc_scaling_required(30_000_000).unwrap());
}
}

#[test]
Expand All @@ -886,7 +897,9 @@ mod tests {
let mut state = vcpu.save_state().unwrap();
state.tsc_khz = Some(
state.tsc_khz.unwrap()
+ state.tsc_khz.unwrap() * TSC_KHZ_TOL_NUMERATOR / TSC_KHZ_TOL_DENOMINATOR * 2,
+ state.tsc_khz.unwrap() * u32::try_from(TSC_KHZ_TOL_NUMERATOR).unwrap()
/ u32::try_from(TSC_KHZ_TOL_DENOMINATOR).unwrap()
* 2,
);

if vm.fd().check_extension(Cap::TscControl) {
Expand Down
Loading