Skip to content

Commit e2d67d3

Browse files
committed
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]>
1 parent c9cc8d1 commit e2d67d3

File tree

2 files changed

+34
-1
lines changed

2 files changed

+34
-1
lines changed

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

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -607,6 +607,17 @@ impl Net {
607607
continue;
608608
}
609609
};
610+
611+
// We only handle frames that are up to MAX_BUFFER_SIZE
612+
if buffer.len() > MAX_BUFFER_SIZE {
613+
error!("net: received too big frame from driver");
614+
self.metrics.tx_malformed_frames.inc();
615+
tx_queue
616+
.add_used(mem, head_index, 0)
617+
.map_err(DeviceError::QueueError)?;
618+
continue;
619+
}
620+
610621
if !Self::rate_limiter_consume_op(&mut self.tx_rate_limiter, buffer.len() as u64) {
611622
tx_queue.undo_pop();
612623
self.metrics.tx_rate_limiter_throttled.inc();
@@ -1323,6 +1334,28 @@ pub mod tests {
13231334
assert!(!tap_traffic_simulator.pop_rx_packet(&mut []));
13241335
}
13251336

1337+
#[test]
1338+
fn test_tx_big_frame() {
1339+
let mut th = TestHelper::get_default();
1340+
th.activate_net();
1341+
let tap_traffic_simulator = TapTrafficSimulator::new(if_index(&th.net().tap));
1342+
1343+
// Send an invalid frame (too big, maximum buffer is MAX_BUFFER_SIZE).
1344+
th.add_desc_chain(NetQueue::Tx, 0, &[(0, 100000, 0)]);
1345+
check_metric_after_block!(
1346+
th.net().metrics.tx_malformed_frames,
1347+
1,
1348+
th.event_manager.run_with_timeout(100)
1349+
);
1350+
1351+
// Check that the used queue advanced.
1352+
assert_eq!(th.txq.used.idx.get(), 1);
1353+
assert!(&th.net().irq_trigger.has_pending_irq(IrqType::Vring));
1354+
th.txq.check_used_elem(0, 0, 0);
1355+
// Check that the frame was skipped.
1356+
assert!(!tap_traffic_simulator.pop_rx_packet(&mut []));
1357+
}
1358+
13261359
#[test]
13271360
fn test_tx_empty_frame() {
13281361
let mut th = TestHelper::get_default();

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -395,7 +395,7 @@ pub mod test {
395395
pub fn get_default() -> TestHelper<'a> {
396396
let mut event_manager = EventManager::new().unwrap();
397397
let mut net = default_net();
398-
let mem = single_region_mem(MAX_BUFFER_SIZE);
398+
let mem = single_region_mem(2 * MAX_BUFFER_SIZE);
399399

400400
// transmute mem_ref lifetime to 'a
401401
let mem_ref = unsafe { mem::transmute::<&GuestMemoryMmap, &'a GuestMemoryMmap>(&mem) };

0 commit comments

Comments
 (0)