Skip to content

Commit bdfc5a0

Browse files
roypatJonathanWoollett-Light
authored andcommitted
test: test_utils mock should not trigger undefined behavior
The TapTrafficSimulator code in test_utils, which is used for some of our unittests, but not in production, was triggering undefined behavior due to mutating through an immutable reference, and returning an "owned" area of stack local memory. This commit fixes those instances of UB. Signed-off-by: Patrick Roy <roypat@amazon.co.uk>
1 parent 6e57e8c commit bdfc5a0

File tree

1 file changed

+17
-5
lines changed

1 file changed

+17
-5
lines changed

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

Lines changed: 17 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -151,10 +151,13 @@ impl TapTrafficSimulator {
151151
pub fn new(tap_index: i32) -> Self {
152152
// Create sockaddr_ll struct.
153153
// SAFETY: sockaddr_storage has no invariants and can be safely zeroed.
154-
let send_addr_ptr = &unsafe { mem::zeroed() } as *const libc::sockaddr_storage;
154+
let mut storage: libc::sockaddr_storage = unsafe { mem::zeroed() };
155+
156+
let send_addr_ptr = &mut storage as *mut libc::sockaddr_storage;
157+
155158
// SAFETY: `sock_addr` is a valid pointer and safe to derference.
156159
unsafe {
157-
let sock_addr: *mut libc::sockaddr_ll = send_addr_ptr as *mut libc::sockaddr_ll;
160+
let sock_addr: *mut libc::sockaddr_ll = send_addr_ptr.cast::<libc::sockaddr_ll>();
158161
(*sock_addr).sll_family = libc::AF_PACKET as libc::sa_family_t;
159162
(*sock_addr).sll_protocol = (libc::ETH_P_ALL as u16).to_be();
160163
(*sock_addr).sll_halen = libc::ETH_ALEN as u8;
@@ -184,9 +187,18 @@ impl TapTrafficSimulator {
184187

185188
Self {
186189
socket,
187-
// SAFETY: Both the cast and the dereference are safe because the point is valid
188-
// and sockaddr_storage is meant to be cast that way.
189-
send_addr: unsafe { *(send_addr_ptr.cast()) },
190+
// SAFETY: size_of::<libc::sockaddr_storage>() is greater than
191+
// sizeof::<libc::sockaddr_ll>(), so to return an owned value of sockaddr_ll
192+
// from the stack-local libc::sockaddr_storage that we have, we need to
193+
// 1. Create a zeroed out libc::sockaddr_ll,
194+
// 2. Copy over the first size_of::<libc::sockaddr_ll>() bytes into the struct we
195+
// want to return
196+
// We cannot simply return "*(send_addr_ptr as *const libc::sockaddr_ll)", as this
197+
// would return a reference to a variable that lives in the stack frame of the current
198+
// function, and which will no longer be valid after returning.
199+
// transmute_copy does all this for us.
200+
// Note that this is how these structures are intended to be used in C.
201+
send_addr: unsafe { mem::transmute_copy(&storage) },
190202
}
191203
}
192204

0 commit comments

Comments
 (0)