Skip to content

Commit 9bc019d

Browse files
JBYoshiJonathanWoollett-Light
authored andcommitted
Apply bit masks to explicitly allow some wrapping
In these cases, values are intentionally truncated, either to split them into multiple values or to ensure that user-supplied data does not cause Firecracker to panic with a try_from/unwrap failure. Clippy will allow us to use "as" when we use bit masks to limit the size of an explicitly-sized value (u16, u32, u64). It doesn't work on usize values or on some more complicated expressions. Signed-off-by: Jonathan Browne <[email protected]>
1 parent c1b97a8 commit 9bc019d

File tree

8 files changed

+24
-11
lines changed

8 files changed

+24
-11
lines changed

src/seccompiler/src/backend.rs

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -279,7 +279,7 @@ impl SeccompCondition {
279279
/// [`SeccompCondition`]: struct.SeccompCondition.html
280280
fn value_segments(&self) -> (u32, u32, u8, u8) {
281281
// Splits the specified value into its most significant and least significant halves.
282-
let (msb, lsb) = ((self.value >> 32) as u32, self.value as u32);
282+
let (msb, lsb) = ((self.value >> 32) as u32, (self.value & 0xFFFFFFFF) as u32);
283283

284284
// Offset to the argument specified by `arg_number`.
285285
// Cannot overflow because the value will be at most 16 + 6 * 8 = 64.
@@ -431,8 +431,11 @@ impl SeccompCondition {
431431
fn into_masked_eq_bpf(self, offset: u8, mask: u64) -> Vec<sock_filter> {
432432
let (_, _, msb_offset, lsb_offset) = self.value_segments();
433433
let masked_value = self.value & mask;
434-
let (msb, lsb) = ((masked_value >> 32) as u32, masked_value as u32);
435-
let (mask_msb, mask_lsb) = ((mask >> 32) as u32, mask as u32);
434+
let (msb, lsb) = (
435+
(masked_value >> 32) as u32,
436+
(masked_value & 0xFFFFFFFF) as u32,
437+
);
438+
let (mask_msb, mask_lsb) = ((mask >> 32) as u32, (mask & 0xFFFFFFFF) as u32);
436439

437440
let mut bpf = match self.arg_len {
438441
SeccompCmpArgLen::Dword => vec![],

src/utils/src/lib.rs

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ pub mod time;
2121
pub mod validators;
2222
pub mod vm_memory;
2323

24+
use std::num::Wrapping;
2425
use std::result::Result;
2526

2627
/// Return the default page size of the platform, in bytes.
@@ -40,3 +41,9 @@ pub fn get_page_size() -> Result<usize, errno::Error> {
4041
pub const fn u64_to_usize(num: u64) -> usize {
4142
num as usize
4243
}
44+
45+
/// Converts a usize into a wrapping u32.
46+
#[inline]
47+
pub const fn wrap_usize_to_u32(num: usize) -> Wrapping<u32> {
48+
Wrapping(((num as u64) & 0xFFFFFFFF) as u32)
49+
}

src/vmm/src/arch/x86_64/gdt.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ fn get_base(entry: u64) -> u64 {
2525
}
2626

2727
fn get_limit(entry: u64) -> u32 {
28-
((((entry) & 0x000F_0000_0000_0000) >> 32) | ((entry) & 0x0000_0000_0000_FFFF)) as u32
28+
((((entry) & 0x000F_0000_0000_0000) >> 32) as u32) | (((entry) & 0x0000_0000_0000_FFFF) as u32)
2929
}
3030

3131
fn get_g(entry: u64) -> u8 {

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -182,7 +182,7 @@ impl DiskProperties {
182182
// The config space is little endian.
183183
let mut config = Vec::with_capacity(BLOCK_CONFIG_SPACE_SIZE);
184184
for i in 0..BLOCK_CONFIG_SPACE_SIZE {
185-
config.push((self.nsectors >> (8 * i)) as u8);
185+
config.push(((self.nsectors >> (8 * i)) & 0xff) as u8);
186186
}
187187
config
188188
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -128,7 +128,7 @@ pub trait VirtioDevice: AsAny + Send {
128128
let avail_features = self.avail_features();
129129
match page {
130130
// Get the lower 32-bits of the features bitfield.
131-
0 => avail_features as u32,
131+
0 => (avail_features & 0xFFFFFFFF) as u32,
132132
// Get the upper 32-bits of the features bitfield.
133133
1 => (avail_features >> 32) as u32,
134134
_ => {

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -288,7 +288,7 @@ impl MmioTransport {
288288
}
289289
0x24 => self.acked_features_select = v,
290290
0x30 => self.queue_select = v,
291-
0x38 => self.update_queue_field(|q| q.size = v as u16),
291+
0x38 => self.update_queue_field(|q| q.size = (v & 0xffff) as u16),
292292
0x44 => self.update_queue_field(|q| q.ready = v == 1),
293293
0x64 => {
294294
if self.check_device_status(device_status::DRIVER_OK, 0) {

src/vmm/src/devices/virtio/vsock/csm/connection.rs

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,7 @@ use std::time::{Duration, Instant};
8585
use log::{debug, error, info, warn};
8686
use utils::epoll::EventSet;
8787
use utils::vm_memory::{GuestMemoryError, GuestMemoryMmap, ReadVolatile, WriteVolatile};
88+
use utils::wrap_usize_to_u32;
8889

8990
use super::super::defs::uapi;
9091
use super::super::packet::VsockPacket;
@@ -475,7 +476,7 @@ where
475476
};
476477
0
477478
});
478-
self.fwd_cnt += Wrapping(flushed as u32);
479+
self.fwd_cnt += wrap_usize_to_u32(flushed);
479480
METRICS.vsock.tx_bytes_count.add(flushed as u64);
480481

481482
// If this connection was shutting down, but is waiting to drain the TX buffer
@@ -627,7 +628,8 @@ where
627628
}
628629
};
629630
// Move the "forwarded bytes" counter ahead by how much we were able to send out.
630-
self.fwd_cnt += Wrapping(written as u32);
631+
// Safe to unwrap because the maximum value is pkt.len(), which is a u32.
632+
self.fwd_cnt += wrap_usize_to_u32(written);
631633
METRICS.vsock.tx_bytes_count.add(written as u64);
632634

633635
// If we couldn't write the whole slice, we'll need to push the remaining data to our

src/vmm/src/devices/virtio/vsock/csm/txbuf.rs

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ use std::io::Write;
77
use std::num::Wrapping;
88

99
use utils::vm_memory::{BitmapSlice, Bytes, VolatileMemoryError, VolatileSlice, WriteVolatile};
10+
use utils::wrap_usize_to_u32;
1011

1112
use super::{defs, VsockCsmError};
1213

@@ -76,7 +77,7 @@ impl TxBuf {
7677

7778
// Either way, we've just pushed exactly `src.len()` bytes, so that's the amount by
7879
// which the (wrapping) buffer head needs to move forward.
79-
self.head += Wrapping(src.len() as u32);
80+
self.head += wrap_usize_to_u32(src.len());
8081

8182
Ok(())
8283
}
@@ -111,7 +112,7 @@ impl TxBuf {
111112
.map_err(VsockCsmError::TxBufFlush)?;
112113

113114
// Move the buffer tail ahead by the amount (of bytes) we were able to flush out.
114-
self.tail += Wrapping(written as u32);
115+
self.tail += wrap_usize_to_u32(written);
115116

116117
// If we weren't able to flush out as much as we tried, there's no point in attempting
117118
// our second write.

0 commit comments

Comments
 (0)