Miscellaneous fixes that don't fall anywhere else

In vmm/builder.rs, the CPU count can be fetched from the VM
configuration without having to do a type conversion.

In the serial device and the networking code, I added explicit checks to
ensure that values fit in bounds.

In the vCPU code, the existing code uses floats for tolerance values. We
can replace these all with integer multiplications and divisions.

Signed-off-by: Jonathan Browne <[email protected]>

Author: JBYoshi

src/vmm/src/builder.rs

@@ -812,7 +812,7 @@ pub fn configure_system_for_boot(
             utils::vm_memory::GuestAddress(crate::arch::x86_64::layout::CMDLINE_START),
             cmdline_size,
             initrd,
-            vcpus.len() as u8,
+            vcpu_config.vcpu_count,
         )
         .map_err(ConfigureSystem)?;
     }

src/vmm/src/devices/legacy/serial.rs

@@ -306,22 +306,22 @@ impl<I: Read + AsRawFd + Send + Debug + 'static>
     SerialWrapper<EventFdTrigger, SerialEventsWrapper, I>
 {
     pub fn bus_read(&mut self, offset: u64, data: &mut [u8]) {
-        if data.len() != 1 {
+        if let (Ok(offset), 1) = (u8::try_from(offset), data.len()) {
+            data[0] = self.serial.read(offset);
+        } else {
             METRICS.uart.missed_read_count.inc();
-            return;
         }
-        data[0] = self.serial.read(offset as u8);
     }
 
     pub fn bus_write(&mut self, offset: u64, data: &[u8]) {
-        if data.len() != 1 {
+        if let (Ok(offset), 1) = (u8::try_from(offset), data.len()) {
+            if let Err(err) = self.serial.write(offset, data[0]) {
+                // Counter incremented for any handle_write() error.
+                error!("Failed the write to serial: {:?}", err);
+                METRICS.uart.error_count.inc();
+            }
+        } else {
             METRICS.uart.missed_write_count.inc();
-            return;
-        }
-        if let Err(err) = self.serial.write(offset as u8, data[0]) {
-            // Counter incremented for any handle_write() error.
-            error!("Failed the write to serial: {:?}", err);
-            METRICS.uart.error_count.inc();
         }
     }
 }

src/vmm/src/dumbo/pdu/udp.rs

@@ -26,7 +26,7 @@ pub const UDP_HEADER_SIZE: usize = 8;
 
 // A UDP datagram is carried in a single IP packet and is hence limited
 // to a maximum payload of 65,507 bytes for IPv4 and 65,527 bytes for IPv6 [2]
-const IPV4_MAX_UDP_PACKET_SIZE: usize = 65507;
+const IPV4_MAX_UDP_PACKET_SIZE: u16 = 65507;
 
 /// Represents errors which may occur while parsing or writing a datagram.
 #[derive(Debug, PartialEq, Eq)]
@@ -136,14 +136,14 @@ impl<'a, T: NetworkBytesMut + Debug> UdpDatagram<'a, T> {
         let mut packet = UdpDatagram::from_bytes(buf, None)?;
         let len = payload.len() + UDP_HEADER_SIZE;
 
-        // TODO working with IPv4 only for now
-        if len > IPV4_MAX_UDP_PACKET_SIZE {
-            return Err(Error::PayloadTooBig);
-        }
+        let len = match u16::try_from(len) {
+            Ok(len) if len <= IPV4_MAX_UDP_PACKET_SIZE => len,
+            _ => return Err(Error::PayloadTooBig),
+        };
 
-        packet.bytes.shrink_unchecked(len);
+        packet.bytes.shrink_unchecked(len.into());
         packet.payload_mut().copy_from_slice(payload);
-        packet.set_len(len as u16);
+        packet.set_len(len);
 
         Ok(Incomplete::new(packet))
     }
@@ -250,7 +250,7 @@ mod tests {
     #[test]
     fn test_failing_construction() {
         let mut raw = [0u8; 8];
-        let huge_payload = [0u8; IPV4_MAX_UDP_PACKET_SIZE];
+        let huge_payload = [0u8; IPV4_MAX_UDP_PACKET_SIZE as usize];
 
         assert_eq!(
             UdpDatagram::write_incomplete_datagram(raw.as_mut(), &huge_payload).unwrap_err(),

src/vmm/src/dumbo/tcp/connection.rs

@@ -877,11 +877,12 @@ impl Connection {
         if let Some((read_buf, payload_seq)) = payload_src {
             // Limit the size of read_buf so it doesn't mess up later calculations (as usual, I take
             // the easy way out).
-            if read_buf.len() > MAX_WINDOW_SIZE as usize {
-                return Err(WriteNextError::PayloadBufTooLarge);
-            }
+            let len = match u32::try_from(read_buf.len()) {
+                Ok(len) if len <= MAX_WINDOW_SIZE => len,
+                _ => return Err(WriteNextError::PayloadBufTooLarge),
+            };
 
-            let payload_end = payload_seq + Wrapping(read_buf.len() as u32);
+            let payload_end = payload_seq + Wrapping(len);
 
             let mut rto_triggered = false;
 

src/vmm/src/vstate/vcpu/x86_64.rs

@@ -29,7 +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: f64 = 250.0 / 1_000_000.0;
+const TSC_KHZ_TOL_NUMERATOR: u32 = 250;
+const TSC_KHZ_TOL_DENOMINATOR: u32 = 1_000_000;
 
 /// Errors associated with the wrappers over KVM ioctls.
 #[derive(Debug, PartialEq, Eq, thiserror::Error, displaydoc::Display)]
@@ -439,7 +440,7 @@ 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 as f64 > (f64::from(state_tsc_freq) * TSC_KHZ_TOL).round())
+        Ok(diff > i64::from(state_tsc_freq * TSC_KHZ_TOL_NUMERATOR / TSC_KHZ_TOL_DENOMINATOR))
     }
 
     /// Scale the TSC frequency of this vCPU to the one provided as a parameter.
@@ -891,7 +892,7 @@ mod tests {
     }
 
     #[test]
-    #[allow(clippy::cast_sign_loss, clippy::redundant_clone)] // always positive, no u32::try_from(f64)
+    #[allow(clippy::redundant_clone)]
     fn test_is_tsc_scaling_required() {
         // Test `is_tsc_scaling_required` as if it were on the same
         // CPU model as the one in the snapshot state.
@@ -901,7 +902,8 @@ mod tests {
         {
             // The frequency difference is within tolerance.
             let mut state = orig_state.clone();
-            state.tsc_khz = Some(state.tsc_khz.unwrap() + (TSC_KHZ_TOL / 2.0).round() as u32);
+            state.tsc_khz =
+                Some(state.tsc_khz.unwrap() + TSC_KHZ_TOL_NUMERATOR / TSC_KHZ_TOL_DENOMINATOR / 2);
             assert!(!vcpu
                 .is_tsc_scaling_required(state.tsc_khz.unwrap())
                 .unwrap());
@@ -910,19 +912,20 @@ mod tests {
         {
             // The frequency difference is over the tolerance.
             let mut state = orig_state;
-            state.tsc_khz = Some(state.tsc_khz.unwrap() + (TSC_KHZ_TOL * 2.0).round() as u32);
+            state.tsc_khz =
+                Some(state.tsc_khz.unwrap() + TSC_KHZ_TOL_NUMERATOR / TSC_KHZ_TOL_DENOMINATOR * 2);
             assert!(!vcpu
                 .is_tsc_scaling_required(state.tsc_khz.unwrap())
                 .unwrap());
         }
     }
 
     #[test]
-    #[allow(clippy::cast_sign_loss)] // always positive, no u32::try_from(f64)
     fn test_set_tsc() {
         let (vm, vcpu, _) = setup_vcpu(0x1000);
         let mut state = vcpu.save_state().unwrap();
-        state.tsc_khz = Some(state.tsc_khz.unwrap() + (TSC_KHZ_TOL * 2.0).round() as u32);
+        state.tsc_khz =
+            Some(state.tsc_khz.unwrap() + TSC_KHZ_TOL_NUMERATOR / TSC_KHZ_TOL_DENOMINATOR * 2);
 
         if vm.fd().check_extension(Cap::TscControl) {
             assert!(vcpu.set_tsc_khz(state.tsc_khz.unwrap()).is_ok());