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]>

Author: bchalios

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. This avoids allocating large buffers in Firecracker memory, in case we
+  receive a malformed MMDS request. In these cases, we copy the frame from guest
+  memory to Firecracker host memory. Allowing such copies could increase the
+  memory footprint of the Firecracker process. Please note that such malformed
+  requests are only possible with a mis-behaving virtio-net driver in the guest.
+
 ## \[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,28 @@ 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, 100000, 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) };