Skip to content

Commit

Permalink
Fix potential unaligned reads
Browse files Browse the repository at this point in the history
  • Loading branch information
operutka committed Aug 6, 2024
1 parent 1b8ee40 commit 5d7a27c
Show file tree
Hide file tree
Showing 14 changed files with 56 additions and 28 deletions.
6 changes: 2 additions & 4 deletions src/exports/mem.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,26 +19,24 @@ use std::os::raw::c_void;

/// Allocate a block of memory with a given size.
#[no_mangle]
#[allow(clippy::cast_ptr_alignment)]
pub unsafe extern "C" fn ac__malloc(size: usize) -> *mut c_void {
let layout_size = std::mem::size_of::<Layout>();
let layout = Layout::from_size_align_unchecked(layout_size + size, 1);
let block = std::alloc::alloc(layout);

*(block as *mut Layout) = layout;
std::ptr::write_unaligned(block as *mut Layout, layout);

block.add(layout_size) as _
}

/// Free a given block of memory.
#[no_mangle]
#[allow(clippy::cast_ptr_alignment)]
pub unsafe extern "C" fn ac__free(ptr: *mut c_void) {
let ptr = ptr as *mut u8;

let layout_size = std::mem::size_of::<Layout>();
let block = ptr.sub(layout_size);
let layout = *(block as *mut Layout);
let layout = std::ptr::read_unaligned(block as *const Layout);

std::alloc::dealloc(block, layout);
}
3 changes: 2 additions & 1 deletion src/net/arrow/proto/msg/control/ack.rs
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,8 @@ impl FromBytes for AckMessage {
}

let ptr = bytes.as_ptr() as *const Self;
let msg = unsafe { &*ptr };

let msg = unsafe { ptr.read_unaligned() };

let res = Self {
err: u32::from_be(msg.err),
Expand Down
3 changes: 2 additions & 1 deletion src/net/arrow/proto/msg/control/connect.rs
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,8 @@ impl FromBytes for ConnectMessage {
}

let ptr = bytes.as_ptr() as *const Self;
let msg = unsafe { &*ptr };

let msg = unsafe { ptr.read_unaligned() };

let res = Self {
service_id: u16::from_be(msg.service_id),
Expand Down
3 changes: 2 additions & 1 deletion src/net/arrow/proto/msg/control/data_ack.rs
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,8 @@ impl FromBytes for DataAckMessage {
}

let ptr = bytes.as_ptr() as *const Self;
let msg = unsafe { &*ptr };

let msg = unsafe { ptr.read_unaligned() };

let res = Self {
session_id: u32::from_be(msg.session_id),
Expand Down
3 changes: 2 additions & 1 deletion src/net/arrow/proto/msg/control/hup.rs
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,8 @@ impl FromBytes for HupMessage {
}

let ptr = bytes.as_ptr() as *const Self;
let msg = unsafe { &*ptr };

let msg = unsafe { ptr.read_unaligned() };

let res = Self {
session_id: u32::from_be(msg.session_id),
Expand Down
3 changes: 2 additions & 1 deletion src/net/arrow/proto/msg/control/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,8 @@ impl FromBytes for ControlMessageHeader {
assert_eq!(bytes.len(), mem::size_of::<Self>());

let ptr = bytes.as_ptr() as *const Self;
let header = unsafe { &*ptr };

let header = unsafe { ptr.read_unaligned() };

let header = Self {
msg_id: u16::from_be(header.msg_id),
Expand Down
2 changes: 1 addition & 1 deletion src/net/raw/arp.rs
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ impl ArpPacket {
let ptr = data.as_ptr();
let ptr = ptr as *const RawArpPacketHeader;

let rh = unsafe { &*ptr };
let rh = unsafe { ptr.read_unaligned() };

let hlen = rh.hlen as usize;
let plen = rh.plen as usize;
Expand Down
2 changes: 1 addition & 1 deletion src/net/raw/ether/packet.rs
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ impl EtherPacketHeader {
let ptr = data.as_ptr();
let ptr = ptr as *const RawEtherPacketHeader;

let rh = unsafe { &*ptr };
let rh = unsafe { ptr.read_unaligned() };

Self {
src: MacAddr::from_slice(&rh.src),
Expand Down
2 changes: 1 addition & 1 deletion src/net/raw/icmp.rs
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ impl IcmpPacket {
let ptr = data.as_ptr();
let ptr = ptr as *const RawIcmpPacketHeader;

let rh = unsafe { &*ptr };
let rh = unsafe { ptr.read_unaligned() };

let body = &data[size..];

Expand Down
8 changes: 5 additions & 3 deletions src/net/raw/ip.rs
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ impl Ipv4PacketHeader {
} else {
let ptr = data.as_ptr();

let rh = unsafe { &*(ptr as *const RawIpv4PacketHeader) };
let rh = unsafe { std::ptr::read_unaligned(ptr as *const RawIpv4PacketHeader) };

let flags_foffset = u16::from_be(rh.flags_foffset);
let ihl = rh.vihl & 0x0f;
Expand All @@ -109,9 +109,11 @@ impl Ipv4PacketHeader {
"unable to parse IPv4 packet, not enough data",
))
} else {
#[allow(clippy::cast_ptr_alignment)]
let options = unsafe {
utils::vec_from_raw_parts(ptr.offset(offset_1) as *const u32, options_len)
utils::vec_from_raw_parts_unaligned(
ptr.offset(offset_1) as *const u32,
options_len,
)
};

let res = Ipv4PacketHeader {
Expand Down
14 changes: 8 additions & 6 deletions src/net/raw/tcp.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@
use std::io;
use std::mem;
use std::slice;

use std::io::Write;

Expand Down Expand Up @@ -78,7 +77,7 @@ impl TcpPacket {
} else {
let ptr = data.as_ptr();

let rh = unsafe { &*(ptr as *const RawTcpPacketHeader) };
let rh = unsafe { std::ptr::read_unaligned(ptr as *const RawTcpPacketHeader) };

let doffset_flags = u16::from_be(rh.doffset_flags);
let doffset = doffset_flags >> 12;
Expand All @@ -92,9 +91,12 @@ impl TcpPacket {
"unable to parse TCP packet, not enough data",
))
} else {
#[allow(clippy::cast_ptr_alignment)]
let options =
unsafe { slice::from_raw_parts(ptr.add(offset_1) as *const u32, options_len) };
let options = unsafe {
utils::vec_from_raw_parts_unaligned(
ptr.add(offset_1) as *const u32,
options_len,
)
};

let payload = &data[offset_2..];

Expand All @@ -106,7 +108,7 @@ impl TcpPacket {
flags: doffset_flags & 0x01ff,
wsize: u16::from_be(rh.wsize),
uptr: u16::from_be(rh.uptr),
options: options.to_vec().into_boxed_slice(),
options: options.into_boxed_slice(),
data: payload.to_vec().into_boxed_slice(),
};

Expand Down
15 changes: 10 additions & 5 deletions src/net/raw/utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,15 +49,20 @@ pub fn sum_slice<T: Sized>(data: &[T]) -> u32 {

/// Sum given raw data as 16-bit unsigned big endian numbers.
#[allow(clippy::missing_safety_doc)]
#[allow(clippy::cast_ptr_alignment)]
pub unsafe fn sum_raw_be(data: *const u8, size: usize) -> u32 {
let sdata = slice::from_raw_parts(data as *const u16, size >> 1);
let slice = slice::from_raw_parts(data, size);
let mut sum: u32 = 0;
for w in sdata {
sum = sum.wrapping_add(u16::from_be(*w) as u32);

let count = size >> 1;

for idx in 0..count {
let ptr = data.add(idx << 1) as *const u16;
let val = u16::from_be(ptr.read_unaligned());

sum = sum.wrapping_add(val as u32);
}

let slice = slice::from_raw_parts(data, size);

if (size & 0x01) != 0 {
sum.wrapping_add((slice[size - 1] as u32) << 8)
} else {
Expand Down
2 changes: 1 addition & 1 deletion src/net/tls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ impl From<SslErrorStack> for TlsError {
thread_local! {
/// An async context set when entering an async function to be later used
/// by the IO methods within the `InnerSslStream`.
static ASYNC_CONTEXT: RefCell<Option<Waker>> = RefCell::new(None);
static ASYNC_CONTEXT: RefCell<Option<Waker>> = const { RefCell::new(None) };
}

/// A struct that will remove the async context when dropped.
Expand Down
18 changes: 17 additions & 1 deletion src/utils/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -95,11 +95,27 @@ pub fn slice_as_bytes<T: Sized>(data: &[T]) -> &[u8] {
///
/// # Safety
/// The given pointer MUST point to an array that contains at least `len`
/// elements. Each element is expected to be of size T.
/// elements. Each element is expected to be of size T. The pointer must meet
/// the alignment requirements for T.
pub unsafe fn vec_from_raw_parts<T: Clone>(ptr: *const T, len: usize) -> Vec<T> {
slice::from_raw_parts(ptr, len).to_vec()
}

/// Convert a given typed pointer into a new vector (copying the data).
///
/// # Safety
/// The given pointer MUST point to an array that contains at least `len`
/// elements. Each element is expected to be of size T.
pub unsafe fn vec_from_raw_parts_unaligned<T: Copy>(ptr: *const T, len: usize) -> Vec<T> {
let mut res = Vec::with_capacity(len);

for idx in 0..len {
res.push(std::ptr::read_unaligned(ptr.add(idx)));
}

res
}

/// Convert a given C-string pointer to a new instance of String.
///
/// # Safety
Expand Down

0 comments on commit 5d7a27c

Please sign in to comment.