Skip to content

Commit

Permalink
Exclude NUL characters from OSStrings returned by getsockopt
Browse files Browse the repository at this point in the history
On FreeBSD, getsockopt appends a single NUL to the returned string.  On
Linux, it pads the entire buffer with NULs.  But both of those behaviors
may be version-dependent.  Adjust GetOsString to handle any number of
NUL characters.
  • Loading branch information
asomers committed Dec 5, 2024
1 parent e7e9809 commit 987e586
Show file tree
Hide file tree
Showing 3 changed files with 23 additions and 9 deletions.
1 change: 1 addition & 0 deletions changelog/2557.fixed.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Properly exclude NUL characters from `OSString`s returned by `getsockopt`.
12 changes: 9 additions & 3 deletions src/sys/socket/sockopt.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,8 @@ use crate::{errno::Errno, Result};
use cfg_if::cfg_if;
use libc::{self, c_int, c_void, socklen_t};
#[cfg(apple_targets)]
use std::ffi::{CStr, CString};
use std::ffi::{OsStr, OsString};
use std::ffi::CString;
use std::ffi::{CStr, OsStr, OsString};
use std::mem::{self, MaybeUninit};
use std::os::unix::ffi::OsStrExt;
#[cfg(linux_android)]
Expand Down Expand Up @@ -1745,7 +1745,13 @@ impl<T: AsMut<[u8]>> Get<OsString> for GetOsString<T> {
unsafe fn assume_init(self) -> OsString {
let len = self.len as usize;
let mut v = unsafe { self.val.assume_init() };
OsStr::from_bytes(&v.as_mut()[0..len]).to_owned()
if let Ok(cs) = CStr::from_bytes_until_nul(&v.as_mut()[0..len]) {
OsStr::from_bytes(cs.to_bytes())
} else {
// The OS returned a non-NUL-terminated string
OsStr::from_bytes(&v.as_mut()[0..len])
}
.to_owned()
}
}

Expand Down
19 changes: 13 additions & 6 deletions test/sys/test_sockopt.rs
Original file line number Diff line number Diff line change
Expand Up @@ -249,14 +249,13 @@ fn test_so_type_unknown() {
}

// The CI doesn't supported getsockopt and setsockopt on emulated processors.
// It's believed that a QEMU issue, the tests run ok on a fully emulated system.
// Current CI just run the binary with QEMU but the Kernel remains the same as the host.
// It's believed to be a QEMU issue; the tests run ok on a fully emulated
// system. Current CI just runs the binary with QEMU but the kernel remains the
// same as the host.
// So the syscall doesn't work properly unless the kernel is also emulated.
#[test]
#[cfg(all(
any(target_arch = "x86", target_arch = "x86_64"),
any(target_os = "freebsd", target_os = "linux")
))]
#[cfg(any(target_os = "freebsd", target_os = "linux"))]
#[cfg_attr(qemu, ignore)]
fn test_tcp_congestion() {
use std::ffi::OsString;

Expand All @@ -269,6 +268,14 @@ fn test_tcp_congestion() {
.unwrap();

let val = getsockopt(&fd, sockopt::TcpCongestion).unwrap();
let bytes = val.as_os_str().as_encoded_bytes();
for b in bytes.iter() {
assert_ne!(
*b, 0,
"OsString should contain no embedded NULs: {:?}",
val
);
}
setsockopt(&fd, sockopt::TcpCongestion, &val).unwrap();

setsockopt(
Expand Down

0 comments on commit 987e586

Please sign in to comment.