From 2a51f584e869502945098bbbbc4bda754e5f99b0 Mon Sep 17 00:00:00 2001 From: gibbz00 Date: Fri, 26 Dec 2025 12:30:32 +0100 Subject: [PATCH 1/7] linux, l4re: address soundness issues of `CMSG_NXTHDR` This change makes sure that the header of `next` is within max, returning null if not. This is similar to how `glibc` does it. No checks were previously being done to assert that `next as usize + size_of::() < max`. Wrapping offset calculations could then lead to buffer over-reads in the following `(*next).cmsg_len`. [glibc ref](https://github.com/bminor/glibc/blob/b71d59074b98ad4abd23c136ec9ad4c26e29ee6d/sysdeps/unix/sysv/linux/cmsg_nxthdr.c#L49-L51) --- src/unix/linux_like/linux_l4re_shared.rs | 24 +++++++++++++++++------- 1 file changed, 17 insertions(+), 7 deletions(-) diff --git a/src/unix/linux_like/linux_l4re_shared.rs b/src/unix/linux_like/linux_l4re_shared.rs index bd3cfafeb6e72..ea958db979203 100644 --- a/src/unix/linux_like/linux_l4re_shared.rs +++ b/src/unix/linux_like/linux_l4re_shared.rs @@ -1493,15 +1493,25 @@ f! { if ((*cmsg).cmsg_len as usize) < size_of::() { return core::ptr::null_mut::(); } - let next = - (cmsg as usize + super::CMSG_ALIGN((*cmsg).cmsg_len as usize)) as *mut crate::cmsghdr; - let max = (*mhdr).msg_control as usize + (*mhdr).msg_controllen as usize; - if (next.wrapping_offset(1)) as usize > max - || next as usize + super::CMSG_ALIGN((*next).cmsg_len as usize) > max - { + + // FIXME(msrv): `.wrapping_byte_add()` stabilized in 1.75 + let next_cmsg = cmsg + .cast::() + .wrapping_add(super::CMSG_ALIGN((*cmsg).cmsg_len as usize)) + .cast::(); + + // In case the addition wrapped. `next_addr > max_addr` + // would otherwise not work as intended. + if (next_cmsg as usize) < (cmsg as usize) { + return core::ptr::null_mut(); + } + + let max_addr = (*mhdr).msg_control as usize + (*mhdr).msg_controllen as usize; + + if next_cmsg as usize + size_of::() > max_addr { core::ptr::null_mut::() } else { - next + next_cmsg as *mut crate::cmsghdr } } From 58304730f65fd3e0b22d87923c3e3033018d9a20 Mon Sep 17 00:00:00 2001 From: gibbz00 Date: Sat, 27 Dec 2025 15:12:18 +0100 Subject: [PATCH 2/7] Remove redundant CMSG_NXTHDR test assertions. Likely written to make assertions in the unsound CMSG_NXTHDR implementations introduced in #1235. CMSG_NXTHDR(mhdr, current_cmsghdr) should not be concerned with the value next_cmsghdr.cmsg_len, which the previous implementation did. --- libc-test/tests/cmsg.rs | 21 ++++++--------------- 1 file changed, 6 insertions(+), 15 deletions(-) diff --git a/libc-test/tests/cmsg.rs b/libc-test/tests/cmsg.rs index b9573e1d040af..7ec282adc75f9 100644 --- a/libc-test/tests/cmsg.rs +++ b/libc-test/tests/cmsg.rs @@ -79,21 +79,12 @@ mod t { if cfg!(target_os = "aix") && cmsg_len % std::mem::size_of::() != 0 { continue; } - for next_cmsg_len in 0..32 { - unsafe { - pcmsghdr.cast::().write_bytes(0, CAPACITY); - (*pcmsghdr).cmsg_len = cmsg_len as _; - let libc_next = libc::CMSG_NXTHDR(&mhdr, pcmsghdr); - let next = cmsg_nxthdr(&mhdr, pcmsghdr); - assert_eq!(libc_next, next); - - if !libc_next.is_null() { - (*libc_next).cmsg_len = next_cmsg_len; - let libc_next = libc::CMSG_NXTHDR(&mhdr, pcmsghdr); - let next = cmsg_nxthdr(&mhdr, pcmsghdr); - assert_eq!(libc_next, next); - } - } + unsafe { + pcmsghdr.cast::().write_bytes(0, CAPACITY); + (*pcmsghdr).cmsg_len = cmsg_len as _; + let libc_next = libc::CMSG_NXTHDR(&mhdr, pcmsghdr); + let next = cmsg_nxthdr(&mhdr, pcmsghdr); + assert_eq!(libc_next, next); } } } From 9d54e8da8df6e9d1195acd8ea05213cb387e29e7 Mon Sep 17 00:00:00 2001 From: gibbz00 Date: Sat, 27 Dec 2025 15:33:01 +0100 Subject: [PATCH 3/7] Properly set `cmsg_len` in `CMSG_NXTHDR` tests. Setting `(*pcmsghdr).cmsg_len = cmsg_len as _;` when cmsg_len ranges from 0 to 64 is invalid as it must always be `>= size_of::()`, rounded up to the nearest alignment boundary. Some implementations (notably glbic) do check that `cmsg_len >= size_of::()` in `CMSG_NXTHDR`, returning null if so. But this is more so an extra precaution that is not mentioned in the POSIX 1003.1-2024. It can therefore not be relied on for tests executed on multiple platforms. The change also removes the ignoring of some testvalues when targeting AIX. --- libc-test/tests/cmsg.rs | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/libc-test/tests/cmsg.rs b/libc-test/tests/cmsg.rs index 7ec282adc75f9..99fa7c1ebc276 100644 --- a/libc-test/tests/cmsg.rs +++ b/libc-test/tests/cmsg.rs @@ -74,14 +74,10 @@ mod t { let pcmsghdr = buffer.0.as_mut_ptr().cast::(); mhdr.msg_control = pcmsghdr.cast::(); mhdr.msg_controllen = (160 - start_ofs) as _; - for cmsg_len in 0..64 { - // Address must be a multiple of 0x4 for testing on AIX. - if cfg!(target_os = "aix") && cmsg_len % std::mem::size_of::() != 0 { - continue; - } + for cmsg_payload_len in 0..64 { unsafe { pcmsghdr.cast::().write_bytes(0, CAPACITY); - (*pcmsghdr).cmsg_len = cmsg_len as _; + (*pcmsghdr).cmsg_len = libc::CMSG_LEN(cmsg_payload_len as _) as _; let libc_next = libc::CMSG_NXTHDR(&mhdr, pcmsghdr); let next = cmsg_nxthdr(&mhdr, pcmsghdr); assert_eq!(libc_next, next); From e40b520f9c7e27edd7e6c38a2afaa7640dbcc332 Mon Sep 17 00:00:00 2001 From: gibbz00 Date: Sat, 27 Dec 2025 15:38:51 +0100 Subject: [PATCH 4/7] sparc64: remove ignore for `CMSG_NXTHDR` tests --- libc-test/tests/cmsg.rs | 5 ----- 1 file changed, 5 deletions(-) diff --git a/libc-test/tests/cmsg.rs b/libc-test/tests/cmsg.rs index 99fa7c1ebc276..b03e0729601a5 100644 --- a/libc-test/tests/cmsg.rs +++ b/libc-test/tests/cmsg.rs @@ -17,8 +17,6 @@ mod t { extern "C" { pub fn cmsg_firsthdr(msgh: *const msghdr) -> *mut cmsghdr; - // see below - #[cfg(not(target_arch = "sparc64"))] pub fn cmsg_nxthdr(mhdr: *const msghdr, cmsg: *const cmsghdr) -> *mut cmsghdr; pub fn cmsg_space(length: c_uint) -> usize; pub fn cmsg_len(length: c_uint) -> usize; @@ -58,9 +56,6 @@ mod t { } } - // Skip on sparc64 - // https://github.com/rust-lang/libc/issues/1239 - #[cfg(not(target_arch = "sparc64"))] #[test] fn test_cmsg_nxthdr() { // Helps to align the buffer on the stack. From a769d7e136e5fa2d669d9ab13a91b05c44df9612 Mon Sep 17 00:00:00 2001 From: gibbz00 Date: Sat, 27 Dec 2025 16:19:30 +0100 Subject: [PATCH 5/7] Test msghdr.controllen boundary behaviour for `CMSG_NXTHDR` --- libc-test/tests/cmsg.rs | 35 +++++++++++++++++++++++++++-------- 1 file changed, 27 insertions(+), 8 deletions(-) diff --git a/libc-test/tests/cmsg.rs b/libc-test/tests/cmsg.rs index b03e0729601a5..a5bd2f8c06cae 100644 --- a/libc-test/tests/cmsg.rs +++ b/libc-test/tests/cmsg.rs @@ -64,18 +64,37 @@ mod t { const CAPACITY: usize = 512; let mut buffer = Align8([0_u8; CAPACITY]); + let pcmsghdr = buffer.0.as_mut_ptr().cast::(); + let mut mhdr: msghdr = unsafe { mem::zeroed() }; - for start_ofs in 0..64 { - let pcmsghdr = buffer.0.as_mut_ptr().cast::(); - mhdr.msg_control = pcmsghdr.cast::(); - mhdr.msg_controllen = (160 - start_ofs) as _; + mhdr.msg_control = pcmsghdr.cast::(); + + for trunc in 0..64 { + mhdr.msg_controllen = (160 - trunc) as _; + for cmsg_payload_len in 0..64 { + let mut current_cmsghdr_ptr = pcmsghdr; + assert!(!current_cmsghdr_ptr.is_null()); + + // Go from first cmsghdr to the last (until null) using various + // cmsg_len increments. `cmsg_len` is set by us to check that + // the jump to the next cmsghdr is correct with respect to + // alignment and payload padding. + while !current_cmsghdr_ptr.is_null() { + unsafe { + (*current_cmsghdr_ptr).cmsg_len = + libc::CMSG_LEN(cmsg_payload_len as _) as _; + + let libc_next = libc::CMSG_NXTHDR(&mhdr, current_cmsghdr_ptr); + let system_next = cmsg_nxthdr(&mhdr, current_cmsghdr_ptr); + assert_eq!(libc_next, system_next); + + current_cmsghdr_ptr = libc_next; + } + } + unsafe { pcmsghdr.cast::().write_bytes(0, CAPACITY); - (*pcmsghdr).cmsg_len = libc::CMSG_LEN(cmsg_payload_len as _) as _; - let libc_next = libc::CMSG_NXTHDR(&mhdr, pcmsghdr); - let next = cmsg_nxthdr(&mhdr, pcmsghdr); - assert_eq!(libc_next, next); } } } From b5903bad01ac66402628d46927af382f0b5620b7 Mon Sep 17 00:00:00 2001 From: gibbz00 Date: Sat, 27 Dec 2025 16:49:43 +0100 Subject: [PATCH 6/7] Add some context to `CMSG_NXTHDR` test assertions. --- libc-test/tests/cmsg.rs | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/libc-test/tests/cmsg.rs b/libc-test/tests/cmsg.rs index a5bd2f8c06cae..2295e78aae1f5 100644 --- a/libc-test/tests/cmsg.rs +++ b/libc-test/tests/cmsg.rs @@ -75,6 +75,7 @@ mod t { for cmsg_payload_len in 0..64 { let mut current_cmsghdr_ptr = pcmsghdr; assert!(!current_cmsghdr_ptr.is_null()); + let mut count = 0; // Go from first cmsghdr to the last (until null) using various // cmsg_len increments. `cmsg_len` is set by us to check that @@ -87,9 +88,14 @@ mod t { let libc_next = libc::CMSG_NXTHDR(&mhdr, current_cmsghdr_ptr); let system_next = cmsg_nxthdr(&mhdr, current_cmsghdr_ptr); - assert_eq!(libc_next, system_next); + assert_eq!( + system_next, libc_next, + "msg_crontrollen: {}, payload_len: {}, count: {}", + mhdr.msg_controllen, cmsg_payload_len, count + ); current_cmsghdr_ptr = libc_next; + count += 1; } } From f624d2381015cb859b53c8462f8d7d6a3975ba47 Mon Sep 17 00:00:00 2001 From: gibbz00 Date: Sat, 3 Jan 2026 13:43:50 +0100 Subject: [PATCH 7/7] linux, emscripten, android, l4re: handle zero-sized payload differences in CMSG_NXTHDR musl and its descendants check `next_addr >= max_addr` whilst the rest do `next_addr > max_addr`. This was previously not reflected in the implementations, coming to light only after testing was extended to execute at the controllen boundary. [musl_ref]: https://www.openwall.com/lists/musl/2025/12/27/1 --- src/unix/linux_like/emscripten/mod.rs | 2 +- src/unix/linux_like/linux_l4re_shared.rs | 9 ++++++++- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/src/unix/linux_like/emscripten/mod.rs b/src/unix/linux_like/emscripten/mod.rs index 35d7001ff5f59..711ed8f8b4c37 100644 --- a/src/unix/linux_like/emscripten/mod.rs +++ b/src/unix/linux_like/emscripten/mod.rs @@ -1257,7 +1257,7 @@ f! { } let next = (cmsg as usize + super::CMSG_ALIGN((*cmsg).cmsg_len as usize)) as *mut cmsghdr; let max = (*mhdr).msg_control as usize + (*mhdr).msg_controllen as usize; - if (next.offset(1)) as usize > max { + if (next.offset(1)) as usize >= max { core::ptr::null_mut::() } else { next as *mut cmsghdr diff --git a/src/unix/linux_like/linux_l4re_shared.rs b/src/unix/linux_like/linux_l4re_shared.rs index ea958db979203..d4bbfbfbf7736 100644 --- a/src/unix/linux_like/linux_l4re_shared.rs +++ b/src/unix/linux_like/linux_l4re_shared.rs @@ -1506,7 +1506,14 @@ f! { return core::ptr::null_mut(); } - let max_addr = (*mhdr).msg_control as usize + (*mhdr).msg_controllen as usize; + let mut max_addr = (*mhdr).msg_control as usize + (*mhdr).msg_controllen as usize; + + if cfg!(any(target_env = "musl", target_env = "ohos")) { + // musl and some of its descendants do `>= max_addr` + // comparisons in the if statement below. + // https://www.openwall.com/lists/musl/2025/12/27/1 + max_addr -= 1; + } if next_cmsg as usize + size_of::() > max_addr { core::ptr::null_mut::()