diff options
author | bors[bot] <26634292+bors[bot]@users.noreply.github.com> | 2023-07-17 22:26:39 +0000 |
---|---|---|
committer | GitHub <noreply@github.com> | 2023-07-17 22:26:39 +0000 |
commit | c375a102ac0af0d08c4942ffd81c84b6d6769d08 (patch) | |
tree | 6ebde1ce38d1646e0d1be66b5bfa09d1bd9530a6 /src/sys/socket/addr.rs | |
parent | 1d61638a72d91e5a5df03da4ab83182352a4d8e4 (diff) | |
parent | 2d60044196bbcf6c4da187f413e7ddd00f0a2ae0 (diff) | |
download | nix-c375a102ac0af0d08c4942ffd81c84b6d6769d08.zip |
Merge #2041master
2041: Set the length of a socket address when calling `recvmsg` on Linux r=asomers a=JarredAllen
# Background
I think I've found a bug with `recvmsg` on Linux where it doesn't set the length of a received socket address. I found this while working with unix datagram sockets with path `struct sockaddr_un` addresses, but I think this applies to any variable-length socket address type.
At present, the `sun_len` field of `UnixAddr` on Linux shows up as 0 whenever I call `recvmsg`. I think it's reading uninitialized memory (afaict the `recvmsg` function never initializes it before we call `MaybeUninit::assume_init`), though it's possible that it's being zero-initialized somewhere that I missed. Either way, this isn't the correct behavior, which should set it to the length of the `struct sockaddr_un` contents (or whatever other type of socket).
# Changes
I changed the `recvmsg` function to construct the returned socket address from the `struct sockaddr` and length returned by the libc `recvmsg` call (using the `from_raw` function the trait provides). Since the trait is sealed so downstream crates can't implement it, I believe this shouldn't be a breaking change.
# Validation
I've tested that my code (which failed due to this bug) now works. I also added a new test case which tests that we construct `UnixAddr`s correctly in the `recvmsg` function, which passes locally for me and fails if I cherry-pick it onto the current `master`.
I've also checked that `cargo test` and `cargo clippy` both still pass on `aarch64-apple-darwin` and on `aarch64-unknown-linux-musl` targets (the two targets I develop for/have access to). Hopefully your CI will confirm that everything else still works.
Co-authored-by: Jarred Allen <jarred@moveparallel.com>
Co-authored-by: Jarred Allen <jarredallen73@gmail.com>
Diffstat (limited to 'src/sys/socket/addr.rs')
-rw-r--r-- | src/sys/socket/addr.rs | 53 |
1 files changed, 51 insertions, 2 deletions
diff --git a/src/sys/socket/addr.rs b/src/sys/socket/addr.rs index ff222b5e..1d86e756 100644 --- a/src/sys/socket/addr.rs +++ b/src/sys/socket/addr.rs @@ -765,6 +765,22 @@ impl SockaddrLike for UnixAddr { { mem::size_of::<libc::sockaddr_un>() as libc::socklen_t } + + unsafe fn set_length(&mut self, new_length: usize) -> std::result::Result<(), SocketAddressLengthNotDynamic> { + // `new_length` is only used on some platforms, so it must be provided even when not used + #![allow(unused_variables)] + cfg_if! { + if #[cfg(any(target_os = "android", + target_os = "fuchsia", + target_os = "illumos", + target_os = "linux", + target_os = "redox", + ))] { + self.sun_len = new_length as u8; + } + }; + Ok(()) + } } impl AsRef<libc::sockaddr_un> for UnixAddr { @@ -914,7 +930,32 @@ pub trait SockaddrLike: private::SockaddrLikePriv { { mem::size_of::<Self>() as libc::socklen_t } + + /// Set the length of this socket address + /// + /// This method may only be called on socket addresses whose lengths are dynamic, and it + /// returns an error if called on a type whose length is static. + /// + /// # Safety + /// + /// `new_length` must be a valid length for this type of address. Specifically, reads of that + /// length from `self` must be valid. + #[doc(hidden)] + unsafe fn set_length(&mut self, _new_length: usize) -> std::result::Result<(), SocketAddressLengthNotDynamic> { + Err(SocketAddressLengthNotDynamic) + } +} + +/// The error returned by [`SockaddrLike::set_length`] on an address whose length is statically +/// fixed. +#[derive(Copy, Clone, Debug)] +pub struct SocketAddressLengthNotDynamic; +impl fmt::Display for SocketAddressLengthNotDynamic { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.write_str("Attempted to set length on socket whose length is statically fixed") + } } +impl std::error::Error for SocketAddressLengthNotDynamic {} impl private::SockaddrLikePriv for () { fn as_mut_ptr(&mut self) -> *mut libc::sockaddr { @@ -1360,6 +1401,15 @@ impl SockaddrLike for SockaddrStorage { None => mem::size_of_val(self) as libc::socklen_t, } } + + unsafe fn set_length(&mut self, new_length: usize) -> std::result::Result<(), SocketAddressLengthNotDynamic> { + match self.as_unix_addr_mut() { + Some(addr) => { + addr.set_length(new_length) + }, + None => Err(SocketAddressLengthNotDynamic), + } + } } macro_rules! accessors { @@ -1678,7 +1728,7 @@ impl PartialEq for SockaddrStorage { } } -mod private { +pub(super) mod private { pub trait SockaddrLikePriv { /// Returns a mutable raw pointer to the inner structure. /// @@ -2215,7 +2265,6 @@ mod datalink { &self.0 } } - } } |