net: limit the size of the guest buffer in TX path

When we switched to using writev for copying a network packet from guest
memory to the tap device we dropped an (implicit) check for the size of
the TX frame.

Reintroduce that check since we should be handling only frames of up to
MAX_BUFFER_SIZE.

This, also, controls the amount of memory we allocate in the Firecracker
process for copying frames that are destined for MMDS from guest memory
to Firecracker memory.

Signed-off-by: Babis Chalios <[email protected]>

CHANGELOG.md

@@ -41,6 +41,15 @@ and this project adheres to
 
 ### Fixed
 
+- [4526](https://github.com/firecracker-microvm/firecracker/pull/4526): Added a
+  check in the network TX path that the size of the network frames the guest
+  passes to us is not bigger than the maximum frame the device expects to
+  handle. On the TX path, we copy frames destined to MMDS from guest memory to
+  Firecracker memory. Without the check, a mis-behaving virtio-net driver could
+  cause an increase in the memory footprint of the Firecracker process. Now, if
+  we receive such a frame, we ignore it and increase `Net::tx_malformed_frames`
+  metric.
+
 ## \[1.7.0\]
 
 ### Added

src/vmm/src/devices/virtio/net/device.rs

@@ -607,6 +607,17 @@ impl Net {
                     continue;
                 }
             };
+
+            // We only handle frames that are up to MAX_BUFFER_SIZE
+            if buffer.len() > MAX_BUFFER_SIZE {
+                error!("net: received too big frame from driver");
+                self.metrics.tx_malformed_frames.inc();
+                tx_queue
+                    .add_used(mem, head_index, 0)
+                    .map_err(DeviceError::QueueError)?;
+                continue;
+            }
+
             if !Self::rate_limiter_consume_op(&mut self.tx_rate_limiter, buffer.len() as u64) {
                 tx_queue.undo_pop();
                 self.metrics.tx_rate_limiter_throttled.inc();
@@ -1323,6 +1334,32 @@ pub mod tests {
         assert!(!tap_traffic_simulator.pop_rx_packet(&mut []));
     }
 
+    #[test]
+    fn test_tx_big_frame() {
+        let mut th = TestHelper::get_default();
+        th.activate_net();
+        let tap_traffic_simulator = TapTrafficSimulator::new(if_index(&th.net().tap));
+
+        // Send an invalid frame (too big, maximum buffer is MAX_BUFFER_SIZE).
+        th.add_desc_chain(
+            NetQueue::Tx,
+            0,
+            &[(0, (MAX_BUFFER_SIZE + 1).try_into().unwrap(), 0)],
+        );
+        check_metric_after_block!(
+            th.net().metrics.tx_malformed_frames,
+            1,
+            th.event_manager.run_with_timeout(100)
+        );
+
+        // Check that the used queue advanced.
+        assert_eq!(th.txq.used.idx.get(), 1);
+        assert!(&th.net().irq_trigger.has_pending_irq(IrqType::Vring));
+        th.txq.check_used_elem(0, 0, 0);
+        // Check that the frame was skipped.
+        assert!(!tap_traffic_simulator.pop_rx_packet(&mut []));
+    }
+
     #[test]
     fn test_tx_empty_frame() {
         let mut th = TestHelper::get_default();

src/vmm/src/devices/virtio/net/test_utils.rs

@@ -395,7 +395,7 @@ pub mod test {
         pub fn get_default() -> TestHelper<'a> {
             let mut event_manager = EventManager::new().unwrap();
             let mut net = default_net();
-            let mem = single_region_mem(MAX_BUFFER_SIZE);
+            let mem = single_region_mem(2 * MAX_BUFFER_SIZE);
 
             // transmute mem_ref lifetime to 'a
             let mem_ref = unsafe { mem::transmute::<&GuestMemoryMmap, &'a GuestMemoryMmap>(&mem) };