Skip to content

Commit e85598e

Browse files
committed
fix(balloon): bound stats descriptor length
A malicious guest could submit a stats descriptor with an arbitrarily large length field, causing the VMM event loop to spin for billions of iterations and become unresponsive. Reject descriptors exceeding MAX_STATS_DESC_LEN (256 * sizeof(BalloonStat)). Cherry-pick of upstream commit 1689689 (PR firecracker-microvm#5794), with two minor 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 also import the warn! macro which is not yet pulled in by this module. (cherry picked from commit 1689689f0a31f9b1107c01ae9c6ed5b6f110050e)
1 parent fb81c21 commit e85598e

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)