Skip to content

Commit

Permalink
Merge #1770
Browse files Browse the repository at this point in the history 
1770: Fix SockaddrLike::from_raw with unaligned inputs r=rtzoeller a=asomers

The major users of this function are functions like gethostname, which
will always properly align their buffers.  But out-of-crate consumers
could manually construct an unaligned buffer.  Handle that correctly.

Enable Clippy's cast_ptr_alignment lint.  It's disabled by default as it
reports many false positives, but it would've caught this problem.

Reported-by:	Miri
Fixes:		1769

Co-authored-by: Alan Somers <asomers@gmail.com>
  • Loading branch information
@bors @asomers
bors[bot] and asomers authored Jul 23, 2022
2 parents 14418f4 + ca2c920 commit 7cc33c1
Show file tree
Hide file tree
Showing 4 changed files with 28 additions and 21 deletions.
2 changes: 1 addition & 1 deletion .cirrus.yml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ build: &BUILD
- $TOOL +$TOOLCHAIN $BUILD $ZFLAGS --target $TARGET --all-targets
- $TOOL +$TOOLCHAIN doc $ZFLAGS --no-deps --target $TARGET
- $TOOL +$TOOLCHAIN clippy $ZFLAGS --target $TARGET --all-targets -- -D warnings
- if [ -z "$NOHACK" ]; then $TOOL +$TOOLCHAIN install cargo-hack; fi
- if [ -z "$NOHACK" ]; then $TOOL +$TOOLCHAIN install --version 0.5.14 cargo-hack; fi
- if [ -z "$NOHACK" ]; then $TOOL +$TOOLCHAIN hack $ZFLAGS check --target $TARGET --each-feature; fi

# Tests that do require executing the binaries
Expand Down
1 change: 1 addition & 0 deletions src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@
#![deny(missing_debug_implementations)]
#![warn(missing_docs)]
#![cfg_attr(docsrs, feature(doc_cfg))]
#![deny(clippy::cast_ptr_alignment)]

// Re-exported external crates
pub use libc;
Expand Down
45 changes: 25 additions & 20 deletions src/sys/socket/addr.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1195,7 +1195,7 @@ impl SockaddrLike for SockaddrIn {
if (*addr).sa_family as i32 != libc::AF_INET as i32 {
return None;
}
Some(SockaddrIn(*(addr as *const libc::sockaddr_in)))
Some(Self(ptr::read_unaligned(addr as *const _)))
}
}

Expand Down Expand Up @@ -1301,7 +1301,7 @@ impl SockaddrLike for SockaddrIn6 {
if (*addr).sa_family as i32 != libc::AF_INET6 as i32 {
return None;
}
Some(SockaddrIn6(*(addr as *const libc::sockaddr_in6)))
Some(Self(ptr::read_unaligned(addr as *const _)))
}
}

Expand Down Expand Up @@ -1870,21 +1870,21 @@ impl SockAddr {
Some(AddressFamily::Unix) => None,
#[cfg(feature = "net")]
Some(AddressFamily::Inet) => Some(SockAddr::Inet(
InetAddr::V4(*(addr as *const libc::sockaddr_in)))),
InetAddr::V4(ptr::read_unaligned(addr as *const _)))),
#[cfg(feature = "net")]
Some(AddressFamily::Inet6) => Some(SockAddr::Inet(
InetAddr::V6(*(addr as *const libc::sockaddr_in6)))),
InetAddr::V6(ptr::read_unaligned(addr as *const _)))),
#[cfg(any(target_os = "android", target_os = "linux"))]
Some(AddressFamily::Netlink) => Some(SockAddr::Netlink(
NetlinkAddr(*(addr as *const libc::sockaddr_nl)))),
NetlinkAddr(ptr::read_unaligned(addr as *const _)))),
#[cfg(all(feature = "ioctl",
any(target_os = "ios", target_os = "macos")))]
Some(AddressFamily::System) => Some(SockAddr::SysControl(
SysControlAddr(*(addr as *const libc::sockaddr_ctl)))),
SysControlAddr(ptr::read_unaligned(addr as *const _)))),
#[cfg(any(target_os = "android", target_os = "linux"))]
#[cfg(feature = "net")]
Some(AddressFamily::Packet) => Some(SockAddr::Link(
LinkAddr(*(addr as *const libc::sockaddr_ll)))),
LinkAddr(ptr::read_unaligned(addr as *const _)))),
#[cfg(any(target_os = "dragonfly",
target_os = "freebsd",
target_os = "ios",
Expand All @@ -1894,7 +1894,7 @@ impl SockAddr {
target_os = "openbsd"))]
#[cfg(feature = "net")]
Some(AddressFamily::Link) => {
let ether_addr = LinkAddr(*(addr as *const libc::sockaddr_dl));
let ether_addr = LinkAddr(ptr::read_unaligned(addr as *const _));
if ether_addr.is_empty() {
None
} else {
Expand All @@ -1903,7 +1903,7 @@ impl SockAddr {
},
#[cfg(any(target_os = "android", target_os = "linux"))]
Some(AddressFamily::Vsock) => Some(SockAddr::Vsock(
VsockAddr(*(addr as *const libc::sockaddr_vm)))),
VsockAddr(ptr::read_unaligned(addr as *const _)))),
// Other address families are currently not supported and simply yield a None
// entry instead of a proper conversion to a `SockAddr`.
Some(_) | None => None,
Expand Down Expand Up @@ -2104,7 +2104,7 @@ pub mod netlink {
if (*addr).sa_family as i32 != libc::AF_NETLINK as i32 {
return None;
}
Some(NetlinkAddr(*(addr as *const libc::sockaddr_nl)))
Some(Self(ptr::read_unaligned(addr as *const _)))
}
}

Expand Down Expand Up @@ -2148,7 +2148,7 @@ pub mod alg {
if (*addr).sa_family as i32 != libc::AF_ALG as i32 {
return None;
}
Some(AlgAddr(*(addr as *const libc::sockaddr_alg)))
Some(Self(ptr::read_unaligned(addr as *const _)))
}
}

Expand Down Expand Up @@ -2220,7 +2220,7 @@ feature! {
pub mod sys_control {
use crate::sys::socket::addr::AddressFamily;
use libc::{self, c_uchar};
use std::{fmt, mem};
use std::{fmt, mem, ptr};
use std::os::unix::io::RawFd;
use crate::{Errno, Result};
use super::{private, SockaddrLike};
Expand Down Expand Up @@ -2262,7 +2262,7 @@ pub mod sys_control {
if (*addr).sa_family as i32 != libc::AF_SYSTEM as i32 {
return None;
}
Some(SysControlAddr(*(addr as *const libc::sockaddr_ctl)))
Some(Self(ptr::read_unaligned(addr as *const _)))
}
}

Expand Down Expand Up @@ -2329,7 +2329,7 @@ pub mod sys_control {
mod datalink {
feature! {
#![feature = "net"]
use super::{fmt, mem, private, SockaddrLike};
use super::{fmt, mem, private, ptr, SockaddrLike};

/// Hardware Address
#[derive(Clone, Copy, Debug, Eq, Hash, PartialEq)]
Expand Down Expand Up @@ -2405,7 +2405,7 @@ mod datalink {
if (*addr).sa_family as i32 != libc::AF_PACKET as i32 {
return None;
}
Some(LinkAddr(*(addr as *const libc::sockaddr_ll)))
Some(Self(ptr::read_unaligned(addr as *const _)))
}
}

Expand All @@ -2430,7 +2430,7 @@ mod datalink {
mod datalink {
feature! {
#![feature = "net"]
use super::{fmt, mem, private, SockaddrLike};
use super::{fmt, mem, private, ptr, SockaddrLike};

/// Hardware Address
#[derive(Clone, Copy, Debug, Eq, Hash, PartialEq)]
Expand Down Expand Up @@ -2525,7 +2525,7 @@ mod datalink {
if (*addr).sa_family as i32 != libc::AF_LINK as i32 {
return None;
}
Some(LinkAddr(*(addr as *const libc::sockaddr_dl)))
Some(Self(ptr::read_unaligned(addr as *const _)))
}
}

Expand Down Expand Up @@ -2569,7 +2569,7 @@ pub mod vsock {
if (*addr).sa_family as i32 != libc::AF_VSOCK as i32 {
return None;
}
Some(VsockAddr(*(addr as *const libc::sockaddr_vm)))
Some(Self(ptr::read_unaligned(addr as *const _)))
}
}

Expand Down Expand Up @@ -2658,6 +2658,8 @@ mod tests {
}

mod link {
#![allow(clippy::cast_ptr_alignment)]

use super::*;
#[cfg(any(target_os = "ios",
target_os = "macos",
Expand Down Expand Up @@ -2698,8 +2700,11 @@ mod tests {
))]
#[test]
fn linux_loopback() {
let bytes = [17u8, 0, 0, 0, 1, 0, 0, 0, 4, 3, 0, 6, 1, 2, 3, 4, 5, 6, 0, 0];
let sa = bytes.as_ptr() as *const libc::sockaddr;
#[repr(align(2))]
struct Raw([u8; 20]);

let bytes = Raw([17u8, 0, 0, 0, 1, 0, 0, 0, 4, 3, 0, 6, 1, 2, 3, 4, 5, 6, 0, 0]);
let sa = bytes.0.as_ptr() as *const libc::sockaddr;
let len = None;
let sock_addr = unsafe { SockaddrStorage::from_raw(sa, len) }.unwrap();
assert_eq!(sock_addr.family(), Some(AddressFamily::Packet));
Expand Down
1 change: 1 addition & 0 deletions src/sys/socket/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -949,6 +949,7 @@ impl ControlMessageOwned {

#[cfg(any(target_os = "android", target_os = "linux"))]
#[cfg(feature = "net")]
#[allow(clippy::cast_ptr_alignment)] // False positive
unsafe fn recv_err_helper<T>(p: *mut libc::c_uchar, len: usize) -> (libc::sock_extended_err, Option<T>) {
let ee = p as *const libc::sock_extended_err;
let err = ptr::read_unaligned(ee);
Expand Down

0 comments on commit 7cc33c1

Please sign in to comment.