Skip to content

Commit 1cdb57c

Browse files
committed
harden(balloon): bound stats descriptor length
Backports upstream firecracker PR firecracker-microvm#5794 (commit 1689689). Adds a MAX_STATS_DESC_LEN (256 stat tags = 2560 bytes) cap on the stats descriptor processed by process_stats_queue(). Pre-fix, a guest could submit a descriptor with arbitrarily large `head.len`, causing the inner loop `for index in (0..head.len).step_by(...)` to iterate billions of times and stall the VMM event loop. No CVE was assigned upstream; AWS classifies this as a host DoS hardening rather than a security advisory. Operationally, SAFE microVMs do not attach a balloon device, so the unfixed code path is unreachable in our deployment. This is defence-in-depth for any future config that does attach one. Cherry-picked cleanly from upstream's source-code diff with two mechanical adaptations for v1.6.5: - drop the unrelated CHANGELOG hunk; - drop the second 'interrupt' argument from balloon.activate() in the new test (added in upstream's later refactor) and explicitly import the warn! macro that this module did not yet pull in. (cherry picked from commit 1689689f0a31f9b1107c01ae9c6ed5b6f110050e)
1 parent 8ff4adf commit 1cdb57c

1 file changed

Lines changed: 93 additions & 1 deletion

File tree

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

Lines changed: 93 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ use std::sync::Arc;
77
use std::time::Duration;
88
use std::{cmp, fmt};
99

10-
use log::error;
10+
use log::{error, warn};
1111
use serde::Serialize;
1212
use timerfd::{ClockId, SetTimeFlags, TimerFd, TimerState};
1313
use utils::eventfd::EventFd;
@@ -35,6 +35,16 @@ use crate::vstate::memory::{Address, ByteValued, Bytes, GuestAddress, GuestMemor
3535

3636
const SIZE_OF_U32: usize = std::mem::size_of::<u32>();
3737
const SIZE_OF_STAT: usize = std::mem::size_of::<BalloonStat>();
38+
/// Upper bound on the number of stats tags a guest may report.
39+
/// The VirtIO spec currently defines 16, but newer kernel versions can
40+
/// add more (e.g. Linux 6.12 added several, see 74c025c5d7e4). We use a
41+
/// generous limit that still bounds computation without breaking on future
42+
/// kernels.
43+
const MAX_STATS_TAGS: u32 = 256;
44+
/// Maximum valid stats descriptor length in bytes.
45+
/// Descriptors exceeding this are rejected to prevent unbounded iteration.
46+
#[allow(clippy::cast_possible_truncation)]
47+
const MAX_STATS_DESC_LEN: u32 = MAX_STATS_TAGS * std::mem::size_of::<BalloonStat>() as u32;
3848

3949
fn mib_to_pages(amount_mib: u32) -> Result<u32, BalloonError> {
4050
amount_mib
@@ -410,6 +420,21 @@ impl Balloon {
410420
.add_used(mem, prev_stats_desc, 0)
411421
.map_err(BalloonError::Queue)?;
412422
}
423+
424+
// Reject oversized descriptors to prevent a guest from causing
425+
// excessive iteration on the VMM event loop.
426+
// We still hold onto the descriptor (via stats_desc_index below)
427+
// so that the stats request/response protocol is preserved and
428+
// trigger_stats_update can return it to the guest later.
429+
if head.len > MAX_STATS_DESC_LEN {
430+
warn!(
431+
"balloon: stats descriptor too large: {} > {}, skipping",
432+
head.len, MAX_STATS_DESC_LEN
433+
);
434+
self.stats_desc_index = Some(head.index);
435+
continue;
436+
}
437+
413438
for index in (0..head.len).step_by(SIZE_OF_STAT) {
414439
// Read the address at position `index`. The only case
415440
// in which this fails is if there is overflow,
@@ -1162,4 +1187,71 @@ pub(crate) mod tests {
11621187
assert_eq!(balloon.num_pages(), 0x1122_3344);
11631188
assert_eq!(balloon.actual_pages(), 0x1234_5678);
11641189
}
1190+
1191+
/// Test that process_stats_queue holds oversized descriptors without
1192+
/// updating stats, and updates stats for valid-length ones.
1193+
#[test]
1194+
fn test_stats_queue_oversized_descriptor_rejected() {
1195+
struct TestCase {
1196+
desc_len: u32,
1197+
stats_updated: bool,
1198+
}
1199+
1200+
let cases = [
1201+
TestCase {
1202+
desc_len: MAX_STATS_DESC_LEN + 1,
1203+
stats_updated: false,
1204+
},
1205+
TestCase {
1206+
desc_len: MAX_STATS_DESC_LEN,
1207+
stats_updated: true,
1208+
},
1209+
];
1210+
1211+
let stat_addr: u64 = 0x1000;
1212+
1213+
for tc in &cases {
1214+
let mut balloon = Balloon::new(0, true, 1, false, false).unwrap();
1215+
let mem = default_mem();
1216+
let statsq = VirtQueue::new(GuestAddress(0), &mem, 16);
1217+
balloon.set_queue(INFLATE_INDEX, statsq.create_queue());
1218+
balloon.set_queue(DEFLATE_INDEX, statsq.create_queue());
1219+
balloon.set_queue(STATS_INDEX, statsq.create_queue());
1220+
balloon.activate(mem.clone()).unwrap();
1221+
1222+
// Fill the descriptor region with a recognisable stat value.
1223+
let n_stats = tc.desc_len as usize / SIZE_OF_STAT;
1224+
for i in 0..n_stats {
1225+
mem.write_obj::<BalloonStat>(
1226+
BalloonStat {
1227+
tag: VIRTIO_BALLOON_S_MEMFREE,
1228+
val: 0xBEEF,
1229+
},
1230+
GuestAddress(stat_addr + (i * SIZE_OF_STAT) as u64),
1231+
)
1232+
.unwrap();
1233+
}
1234+
1235+
set_request(&statsq, 0, stat_addr, tc.desc_len, VIRTQ_DESC_F_NEXT);
1236+
balloon.queue_events()[STATS_INDEX].write(1).unwrap();
1237+
balloon.process_stats_queue_event().unwrap();
1238+
1239+
// The descriptor should always be held (stats protocol preserved)
1240+
// regardless of whether the stats were updated.
1241+
assert!(
1242+
balloon.stats_desc_index.is_some(),
1243+
"desc_len={}: descriptor should be held",
1244+
tc.desc_len,
1245+
);
1246+
1247+
// Verify stats were only updated for valid descriptors.
1248+
assert_eq!(
1249+
balloon.latest_stats.free_memory.is_some(),
1250+
tc.stats_updated,
1251+
"desc_len={}: expected stats_updated={}",
1252+
tc.desc_len,
1253+
tc.stats_updated,
1254+
);
1255+
}
1256+
}
11651257
}

0 commit comments

Comments
 (0)