Skip to content

Commit 7a2154c

Browse files
committed
fix(net): defer the named-unix sendto path and align batch partial semantics
Addresses review on the connected-AF_UNIX sendmsg change. - sendto_named_unix_on_behalf() still sent inline with a blocking libc::sendto() on the notification loop. A child with only fs-write grants (no net rule) can bind a datagram socket under a writable path, never drain it, and send until the queue fills; the next blocking sendto would wedge the whole loop — the same DoS this change fixes for the other send paths. Route it through resolve_send (first attempt MSG_DONTWAIT on the loop, a blocking child's would-block completed off-loop) by wrapping the already-copied data and pinned inode in a MaterializedMsg, mirroring the sendmsg path. - complete_batch_entry(): align the deferred batch tail with the kernel's blocking-stream semantics. A sendmsg that makes any progress returns that byte count and is a completed message (a hard error after partial progress surfaces on the next call). Merge the two Ok arms: for any Ok(n) write n back as the entry's msg_len and count it (prior_count + 1). The prior shape left a partially-sent-then-errored entry uncounted, so the child would re-send bytes the kernel already queued (duplicate data) or spin forever on a zero-progress retry; it also never returns 0 for vlen > 0 now.
1 parent cc3f33b commit 7a2154c

1 file changed

Lines changed: 34 additions & 27 deletions

File tree

crates/sandlock-core/src/network.rs

Lines changed: 34 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -1000,21 +1000,25 @@ fn sendto_named_unix_on_behalf(
10001000
Ok(fd) => fd,
10011001
Err(e) => return NotifAction::Errno(e.raw_os_error().unwrap_or(libc::EBADF)),
10021002
};
1003-
let ret = unsafe {
1004-
libc::sendto(
1005-
dup_fd.as_raw_fd(),
1006-
data.as_ptr() as *const libc::c_void,
1007-
data.len(),
1008-
flags,
1009-
&sun as *const libc::sockaddr_un as *const libc::sockaddr,
1010-
len,
1011-
)
1012-
};
1013-
if ret >= 0 {
1014-
NotifAction::ReturnValue(ret as i64)
1015-
} else {
1016-
NotifAction::Errno(unsafe { *libc::__errno_location() })
1003+
// Route through resolve_send like the sendmsg path instead of an inline
1004+
// blocking sendto: the dup shares the child's blocking mode, so an inline
1005+
// send on the notification loop wedges the whole loop when a child fills a
1006+
// datagram queue it never drains — the same DoS this change fixes elsewhere.
1007+
// The first attempt is non-blocking on the loop; a blocking child's would-
1008+
// block is completed off-loop.
1009+
let addr = unsafe {
1010+
std::slice::from_raw_parts(&sun as *const libc::sockaddr_un as *const u8, len as usize)
10171011
}
1012+
.to_vec();
1013+
let m = MaterializedMsg {
1014+
data,
1015+
control: None,
1016+
addr,
1017+
_scm_fds: Vec::new(),
1018+
_pinned: Some(pinned),
1019+
};
1020+
let blocking = wants_blocking(dup_fd.as_raw_fd(), flags);
1021+
resolve_send(dup_fd, m, flags, blocking)
10181022
}
10191023

10201024
/// True if `real` (an already-canonical path) is at or under any of `prefixes`,
@@ -1694,13 +1698,20 @@ async fn defer_send(dup_fd: OwnedFd, m: MaterializedMsg, flags: i32, offset: usi
16941698
/// Deferred tail shared by the three `sendmmsg` batch loops. Completes entry
16951699
/// `prior_count` (which either would-block entirely, offset 0, or partially sent
16961700
/// a stream, offset > 0) off the loop, then reports the *message* count — not a
1697-
/// byte count — as `sendmmsg` requires: `prior_count + 1` once the entry is
1698-
/// fully delivered (writing its full `msg_len` back so a blocking caller that
1699-
/// ignores per-entry `msg_len` isn't silently truncated). On error it reports
1700-
/// `prior_count` (the entries already fully sent), or the errno only when
1701-
/// nothing at all has been sent (`prior_count == 0` and this entry made no
1702-
/// progress). Entries beyond this one are left for the child to retry, so the
1703-
/// batch is never materialized whole.
1701+
/// byte count — as `sendmmsg` requires.
1702+
///
1703+
/// Aligns with the kernel's blocking-stream semantics: a `sendmsg` that makes
1704+
/// any progress returns that byte count and is a completed message; a hard error
1705+
/// after partial progress surfaces on the child's *next* call. So for any
1706+
/// `Ok(n)` (n is the full length on success, or the bytes queued before a hard
1707+
/// error) we write `n` back as this entry's `msg_len` and count it as
1708+
/// `prior_count + 1`. This never returns 0 for `vlen > 0`, and — crucially —
1709+
/// never leaves an already-queued entry uncounted, which would make the child
1710+
/// re-send bytes the kernel already accepted (duplicate data) or spin forever on
1711+
/// a zero-progress retry. `Err(e)` (nothing sent at all) reports the errno only
1712+
/// when nothing has been sent yet (`prior_count == 0`), else the prior count.
1713+
/// Entries beyond this one are left for the child to retry, so the batch is
1714+
/// never materialized whole.
17041715
fn complete_batch_entry(
17051716
dup_fd: OwnedFd,
17061717
m: MaterializedMsg,
@@ -1712,17 +1723,13 @@ fn complete_batch_entry(
17121723
msglen_addr: u64,
17131724
prior_count: usize,
17141725
) -> NotifAction {
1715-
let m_len = m.data.len();
17161726
NotifAction::defer(async move {
17171727
match push_until_done(dup_fd, m, flags, offset).await {
1718-
Ok(n) if n == m_len => {
1719-
let bytes = (m_len as u32).to_ne_bytes();
1728+
Ok(n) => {
1729+
let bytes = (n as u32).to_ne_bytes();
17201730
let _ = write_child_mem(notif_fd, notif_id, notif_pid, msglen_addr, &bytes);
17211731
NotifAction::ReturnValue((prior_count + 1) as i64)
17221732
}
1723-
// Partial stream then hard error: entry not fully delivered, so it is
1724-
// not counted; the child retries it (and the rest).
1725-
Ok(_) => NotifAction::ReturnValue(prior_count as i64),
17261733
Err(e) => {
17271734
if prior_count == 0 {
17281735
NotifAction::Errno(e)

0 commit comments

Comments
 (0)