Repository navigation
fix(cmux-wg-tun): a socketpair is never read as a utun (the name query is LOCAL_PEERPID on a local socket) - #34248
Conversation
…e utun name query is LOCAL_PEERPID there) a_socketpair_is_not_mistaken_for_utun failed in some full runs and passed alone. The cause is not test order: on macOS, getsockopt(level 2, option 2) on a local socket is LOCAL_PEERPID (its handler ignores the level), so a socketpair answers the utun name query with this process's pid. nextest runs each test in its own process, so the 4 pid bytes read as a name in about half the runs. the_utun_name_query_answers_a_local_socket_with_its_peer_pid pins the cause (green). is_kernel_control keeps the current rule (the fd answers the name query), so a_socketpair_is_not_a_kernel_control_socket is red on macOS in every process. The next commit fixes the rule.
…by the name query alone is_kernel_control now asks getpeername for the fd's peer address family and accepts only AF_SYSTEM (kernel control sockets), as WireGuard's Apple client does. A local socket, which answers the utun name query with its peer pid (LOCAL_PEERPID), is no longer read as an interface. a_socketpair_is_not_a_kernel_control_socket and a_socketpair_is_not_mistaken_for_utun are now green in every process.
|
cmux status (one comment, edited in place; each line is updated by the job that owns it) Review App of this head (app.zip, kept 1 day)
red: cmux-next checks (god files, concurrency, crash safety, l10n); base red: the same failure on the base cf0fdde; does not block Shipped in:
|
|
Review (claude-opus-reviewer): SHIP at 792a09a Independent review (read-only). The cause is correct: XNU's uipc_ctloutput switches on the option name only, so (2, 2) is LOCAL_PEERPID on an AF_UNIX socket, and a datagram socketpair is connected. The premise test matches the pid bytes as a 4-byte pid_t. The fix compiles on macOS and Linux (ss_family is u8 or u16, and c_int::from accepts both). A real utun answers getpeername with AF_SYSTEM, the same check WireGuard-apple makes. The red commit is red on macOS for any pid and green on Linux. The PR adds no panic-ratchet site in non-test code. Non-blocking wording note: the bytes read as a name when they are valid UTF-8 up to the first NUL, not only when they are ASCII. |
|
Found 3 test failures on Blacksmith runners: Failures
|
![Fix with [code]smith](https://pr-comments-assets.blacksmith.sh/codesmith/fix-with-codesmith-light-3x.png)
GitHub Actions
Inactive
GitHub Actions
Inactive
GitHub Actions
Inactive
|
Red and green proof at e61e5a7 (red) and 792a09a (head). Command: Owned Macs (aws-m4pro):
Also on Blacksmith macOS 26.3 (not counted above): red failed 2 of 2, green passed 3 of 3. |
lawrencecchen
added
the
ready-to-land
…tions) and acpmux repo_sync_tests.rs (field_reassign_with_default) (#34733) Both are red at the feat-cmux-next tip and hold every Rust PR's --arg all step (for example R-INPUT's #34641, green step 5a79450ad2d0, whose own tests passed 57 of 57): - cmux-tui/crates/cmux-wg-tun/src/utun.rs:32: `std::mem::size_of` is in the prelude (unused_qualifications under -D warnings), from f18c96c (#34248). - cmux-tui/crates/acpmux/src/hub/permissions/repo_sync_tests.rs:11: a field set right after Config::default() (clippy field_reassign_with_default), from bf98a75 (#34491). The fixture now builds Config with `..Default::default()`, as folder_profiles_tests.rs does. No behavior change.
Summary
cmux-wg-tun's testengine::tests::a_socketpair_is_not_mistaken_for_utunfailed in a full run and passed alone (fleet stepf74200465feb95b217cf67d8). The cause is not test order and not shared state.Cause:
utun::interface_nameasksgetsockopt(fd, 2, 2)(SYSPROTO_CONTROL,UTUN_OPT_IFNAME). On a local (AF_UNIX) socket, macOS reads the same level and option asLOCAL_PEERPID; its handler ignores the level. So a socketpair answers the query with this process's pid as 4 bytes, and those bytes read as a "name" whenever they are ASCII. nextest runs each test in its own process, so the pid, and the result, changed from run to run.Fix: a utun is found by its peer address family.
is_kernel_controlaccepts only anAF_SYSTEMpeer (getpeername), which is what WireGuard's Apple client checks. Only then doesinterface_nameask for the name.The crate came in lawrencecchen's direct push
ad2c76731f28..cc33e089376eand stays on the tip after the revert #34198, so this is a tip defect. This PR does not depend on #34220 (they change different lines ofutun.rs).Testing
Red first:
e61e5a7a04e3adds the tests, andis_kernel_controlkeeps the old rule (the fd answers the name query).the_utun_name_query_answers_a_local_socket_with_its_peer_pid(macOS) pins the cause: the query succeeds on a socketpair, and the 4 reply bytes are this process's pid.a_socketpair_is_not_a_kernel_control_socketis red on macOS at the red commit in every process, whatever the pid.cmux-tui-rust-check.sh test engine::tests::: red commit, step140d2532959adadca832e04c; fix792a09aec9e3, step6b8d0dc2737388103d413953. Linux panic ratchet at the fix: step307697ab0db2034d5f06fccc. Results in a comment.Observed / Not run
find_utun_fdinside a real NetworkExtension provider (needs a signed system extension).getpeernamewith anAF_SYSTEMcheck is the method WireGuard's Apple client uses on the same fd.Changelog
none
Open in [code]smith · Need help on this PR? Tag
@codesmith-botwith what you need.Summary by cubic
Fixes a macOS test flake in
cmux-wg-tunwhere a socketpair could be mistaken for a utun interface. The utun name query (levelSYSPROTO_CONTROL, optionUTUN_OPT_IFNAME) is alsoLOCAL_PEERPIDon a local socket, so a socketpair answered it with this process's pid as 4 bytes, which read as an ASCII "name" roughly half the time — making the result pid-dependent and flaky between runs.Now a kernel control socket is identified by its peer address family (
AF_SYSTEMviagetpeername), the same check WireGuard's Apple client uses, before the interface name is queried. Adds regression tests pinning the root cause and the corrected rule.Written for commit 9b59b8c. Summary will update on new commits.