Backport v1.15.1 hardening to v1.6.x

src/cpu-template-helper/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "cpu-template-helper"
-version = "1.6.5"
+version = "1.6.6"
 authors = ["Amazon Firecracker team <firecracker-devel@amazon.com>"]
 edition = "2021"
 license = "Apache-2.0"

src/firecracker/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "firecracker"
-version = "1.6.5"
+version = "1.6.6"
 authors = ["Amazon Firecracker team <firecracker-devel@amazon.com>"]
 edition = "2021"
 build = "build.rs"

src/jailer/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "jailer"
-version = "1.6.5"
+version = "1.6.6"
 authors = ["Amazon Firecracker team <firecracker-devel@amazon.com>"]
 edition = "2021"
 description = "Process for starting Firecracker in production scenarios; applies a cgroup/namespace isolation barrier and then drops privileges."

src/rebase-snap/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "rebase-snap"
-version = "1.6.5"
+version = "1.6.6"
 authors = ["Amazon Firecracker team <firecracker-devel@amazon.com>"]
 edition = "2021"
 license = "Apache-2.0"

src/seccompiler/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "seccompiler"
-version = "1.6.5"
+version = "1.6.6"
 authors = ["Amazon Firecracker team <firecracker-devel@amazon.com>"]
 edition = "2021"
 description = "Program that compiles multi-threaded seccomp-bpf filters expressed as JSON into raw BPF programs, serializing them and outputting them to a file."

src/snapshot-editor/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "snapshot-editor"
-version = "1.6.5"
+version = "1.6.6"
 authors = ["Amazon Firecracker team <firecracker-devel@amazon.com>"]
 edition = "2021"
 license = "Apache-2.0"

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

@@ -7,7 +7,7 @@ use std::sync::Arc;
 use std::time::Duration;
 use std::{cmp, fmt};
 
-use log::error;
+use log::{error, warn};
 use serde::Serialize;
 use timerfd::{ClockId, SetTimeFlags, TimerFd, TimerState};
 use utils::eventfd::EventFd;
@@ -35,6 +35,16 @@ use crate::vstate::memory::{Address, ByteValued, Bytes, GuestAddress, GuestMemor
 
 const SIZE_OF_U32: usize = std::mem::size_of::<u32>();
 const SIZE_OF_STAT: usize = std::mem::size_of::<BalloonStat>();
+/// Upper bound on the number of stats tags a guest may report.
+/// The VirtIO spec currently defines 16, but newer kernel versions can
+/// add more (e.g. Linux 6.12 added several, see 74c025c5d7e4). We use a
+/// generous limit that still bounds computation without breaking on future
+/// kernels.
+const MAX_STATS_TAGS: u32 = 256;
+/// Maximum valid stats descriptor length in bytes.
+/// Descriptors exceeding this are rejected to prevent unbounded iteration.
+#[allow(clippy::cast_possible_truncation)]
+const MAX_STATS_DESC_LEN: u32 = MAX_STATS_TAGS * std::mem::size_of::<BalloonStat>() as u32;
 
 fn mib_to_pages(amount_mib: u32) -> Result<u32, BalloonError> {
     amount_mib
@@ -410,6 +420,21 @@ impl Balloon {
                     .add_used(mem, prev_stats_desc, 0)
                     .map_err(BalloonError::Queue)?;
             }
+
+            // Reject oversized descriptors to prevent a guest from causing
+            // excessive iteration on the VMM event loop.
+            // We still hold onto the descriptor (via stats_desc_index below)
+            // so that the stats request/response protocol is preserved and
+            // trigger_stats_update can return it to the guest later.
+            if head.len > MAX_STATS_DESC_LEN {
+                warn!(
+                    "balloon: stats descriptor too large: {} > {}, skipping",
+                    head.len, MAX_STATS_DESC_LEN
+                );
+                self.stats_desc_index = Some(head.index);
+                continue;
+            }
+
             for index in (0..head.len).step_by(SIZE_OF_STAT) {
                 // Read the address at position `index`. The only case
                 // in which this fails is if there is overflow,
@@ -1162,4 +1187,71 @@ pub(crate) mod tests {
         assert_eq!(balloon.num_pages(), 0x1122_3344);
         assert_eq!(balloon.actual_pages(), 0x1234_5678);
     }
+
+    /// Test that process_stats_queue holds oversized descriptors without
+    /// updating stats, and updates stats for valid-length ones.
+    #[test]
+    fn test_stats_queue_oversized_descriptor_rejected() {
+        struct TestCase {
+            desc_len: u32,
+            stats_updated: bool,
+        }
+
+        let cases = [
+            TestCase {
+                desc_len: MAX_STATS_DESC_LEN + 1,
+                stats_updated: false,
+            },
+            TestCase {
+                desc_len: MAX_STATS_DESC_LEN,
+                stats_updated: true,
+            },
+        ];
+
+        let stat_addr: u64 = 0x1000;
+
+        for tc in &cases {
+            let mut balloon = Balloon::new(0, true, 1, false, false).unwrap();
+            let mem = default_mem();
+            let statsq = VirtQueue::new(GuestAddress(0), &mem, 16);
+            balloon.set_queue(INFLATE_INDEX, statsq.create_queue());
+            balloon.set_queue(DEFLATE_INDEX, statsq.create_queue());
+            balloon.set_queue(STATS_INDEX, statsq.create_queue());
+            balloon.activate(mem.clone()).unwrap();
+
+            // Fill the descriptor region with a recognisable stat value.
+            let n_stats = tc.desc_len as usize / SIZE_OF_STAT;
+            for i in 0..n_stats {
+                mem.write_obj::<BalloonStat>(
+                    BalloonStat {
+                        tag: VIRTIO_BALLOON_S_MEMFREE,
+                        val: 0xBEEF,
+                    },
+                    GuestAddress(stat_addr + (i * SIZE_OF_STAT) as u64),
+                )
+                .unwrap();
+            }
+
+            set_request(&statsq, 0, stat_addr, tc.desc_len, VIRTQ_DESC_F_NEXT);
+            balloon.queue_events()[STATS_INDEX].write(1).unwrap();
+            balloon.process_stats_queue_event().unwrap();
+
+            // The descriptor should always be held (stats protocol preserved)
+            // regardless of whether the stats were updated.
+            assert!(
+                balloon.stats_desc_index.is_some(),
+                "desc_len={}: descriptor should be held",
+                tc.desc_len,
+            );
+
+            // Verify stats were only updated for valid descriptors.
+            assert_eq!(
+                balloon.latest_stats.free_memory.is_some(),
+                tc.stats_updated,
+                "desc_len={}: expected stats_updated={}",
+                tc.desc_len,
+                tc.stats_updated,
+            );
+        }
+    }
 }

src/vmm/src/devices/virtio/mmio.rs

@@ -172,55 +172,58 @@ impl MmioTransport {
     #[allow(unused_assignments)]
     fn set_device_status(&mut self, status: u32) {
         use device_status::*;
-        // match changed bits
-        match !self.device_status & status {
-            ACKNOWLEDGE if self.device_status == INIT => {
-                self.device_status = status;
-            }
-            DRIVER if self.device_status == ACKNOWLEDGE => {
-                self.device_status = status;
+
+        const VALID_TRANSITIONS: &[(u32, u32)] = &[
+            (INIT, ACKNOWLEDGE),
+            (ACKNOWLEDGE, ACKNOWLEDGE | DRIVER),
+            (ACKNOWLEDGE | DRIVER, ACKNOWLEDGE | DRIVER | FEATURES_OK),
+            (
+                ACKNOWLEDGE | DRIVER | FEATURES_OK,
+                ACKNOWLEDGE | DRIVER | FEATURES_OK | DRIVER_OK,
+            ),
+        ];
+
+        if (status & FAILED) != 0 {
+            // TODO: notify backend driver to stop the device
+            self.device_status |= FAILED;
+        } else if status == INIT {
+            if self.locked_device().is_activated() {
+                let mut device_status = self.device_status;
+                let reset_result = self.locked_device().reset();
+                match reset_result {
+                    Some((_interrupt_evt, mut _queue_evts)) => {}
+                    None => {
+                        device_status |= FAILED;
+                    }
+                }
+                self.device_status = device_status;
             }
-            FEATURES_OK if self.device_status == (ACKNOWLEDGE | DRIVER) => {
-                self.device_status = status;
+
+            // If the backend device driver doesn't support reset,
+            // just leave the device marked as FAILED.
+            if self.device_status & FAILED == 0 {
+                self.reset();
             }
-            DRIVER_OK if self.device_status == (ACKNOWLEDGE | DRIVER | FEATURES_OK) => {
-                self.device_status = status;
+        } else if VALID_TRANSITIONS
+            .iter()
+            .any(|&(from, to)| self.device_status == from && status == to)
+        {
+            self.device_status = status;
+
+            // Activate the device when transitioning to DRIVER_OK.
+            if status == (ACKNOWLEDGE | DRIVER | FEATURES_OK | DRIVER_OK) {
                 let device_activated = self.locked_device().is_activated();
                 if !device_activated && self.are_queues_valid() {
                     self.locked_device()
                         .activate(self.mem.clone())
                         .expect("Failed to activate device");
                 }
             }
-            _ if (status & FAILED) != 0 => {
-                // TODO: notify backend driver to stop the device
-                self.device_status |= FAILED;
-            }
-            _ if status == 0 => {
-                if self.locked_device().is_activated() {
-                    let mut device_status = self.device_status;
-                    let reset_result = self.locked_device().reset();
-                    match reset_result {
-                        Some((_interrupt_evt, mut _queue_evts)) => {}
-                        None => {
-                            device_status |= FAILED;
-                        }
-                    }
-                    self.device_status = device_status;
-                }
-
-                // If the backend device driver doesn't support reset,
-                // just leave the device marked as FAILED.
-                if self.device_status & FAILED == 0 {
-                    self.reset();
-                }
-            }
-            _ => {
-                warn!(
-                    "invalid virtio driver status transition: 0x{:x} -> 0x{:x}",
-                    self.device_status, status
-                );
-            }
+        } else {
+            warn!(
+                "invalid virtio driver status transition: {:#x} -> {:#x}",
+                self.device_status, status
+            );
         }
     }
 }

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

@@ -23,6 +23,13 @@ use crate::vstate::memory::GuestMemoryMmap;
 
 pub const ENTROPY_DEV_ID: &str = "rng";
 
+// Cap the per-request entropy allocation. A guest can craft a virtio
+// descriptor chain whose total `iovec.len()` is the sum of many overlapping
+// descriptors, reaching multi-GiB sizes from a small guest memory and
+// exhausting host RAM. The guest still gets up to this many bytes of entropy
+// per request.
+const MAX_ENTROPY_BYTES: u32 = 64 * 1024;
+
 #[derive(Debug, thiserror::Error, displaydoc::Display)]
 pub enum EntropyError {
     /// Error while handling an Event file descriptor: {0}
@@ -112,15 +119,17 @@ impl Entropy {
             return Ok(0);
         }
 
-        let mut rand_bytes = vec![0; iovec.len()];
+        let len = std::cmp::min(iovec.len(), MAX_ENTROPY_BYTES as usize);
+        let mut rand_bytes = vec![0; len];
         rand::fill(&mut rand_bytes).map_err(|err| {
             METRICS.host_rng_fails.inc();
             err
         })?;
 
-        // It is ok to unwrap here. We are writing `iovec.len()` bytes at offset 0.
+        // We are writing at most `iovec.len()` bytes starting at offset 0.
         iovec.write_all_volatile_at(&rand_bytes, 0).unwrap();
-        Ok(iovec.len().try_into().unwrap())
+        // `len` fits in u32: bounded above by MAX_ENTROPY_BYTES (64 KiB).
+        Ok(len.try_into().unwrap())
     }
 
     fn process_entropy_queue(&mut self) {