Skip to content

Commit 5ab4e26

Browse files
committed
Do not align cmsg access on first header
The implementation of CmsgInterator did not correctly mirror the way the CMSG_FIRSTHDR and CMSG_NEXTHDR macros work in C. CMSG_FIRSTHDR does not attempt to align access since the pointer is already aligned due to being part of the msghdr struct. CmsgInterator was always aligning access which happened to work on all platforms except OpenBSD where the use of alignment was adding unexpected bytes to the expected size and causing the `cmsg_align(cmsg_len) > self.buf.len()` guard clause to return early.
1 parent 1c4dcd9 commit 5ab4e26

File tree

2 files changed

+24
-11
lines changed

2 files changed

+24
-11
lines changed

src/sys/socket/ffi.rs

+4-3
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,10 @@
33

44
pub use libc::{socket, listen, bind, accept, connect, setsockopt, sendto, recvfrom, getsockname, getpeername, recv, send};
55

6-
use libc::{c_int, c_void, socklen_t, size_t, ssize_t};
6+
use libc::{c_int, c_void, socklen_t, ssize_t};
7+
8+
#[cfg(not(target_os = "macos"))]
9+
use libc::size_t;
710

811
#[cfg(not(target_os = "linux"))]
912
use libc::c_uint;
@@ -29,8 +32,6 @@ pub type type_of_msg_iovlen = size_t;
2932
#[cfg(not(target_os = "linux"))]
3033
pub type type_of_msg_iovlen = c_uint;
3134

32-
33-
3435
// Private because we don't expose any external functions that operate
3536
// directly on this type; we just use it internally at FFI boundaries.
3637
// Note that in some cases we store pointers in *const fields that the

src/sys/socket/mod.rs

+20-8
Original file line numberDiff line numberDiff line change
@@ -204,11 +204,17 @@ impl<'a> RecvMsg<'a> {
204204
/// Iterate over the valid control messages pointed to by this
205205
/// msghdr.
206206
pub fn cmsgs(&self) -> CmsgIterator {
207-
CmsgIterator(self.cmsg_buffer)
207+
CmsgIterator {
208+
buf: self.cmsg_buffer,
209+
next: 0
210+
}
208211
}
209212
}
210213

211-
pub struct CmsgIterator<'a>(&'a [u8]);
214+
pub struct CmsgIterator<'a> {
215+
buf: &'a [u8],
216+
next: usize,
217+
}
212218

213219
impl<'a> Iterator for CmsgIterator<'a> {
214220
type Item = ControlMessage<'a>;
@@ -217,12 +223,11 @@ impl<'a> Iterator for CmsgIterator<'a> {
217223
// although we handle the invariants in slightly different places to
218224
// get a better iterator interface.
219225
fn next(&mut self) -> Option<ControlMessage<'a>> {
220-
let buf = self.0;
221226
let sizeof_cmsghdr = mem::size_of::<cmsghdr>();
222-
if buf.len() < sizeof_cmsghdr {
227+
if self.buf.len() < sizeof_cmsghdr {
223228
return None;
224229
}
225-
let cmsg: &cmsghdr = unsafe { mem::transmute(buf.as_ptr()) };
230+
let cmsg: &cmsghdr = unsafe { mem::transmute(self.buf.as_ptr()) };
226231

227232
// This check is only in the glibc implementation of CMSG_NXTHDR
228233
// (although it claims the kernel header checks this), but such
@@ -232,13 +237,20 @@ impl<'a> Iterator for CmsgIterator<'a> {
232237
return None;
233238
}
234239
let len = cmsg_len - sizeof_cmsghdr;
240+
let aligned_cmsg_len = if self.next == 0 {
241+
// CMSG_FIRSTHDR
242+
cmsg_len
243+
} else {
244+
// CMSG_NXTHDR
245+
cmsg_align(cmsg_len)
246+
};
235247

236248
// Advance our internal pointer.
237-
if cmsg_align(cmsg_len) > buf.len() {
238-
println!("cmsg_align(cmsg_len) > buf.len(): {} > {}", cmsg_align(cmsg_len), buf.len());
249+
if aligned_cmsg_len > self.buf.len() {
239250
return None;
240251
}
241-
self.0 = &buf[cmsg_align(cmsg_len)..];
252+
self.buf = &self.buf[aligned_cmsg_len..];
253+
self.next += 1;
242254

243255
match (cmsg.cmsg_level, cmsg.cmsg_type) {
244256
(libc::SOL_SOCKET, libc::SCM_RIGHTS) => unsafe {

0 commit comments

Comments
 (0)