diff --git a/.github/AGENTS.md b/.github/AGENTS.md index ceaeed1f..9babd9c4 100644 --- a/.github/AGENTS.md +++ b/.github/AGENTS.md @@ -16,6 +16,8 @@ binary points at that page. Renaming or removing an id silently drops the prefill for binaries that can never be updated retroactively. - Keep Linux system dependencies aligned with real build needs for Wayland, Cairo, Pango, GTK4/libadwaita, D-Bus, packaging, and Nix checks. +- `deny.toml` is the dependency policy and the `dependency-policy` job enforces it. Advisories report without blocking, so a new advisory cannot fail an unrelated pull request; bans, licenses, and sources block. +- `dependabot.yml` proposes weekly Cargo and GitHub Actions updates. Its zbus major-version ignore is coupled to the advisory ignores in `deny.toml`. - Do not weaken release checks or skip package layout/version validation without a documented reason. ## Coupled Changes diff --git a/.github/dependabot.yml b/.github/dependabot.yml new file mode 100644 index 00000000..08ed61af --- /dev/null +++ b/.github/dependabot.yml @@ -0,0 +1,27 @@ +version: 2 +updates: + - package-ecosystem: cargo + directory: / + schedule: + interval: weekly + open-pull-requests-limit: 5 + groups: + compatible: + update-types: + - minor + - patch + ignore: + # ksni, which the tray feature needs, still requires zbus 5. Remove this + # with the matching advisory ignores in deny.toml. + - dependency-name: zbus + update-types: + - version-update:semver-major + + - package-ecosystem: github-actions + directory: / + schedule: + interval: weekly + groups: + actions: + patterns: + - "*" diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 8ff15073..b19df8af 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -144,3 +144,26 @@ jobs: - name: Check flake package and development shell instantiation run: dotnet run tools/wayscriber.cs --no-build -- check nix-instantiation + + dependency-policy: + name: Dependency policy (${{ matrix.checks }}) + runs-on: ubuntu-latest + timeout-minutes: 10 + strategy: + fail-fast: false + matrix: + checks: + - advisories + - bans licenses sources + # A newly published advisory must not block unrelated pull requests. + continue-on-error: ${{ matrix.checks == 'advisories' }} + steps: + - name: Checkout + uses: actions/checkout@v4 + + - name: Check dependency policy + uses: EmbarkStudios/cargo-deny-action@v2 + with: + arguments: --workspace --locked + command: check ${{ matrix.checks }} + rust-version: 1.99.1 diff --git a/AGENTS.md b/AGENTS.md index 1f19ebe2..8194e042 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -40,6 +40,7 @@ - Never reach inside a poisoned lock. Use `expect` when the critical section cannot panic, or a fallback that leaves the data alone. Tests that serialize on process-wide state use `test_env::lock()` or a `test_env::Lease`. - `catch_unwind` is reserved for the reviewed worker-isolation sites that carry an `#[expect]` reason. Tests observe panics through a joined scoped thread. - Getters are `foo()`/`foo_mut()`, a fallible constructor is `try_new`, and type lifetimes have descriptive names. Every `unsafe` block has a `// SAFETY:` comment that says why it is sound. +- Make kernel calls through `std` or `rustix`, which need no `unsafe`, and rename on import when a module name clashes with `std` (`use rustix::fs as rustix_fs`). Use raw `libc` calls only where neither fits: signal handlers, the post-`clone` broker stub, `fail_stop`, the zero-signal pidfd check, validating the broker's inherited descriptors, the inotify header parser, `dup2` onto a fixed descriptor number, and `localtime_r`. `libc` constants stay fine. - Convert numbers losslessly with `From` (`f64::from(width)`) wherever it can be called; `const` initializers keep `as`. Take the middle of two values with `midpoint` (`f64::midpoint(a, b)`), which cannot overflow. - Inline named format arguments: `format!("{name}: {err}")`. - Use `map_or`, `map_or_else`, `is_some_and`, or `is_ok_and` instead of `map(..).unwrap_or(..)`. Use `let ... else` for a `match` that only unwraps or leaves, and `if let ... else` for a `match` with one pattern and a fallback. @@ -51,8 +52,10 @@ - Config changes often require `config.example.toml`, `docs/CONFIG.md`, configurator models/views, schema behavior, and tests. - Tool/action/keybinding changes often require config defaults, action metadata, help/command UI, toolbar UI, configurator labels/search, docs, and tests. - Daemon, service, shortcut, path, and packaging changes often need updates across `src/daemon/`, `src/paths/`, `src/systemd_user_service.rs`, `src/shortcut_hint.rs`, `configurator/src/app/daemon_setup/`, and `packaging/`. +- Dependency changes must satisfy `deny.toml`. A new license or duplicate version needs an entry there with its reason. ## Validation +- For a fast full-suite run while iterating, use `cargo nextest run --workspace --all-features`. Each test gets its own process, so it runs in parallel despite the native-font race that keeps the gate serial. It skips doctests and the isolated render tests, so it does not replace the gate. - Full local CI is `./tools/lint-and-test.sh`. It needs the .NET SDK selected by `global.json`. Without it, `cargo test` still runs the Rust source guards in `tests/repository_guards` (process sites, config writers, shared dependencies, use statements, no Python), but not the C# checks, including the source-coverage check that finds `.rs` files Cargo never compiles. - That script runs the C# repository checks (assets, version, nixpkgs recipe, source coverage, legacy tools), the C# tool tests (which also run the packaging shell contracts), `cargo fmt --all -- --check`, clippy with all targets/features, all-feature tests, and no-default-feature tests. - Run `git diff --check` before handoff. When relevant files are untracked and the index must stay unchanged, check those files directly as well. diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index bda172f3..700426fd 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -125,6 +125,16 @@ cargo test session:: cargo test -p wayscriber-configurator ``` +For a fast run of the whole suite, use [cargo-nextest](https://nexte.st/), which the Nix shell +includes. It gives each test its own process, so it can run in parallel despite the native font +race that keeps the local CI entry point serial: + +```bash +cargo nextest run --workspace --all-features +``` + +It skips doctests and the isolated render tests, so it does not replace that entry point. + Before submitting a broad or cross-package change, run the local CI entry point: ```bash @@ -139,6 +149,13 @@ Rust tests include the repository guards in `tests/repository_guards`. The sourc gate uses current rustc dep-info and rejects tracked or unignored `.rs` files that are outside the supported Cargo target/feature matrix. +Dependency changes must also satisfy the policy in `deny.toml`, which CI enforces in its own +job. The Nix shell includes `cargo-deny`; elsewhere, install it and run: + +```bash +cargo deny --workspace --locked check +``` + The all-feature portal transport tests require `dbus-daemon`. Each fixture owns a private session bus and connects through its explicit address, leaving the desktop session bus alone. diff --git a/Cargo.lock b/Cargo.lock index 1c79f711..8d11b397 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -294,7 +294,7 @@ version = "2.5.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "da7c62ceae207dd37ea5b845da6a0696c799f85e97da1ab5b7910be3c1c80223" dependencies = [ - "getrandom 0.4.3", + "getrandom", ] [[package]] @@ -513,18 +513,6 @@ dependencies = [ "system-deps 9.0.1", ] -[[package]] -name = "getrandom" -version = "0.3.4" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "899def5c37c4fd7b2664648c28120ecec138e4d395b459e5ca34f9cce2dd77fd" -dependencies = [ - "cfg-if", - "libc", - "r-efi 5.3.0", - "wasip2", -] - [[package]] name = "getrandom" version = "0.4.3" @@ -534,7 +522,7 @@ dependencies = [ "cfg-if", "js-sys", "libc", - "r-efi 6.0.0", + "r-efi", "wasm-bindgen", ] @@ -882,6 +870,12 @@ dependencies = [ "zbus", ] +[[package]] +name = "lexopt" +version = "0.3.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "803ec87c9cfb29b9d2633f20cba1f488db3fd53f2158b1024cbefb47ba05d413" + [[package]] name = "libadwaita" version = "0.9.2" @@ -1155,12 +1149,6 @@ dependencies = [ "proc-macro2", ] -[[package]] -name = "r-efi" -version = "5.3.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "69cdb34c158ceb288df11e18b4bd39de994f6657d83847bdffdbd7f346754b0f" - [[package]] name = "r-efi" version = "6.0.0" @@ -1528,7 +1516,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "32497e9a4c7b38532efcdebeef879707aa9f794296a4f0244f6f69e9bc8574bd" dependencies = [ "fastrand", - "getrandom 0.4.3", + "getrandom", "once_cell", "rustix", "windows-sys 0.61.2", @@ -1724,15 +1712,6 @@ version = "0.11.1+wasi-snapshot-preview1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "ccf3ec651a847eb01de73ccad15eb7d99f80485de043efb2f370cd654f4ea44b" -[[package]] -name = "wasip2" -version = "1.0.4+wasi-0.2.12" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "b67efb37e106e55ce722a510d6b5f9c17f083e5fc79afc2badeb12cc313d9487" -dependencies = [ - "wit-bindgen", -] - [[package]] name = "wasm-bindgen" version = "0.2.129" @@ -1903,17 +1882,19 @@ dependencies = [ "anyhow", "cairo-rs", "flate2", - "getrandom 0.3.4", + "getrandom", "glib", "gtk4", "gtk4-layer-shell", "input", "ksni", + "lexopt", "libc", "log", "pango", "pangocairo", "png", + "rustix", "schemars", "serde", "serde_ignored", @@ -2035,12 +2016,6 @@ dependencies = [ "memchr", ] -[[package]] -name = "wit-bindgen" -version = "0.57.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "1ebf944e87a7c253233ad6766e082e3cd714b5d03812acc24c318f549614536e" - [[package]] name = "xcursor" version = "0.3.11" diff --git a/Cargo.toml b/Cargo.toml index 5ea8103d..50f2d279 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -3,6 +3,7 @@ name = "wayscriber" version = "0.9.28" edition = "2024" rust-version = "1.99.1" +license = "MIT" description = "Screen annotation tool for Wayland compositors" homepage = "https://wayscriber.com" repository = "https://github.com/devmobasa/wayscriber" @@ -92,10 +93,16 @@ serde = { version = "1.0", features = ["derive"] } serde_ignored = "0.1" schemars = { version = "1.2", features = ["derive"], optional = true } libc = "0.2" +# Safe wrappers for the kernel interfaces that the std library does not cover. +rustix = { version = "1", features = ["event", "fs", "net", "pipe", "process", "thread", "time"] } +# Kernel random bytes for portal handle tokens and daemon protocol identifiers. +getrandom = { version = "0.4", features = ["std"] } # Securely created, self-deleting temporary files for the OCR engine handoff. tempfile = "3" flate2 = "1.0" zune-jpeg = "0.5" +# Command-line tokenizing without a parser framework. +lexopt = "0.3" # Grapheme/word segmentation for the text-editor caret and selection. unicode-segmentation = "1" @@ -105,7 +112,6 @@ tokio = { version = "1.0", features = ["rt-multi-thread", "macros", "time", "syn # Screenshot capture zbus = { version = "5.0", optional = true, default-features = false, features = ["tokio"] } -getrandom = { version = "0.3", optional = true } serde_json = "1.0" png = "0.18" @@ -125,7 +131,7 @@ xkbcommon = { version = "0.8", optional = true } tablet-input = [] # D-Bus dependent functionality (portal capture, notifications, tray) dbus = ["zbus"] -portal = ["dbus", "dep:getrandom"] +portal = ["dbus"] tray = ["dbus", "ksni"] config-schema = ["dep:schemars"] # GTK4-rendered toolbars on layer-shell compositors; the built-in Cairo diff --git a/configurator/Cargo.toml b/configurator/Cargo.toml index 0b9608ed..a41be09e 100644 --- a/configurator/Cargo.toml +++ b/configurator/Cargo.toml @@ -3,6 +3,8 @@ name = "wayscriber-configurator" version = "0.9.28" edition = "2024" rust-version = "1.99.1" +license = "MIT" +publish = false [lints] workspace = true diff --git a/deny.toml b/deny.toml new file mode 100644 index 00000000..d70375b6 --- /dev/null +++ b/deny.toml @@ -0,0 +1,42 @@ +# Dependency policy for the workspace. CI runs it in the `dependency-policy` job; +# `cargo deny --workspace --locked check` gives the same result locally. + +[graph] +all-features = true +targets = [ + { triple = "x86_64-unknown-linux-gnu" }, + { triple = "aarch64-unknown-linux-gnu" }, +] + +[advisories] +# zbus 6 absorbed these crates. ksni, which the `tray` feature needs, still +# requires zbus 5, so remove these entries once it moves. +ignore = [ + { id = "RUSTSEC-2026-0370", reason = "zvariant: blocked on ksni supporting zbus 6" }, + { id = "RUSTSEC-2026-0371", reason = "zbus_names: blocked on ksni supporting zbus 6" }, + { id = "RUSTSEC-2026-0372", reason = "zvariant_derive: blocked on ksni supporting zbus 6" }, +] + +[licenses] +allow = [ + "Apache-2.0", + "Apache-2.0 WITH LLVM-exception", + "MIT", + "Unicode-3.0", + "Unlicense", +] + +[bans] +multiple-versions = "deny" +wildcards = "deny" +allow-wildcard-paths = true +skip = [ + { crate = "miniz_oxide@0.8", reason = "png needs 0.8 while flate2 needs 0.9" }, + { crate = "syn@2", reason = "proc macros have not all moved to syn 3" }, + { crate = "system-deps@7", reason = "build-only: the GTK sys crates span three majors" }, + { crate = "system-deps@8", reason = "build-only: the GTK sys crates span three majors" }, +] + +[sources] +unknown-registry = "deny" +unknown-git = "deny" diff --git a/flake.nix b/flake.nix index 43b179a8..583af8f8 100644 --- a/flake.nix +++ b/flake.nix @@ -32,6 +32,8 @@ servicePath = pkgs.lib.makeBinPath [ pkgs.grim pkgs.slurp pkgs.wl-clipboard ]; developmentInputs = with pkgs; [ rustToolchain + cargo-deny + cargo-nextest util-linux pkg-config cairo diff --git a/src/app/mod.rs b/src/app/mod.rs index 58e531bb..584d3991 100644 --- a/src/app/mod.rs +++ b/src/app/mod.rs @@ -22,10 +22,14 @@ use crate::{ }; use anyhow::Context; use env::env_flag_enabled; +#[cfg(unix)] +use rustix::process as rustix_process; use session::run_session_cli_commands; use std::ffi::OsString; use std::fs::{File, OpenOptions}; use std::io::ErrorKind; +#[cfg(unix)] +use std::io::IsTerminal; use std::path::{Path, PathBuf}; use usage::{log_overlay_controls, print_usage}; @@ -170,16 +174,17 @@ fn run_update_check() -> anyhow::Result<()> { #[cfg(unix)] fn detach_from_tty() { // Start a new session to drop the controlling terminal (prevents stuck shells). - // SAFETY: setsid takes no arguments; a failure only leaves the session as is. - unsafe { - let _ = libc::setsid(); - } + // A failure only leaves the session as is. + let _ = rustix_process::setsid(); // Point stdio that still refers to a TTY at /dev/null. Closing it instead // frees the descriptor number for the next open, and the logger's stderr // writes would then land in whatever file that open returned. - for fd in [libc::STDIN_FILENO, libc::STDOUT_FILENO, libc::STDERR_FILENO] { - // SAFETY: isatty only inspects the descriptor number it is given. - let is_tty = unsafe { libc::isatty(fd) } == 1; + let stdio = [ + (libc::STDIN_FILENO, io::stdin().is_terminal()), + (libc::STDOUT_FILENO, io::stdout().is_terminal()), + (libc::STDERR_FILENO, io::stderr().is_terminal()), + ]; + for (fd, is_tty) in stdio { if is_tty && let Err(error) = redirect_to_dev_null(fd) { log::warn!("Failed to detach descriptor {fd} from the terminal: {error}"); } @@ -414,6 +419,8 @@ mod tests { use std::io::Write; use std::os::fd::AsRawFd; + use rustix::io as rustix_io; + let temp = test_temp::tempdir().unwrap(); let path = temp.path().join("was-a-terminal"); let mut file = File::create(&path).unwrap(); @@ -422,8 +429,7 @@ mod tests { redirect_to_dev_null(file.as_raw_fd()).unwrap(); file.write_all(b"after").unwrap(); - // SAFETY: F_GETFD only inspects the descriptor the file still owns. - assert!(unsafe { libc::fcntl(file.as_raw_fd(), libc::F_GETFD) } >= 0); + rustix_io::fcntl_getfd(&file).unwrap(); assert_eq!(fs::read(&path).unwrap(), b"before "); } diff --git a/src/backend/wayland/backend/event_loop/dispatch.rs b/src/backend/wayland/backend/event_loop/dispatch.rs index 0d85bb31..0a6edcf2 100644 --- a/src/backend/wayland/backend/event_loop/dispatch.rs +++ b/src/backend/wayland/backend/event_loop/dispatch.rs @@ -64,11 +64,8 @@ pub(super) fn dispatch_events( #[cfg(test)] mod tests { - use std::io; - use std::io::ErrorKind; - use std::os::fd::AsRawFd; use std::path::PathBuf; - use std::time::Instant; + use std::time::{Duration, Instant}; use super::*; use crate::backend::wayland::session::{ @@ -126,27 +123,12 @@ mod tests { } fn wait_for_runtime_wake(runtime_wake: &RuntimeWakeSource) { - let mut pollfd = libc::pollfd { - fd: runtime_wake.poll_fd().as_raw_fd(), - events: libc::POLLIN, - revents: 0, - }; - loop { - // SAFETY: pollfd and the runtime wake descriptor stay valid for this - // bounded test wait. - let ready = unsafe { libc::poll(&raw mut pollfd, 1, 1_000) }; - if ready > 0 { - assert_ne!(pollfd.revents & libc::POLLIN, 0); - return; - } - if ready == 0 { - panic!("persistence worker did not wake the production route"); - } - let err = io::Error::last_os_error(); - if err.kind() != ErrorKind::Interrupted { - panic!("runtime wake poll failed: {err}"); - } - } + assert!( + runtime_wake + .is_readable_within(Duration::from_secs(1)) + .unwrap(), + "persistence worker did not wake the production route" + ); } #[test] diff --git a/src/backend/wayland/backend/helpers.rs b/src/backend/wayland/backend/helpers.rs index 1f1da97e..1a5f23f2 100644 --- a/src/backend/wayland/backend/helpers.rs +++ b/src/backend/wayland/backend/helpers.rs @@ -3,8 +3,9 @@ use std::io::ErrorKind; use anyhow::{Context, Result}; use log::warn; +use rustix::event::{PollFd, PollFlags}; use std::env; -use std::os::fd::{AsRawFd, RawFd}; +use std::os::fd::BorrowedFd; use std::time::Duration; use wayland_client::EventQueue; use wayland_client::backend::{ReadEventsGuard, WaylandError}; @@ -13,7 +14,8 @@ use super::super::state::WaylandState; #[cfg(test)] use super::runtime_wake::timeout_to_poll_ms; use super::runtime_wake::{ - RuntimeWakeSource, TerminalReadinessPolicy, poll_with_retry, validate_poll_readiness, + RuntimeWakeSource, TerminalReadinessPolicy, poll_for_ms, poll_with_retry, + validate_poll_readiness, }; use crate::capture::CaptureFailureKind; use crate::i18n::tr; @@ -73,13 +75,13 @@ struct RuntimeReadOutcome { } trait PreparedWaylandRead { - fn connection_raw_fd(&self) -> RawFd; + fn connection_fd(&self) -> BorrowedFd<'_>; fn read(self) -> Result; } impl PreparedWaylandRead for ReadEventsGuard { - fn connection_raw_fd(&self) -> RawFd { - self.connection_fd().as_raw_fd() + fn connection_fd(&self) -> BorrowedFd<'_> { + ReadEventsGuard::connection_fd(self) } fn read(self) -> Result { @@ -87,52 +89,28 @@ impl PreparedWaylandRead for ReadEventsGuard { } } +/// Applies the retry and readiness rules to `poll_once`, which reports the +/// Wayland and runtime wake readiness in that order. Both empty means timeout. fn poll_runtime_fds_with( - wayland_fd: RawFd, - wake_fd: RawFd, timeout: Option, - mut poll_once: impl FnMut(&mut [libc::pollfd], i32) -> io::Result, + mut poll_once: impl FnMut(i32) -> io::Result<[PollFlags; 2]>, ) -> io::Result { poll_with_retry(timeout, |timeout_ms| { - let mut pollfds = [ - libc::pollfd { - fd: wayland_fd, - events: libc::POLLIN, - revents: 0, - }, - libc::pollfd { - fd: wake_fd, - events: libc::POLLIN, - revents: 0, - }, - ]; - match poll_once(&mut pollfds, timeout_ms) { - Ok(0) => Ok(None), - Ok(_) => { - let readiness = RuntimePollReadiness { - // A Wayland socket can report final buffered protocol data - // together with HUP/ERR. Read and dispatch it first so the - // compositor's actual disconnect error is preserved. - wayland: validate_poll_readiness( - &pollfds[0], - "Wayland", - TerminalReadinessPolicy::ReadBuffered, - )?, - wake: validate_poll_readiness( - &pollfds[1], - "runtime wake", - TerminalReadinessPolicy::Reject, - )?, - }; - if !readiness.wayland && !readiness.wake { - return Err(io::Error::other( - "runtime poll reported readiness without a readable descriptor", - )); - } - Ok(Some(readiness)) - } - Err(err) => Err(err), + let [wayland, wake] = poll_once(timeout_ms)?; + if wayland.is_empty() && wake.is_empty() { + return Ok(None); } + Ok(Some(RuntimePollReadiness { + // A Wayland socket can report final buffered protocol data + // together with HUP/ERR. Read and dispatch it first so the + // compositor's actual disconnect error is preserved. + wayland: validate_poll_readiness( + wayland, + "Wayland", + TerminalReadinessPolicy::ReadBuffered, + )?, + wake: validate_poll_readiness(wake, "runtime wake", TerminalReadinessPolicy::Reject)?, + })) }) .map(|readiness| { readiness.unwrap_or(RuntimePollReadiness { @@ -143,25 +121,17 @@ fn poll_runtime_fds_with( } fn poll_runtime_fds( - wayland_fd: RawFd, - wake_fd: RawFd, + wayland_fd: BorrowedFd<'_>, + wake_fd: BorrowedFd<'_>, timeout: Option, ) -> io::Result { - poll_runtime_fds_with(wayland_fd, wake_fd, timeout, |pollfds, timeout_ms| { - // SAFETY: pollfds is a live mutable slice for the duration of this call, - // and both descriptors remain borrowed by their runtime owners. - let ready = unsafe { - libc::poll( - pollfds.as_mut_ptr(), - pollfds.len() as libc::nfds_t, - timeout_ms, - ) - }; - if ready < 0 { - Err(io::Error::last_os_error()) - } else { - Ok(ready) - } + poll_runtime_fds_with(timeout, |timeout_ms| { + let mut pollfds = [ + PollFd::from_borrowed_fd(wayland_fd, PollFlags::IN), + PollFd::from_borrowed_fd(wake_fd, PollFlags::IN), + ]; + poll_for_ms(&mut pollfds, timeout_ms)?; + Ok([pollfds[0].revents(), pollfds[1].revents()]) }) } @@ -170,12 +140,8 @@ fn read_events_with_runtime_wake( runtime_wake: &RuntimeWakeSource, timeout: Option, ) -> Result { - let readiness = poll_runtime_fds( - guard.connection_raw_fd(), - runtime_wake.poll_fd().as_raw_fd(), - timeout, - ) - .context("runtime readiness poll failed")?; + let readiness = poll_runtime_fds(guard.connection_fd(), runtime_wake.poll_fd(), timeout) + .context("runtime readiness poll failed")?; let wayland_read = if readiness.wayland { let _ = @@ -331,6 +297,7 @@ mod tests { use crate::set_runtime_session_override; use std::collections::VecDeque; use std::io::Write; + use std::os::fd::AsFd; use std::os::unix::net::UnixStream; use std::sync::Arc; use std::sync::atomic::{AtomicUsize, Ordering}; @@ -372,12 +339,7 @@ mod tests { wake.handle().wake().unwrap(); assert_eq!( - poll_runtime_fds( - wayland_read.as_raw_fd(), - wake.poll_fd().as_raw_fd(), - Some(Duration::ZERO), - ) - .unwrap(), + poll_runtime_fds(wayland_read.as_fd(), wake.poll_fd(), Some(Duration::ZERO),).unwrap(), RuntimePollReadiness { wayland: false, wake: true, @@ -392,12 +354,7 @@ mod tests { wayland_write.write_all(&[1]).unwrap(); assert_eq!( - poll_runtime_fds( - wayland_read.as_raw_fd(), - wake.poll_fd().as_raw_fd(), - Some(Duration::ZERO), - ) - .unwrap(), + poll_runtime_fds(wayland_read.as_fd(), wake.poll_fd(), Some(Duration::ZERO),).unwrap(), RuntimePollReadiness { wayland: true, wake: false, @@ -413,12 +370,7 @@ mod tests { drop(wayland_write); assert_eq!( - poll_runtime_fds( - wayland_read.as_raw_fd(), - wake.poll_fd().as_raw_fd(), - Some(Duration::ZERO), - ) - .unwrap(), + poll_runtime_fds(wayland_read.as_fd(), wake.poll_fd(), Some(Duration::ZERO),).unwrap(), RuntimePollReadiness { wayland: true, wake: false, @@ -434,8 +386,8 @@ mod tests { assert_eq!( poll_runtime_fds( - wayland_read.as_raw_fd(), - wake.poll_fd().as_raw_fd(), + wayland_read.as_fd(), + wake.poll_fd(), Some(Duration::from_secs(30)), ) .unwrap(), @@ -454,12 +406,7 @@ mod tests { wake.handle().wake().unwrap(); assert_eq!( - poll_runtime_fds( - wayland_read.as_raw_fd(), - wake.poll_fd().as_raw_fd(), - Some(Duration::ZERO), - ) - .unwrap(), + poll_runtime_fds(wayland_read.as_fd(), wake.poll_fd(), Some(Duration::ZERO),).unwrap(), RuntimePollReadiness { wayland: true, wake: true, @@ -475,8 +422,8 @@ mod tests { } impl PreparedWaylandRead for FakePreparedWaylandRead { - fn connection_raw_fd(&self) -> RawFd { - self.stream.as_raw_fd() + fn connection_fd(&self) -> BorrowedFd<'_> { + self.stream.as_fd() } fn read(mut self) -> Result { @@ -684,13 +631,12 @@ mod tests { #[test] fn runtime_poll_retries_interruption() { let mut calls = 0; - let readiness = poll_runtime_fds_with(10, 11, None, |pollfds, _| { + let readiness = poll_runtime_fds_with(None, |_| { calls += 1; if calls == 1 { return Err(io::Error::from(ErrorKind::Interrupted)); } - pollfds[1].revents = libc::POLLIN; - Ok(1) + Ok([PollFlags::empty(), PollFlags::IN]) }) .unwrap(); @@ -706,11 +652,8 @@ mod tests { #[test] fn runtime_poll_rejects_invalid_descriptor_readiness() { - let err = poll_runtime_fds_with(10, 11, None, |pollfds, _| { - pollfds[1].revents = libc::POLLNVAL; - Ok(1) - }) - .unwrap_err(); + let err = + poll_runtime_fds_with(None, |_| Ok([PollFlags::empty(), PollFlags::NVAL])).unwrap_err(); assert!(err.to_string().contains("runtime wake")); assert!(err.to_string().contains("readiness")); @@ -718,9 +661,9 @@ mod tests { #[test] fn runtime_poll_timeout_reports_no_readiness() { - let readiness = poll_runtime_fds_with(10, 11, Some(Duration::ZERO), |_, timeout_ms| { + let readiness = poll_runtime_fds_with(Some(Duration::ZERO), |timeout_ms| { assert_eq!(timeout_ms, 0); - Ok(0) + Ok([PollFlags::empty(); 2]) }) .unwrap(); diff --git a/src/backend/wayland/backend/runtime_wake.rs b/src/backend/wayland/backend/runtime_wake.rs index 4b1ad245..5a57c8ab 100644 --- a/src/backend/wayland/backend/runtime_wake.rs +++ b/src/backend/wayland/backend/runtime_wake.rs @@ -1,8 +1,11 @@ use std::io; -use std::os::fd::{AsFd, AsRawFd, BorrowedFd, FromRawFd, OwnedFd}; +use std::os::fd::{AsFd, BorrowedFd, OwnedFd}; use std::sync::Arc; use std::time::{Duration, Instant}; +use rustix::event::{self, EventfdFlags, PollFd, PollFlags, Timespec}; +use rustix::io::{self as rustix_io, Errno}; + #[derive(Debug)] pub(crate) struct RuntimeWakeSource { fd: Arc, @@ -20,33 +23,43 @@ pub(super) enum TerminalReadinessPolicy { } pub(super) fn validate_poll_readiness( - pollfd: &libc::pollfd, + revents: PollFlags, label: &str, terminal_policy: TerminalReadinessPolicy, ) -> io::Result { - let readable = pollfd.revents & libc::POLLIN != 0; - if pollfd.revents & libc::POLLNVAL != 0 { + let readable = revents.contains(PollFlags::IN); + if revents.contains(PollFlags::NVAL) { return Err(io::Error::other(format!( "{label} poll descriptor failed with readiness {:#x}", - pollfd.revents + revents.bits() ))); } - let terminal = pollfd.revents & (libc::POLLERR | libc::POLLHUP); - if terminal != 0 && !(terminal_policy == TerminalReadinessPolicy::ReadBuffered && readable) { + let terminal = revents.intersects(PollFlags::ERR | PollFlags::HUP); + if terminal && !(terminal_policy == TerminalReadinessPolicy::ReadBuffered && readable) { return Err(io::Error::other(format!( "{label} poll descriptor failed with readiness {:#x}", - pollfd.revents + revents.bits() ))); } - if pollfd.revents != 0 && !readable { + if !revents.is_empty() && !readable { return Err(io::Error::other(format!( "{label} poll descriptor returned unexpected readiness {:#x}", - pollfd.revents + revents.bits() ))); } Ok(readable) } +/// `poll(2)` with a millisecond timeout from [`timeout_to_poll_ms`], where a +/// negative value blocks. +pub(super) fn poll_for_ms(fds: &mut [PollFd<'_>], timeout_ms: i32) -> io::Result { + let timeout = (timeout_ms >= 0).then(|| Timespec { + tv_sec: (timeout_ms / 1_000).into(), + tv_nsec: (timeout_ms % 1_000 * 1_000_000).into(), + }); + Ok(event::poll(fds, timeout.as_ref())?) +} + pub(super) fn poll_with_retry( timeout: Option, mut attempt: impl FnMut(i32) -> io::Result>, @@ -67,15 +80,9 @@ pub(super) fn poll_with_retry( impl RuntimeWakeSource { pub(crate) fn try_new() -> io::Result { - // SAFETY: eventfd returns a new owned descriptor on success. EFD_NONBLOCK - // keeps both producer writes and consumer drains bounded, and EFD_CLOEXEC - // prevents subprocesses from extending the runtime descriptor lifetime. - let raw_fd = unsafe { libc::eventfd(0, libc::EFD_NONBLOCK | libc::EFD_CLOEXEC) }; - if raw_fd < 0 { - return Err(io::Error::last_os_error()); - } - // SAFETY: raw_fd was just returned by eventfd and has not been wrapped. - let fd = unsafe { OwnedFd::from_raw_fd(raw_fd) }; + // NONBLOCK keeps both producer writes and consumer drains bounded, and + // CLOEXEC prevents subprocesses from extending the descriptor lifetime. + let fd = event::eventfd(0, EventfdFlags::NONBLOCK | EventfdFlags::CLOEXEC)?; Ok(Self { fd: Arc::new(fd) }) } @@ -93,32 +100,20 @@ impl RuntimeWakeSource { /// One eventfd read consumes the entire accumulated counter, so a second /// successful-path read would only issue a guaranteed-EAGAIN syscall. pub(crate) fn drain(&self) -> io::Result { + let mut value = [0_u8; size_of::()]; loop { - let mut value = 0_u64; - // SAFETY: value points to a writable u64 and the owned eventfd remains - // valid for the duration of this read. - let result = unsafe { - libc::read( - self.fd.as_raw_fd(), - (&raw mut value).cast(), - size_of::(), - ) - }; - if result == size_of::() as isize { - return Ok(true); - } - if result < 0 { - let err = io::Error::last_os_error(); - match err.kind() { - io::ErrorKind::Interrupted => continue, - io::ErrorKind::WouldBlock => return Ok(false), - _ => return Err(err), + match rustix_io::read(&self.fd, &mut value) { + Ok(read) if read == value.len() => return Ok(true), + Ok(read) => { + return Err(io::Error::new( + io::ErrorKind::UnexpectedEof, + format!("runtime wake eventfd returned a short read ({read} bytes)"), + )); } + Err(Errno::INTR) => {} + Err(Errno::WOULDBLOCK) => return Ok(false), + Err(errno) => return Err(errno.into()), } - return Err(io::Error::new( - io::ErrorKind::UnexpectedEof, - format!("runtime wake eventfd returned a short read ({result} bytes)"), - )); } } @@ -126,24 +121,15 @@ impl RuntimeWakeSource { #[cfg(test)] pub(crate) fn wait_readable(&self, timeout: Option) -> io::Result { let readable = poll_with_retry(timeout, |timeout_ms| { - let mut pollfd = libc::pollfd { - fd: self.fd.as_raw_fd(), - events: libc::POLLIN, - revents: 0, - }; - // SAFETY: pollfd is valid and the Arc-owned descriptor remains open - // throughout this bounded or producer-woken wait. - let ready = unsafe { libc::poll(&raw mut pollfd, 1, timeout_ms) }; - if ready == 0 { + let mut pollfds = [PollFd::new(&self.fd, PollFlags::IN)]; + if poll_for_ms(&mut pollfds, timeout_ms)? == 0 { return Ok(None); } - if ready < 0 { - return Err(io::Error::last_os_error()); - } - if !validate_poll_readiness(&pollfd, "runtime wake", TerminalReadinessPolicy::Reject)? { + let revents = pollfds[0].revents(); + if !validate_poll_readiness(revents, "runtime wake", TerminalReadinessPolicy::Reject)? { return Err(io::Error::other(format!( "runtime wake poll reported readiness without a readable descriptor ({:#x})", - pollfd.revents + revents.bits() ))); } Ok(Some(())) @@ -154,6 +140,27 @@ impl RuntimeWakeSource { Ok(false) } } + + /// Reports whether a wake arrives within `timeout`, without draining it. + /// Any other readiness is an error, so a broken descriptor cannot pass for + /// silence in a test that expects no wake. + #[cfg(test)] + pub(crate) fn is_readable_within(&self, timeout: Duration) -> io::Result { + let readable = poll_with_retry(Some(timeout), |timeout_ms| { + let mut pollfds = [PollFd::new(&self.fd, PollFlags::IN)]; + if poll_for_ms(&mut pollfds, timeout_ms)? == 0 { + return Ok(None); + } + validate_poll_readiness( + pollfds[0].revents(), + "runtime wake", + TerminalReadinessPolicy::Reject, + ) + .map(Some) + })?; + + Ok(readable == Some(true)) + } } pub(super) fn timeout_to_poll_ms(timeout: Option) -> i32 { @@ -169,34 +176,22 @@ pub(super) fn timeout_to_poll_ms(timeout: Option) -> i32 { impl RuntimeWakeHandle { pub(crate) fn wake(&self) -> io::Result<()> { - let value = 1_u64; + let value = 1_u64.to_ne_bytes(); loop { - // SAFETY: value points to a readable u64 and the shared eventfd remains - // valid for the duration of this write. - let result = unsafe { - libc::write( - self.fd.as_raw_fd(), - (&raw const value).cast(), - size_of::(), - ) - }; - if result == size_of::() as isize { - return Ok(()); - } - if result < 0 { - let err = io::Error::last_os_error(); - match err.kind() { - io::ErrorKind::Interrupted => continue, - // A saturated eventfd is already readable, so the wake is - // successfully coalesced rather than lost. - io::ErrorKind::WouldBlock => return Ok(()), - _ => return Err(err), + match rustix_io::write(&self.fd, &value) { + Ok(written) if written == value.len() => return Ok(()), + Ok(written) => { + return Err(io::Error::new( + io::ErrorKind::WriteZero, + format!("runtime wake eventfd returned a short write ({written} bytes)"), + )); } + Err(Errno::INTR) => {} + // A saturated eventfd is already readable, so the wake is + // successfully coalesced rather than lost. + Err(Errno::WOULDBLOCK) => return Ok(()), + Err(errno) => return Err(errno.into()), } - return Err(io::Error::new( - io::ErrorKind::WriteZero, - format!("runtime wake eventfd returned a short write ({result} bytes)"), - )); } } } @@ -207,21 +202,13 @@ mod tests { use std::time::{Duration, Instant}; use std::{fs, thread}; + use rustix::pipe; + use rustix::process::Pid; + use rustix::thread as rustix_thread; + use super::*; - fn poll_readable(source: &RuntimeWakeSource, timeout_ms: i32) -> bool { - let mut pollfd = libc::pollfd { - fd: source.poll_fd().as_raw_fd(), - events: libc::POLLIN, - revents: 0, - }; - // SAFETY: pollfd is valid for the duration of this call. - let ready = unsafe { libc::poll(&raw mut pollfd, 1, timeout_ms) }; - assert!(ready >= 0, "poll failed: {}", io::Error::last_os_error()); - ready > 0 && pollfd.revents & libc::POLLIN != 0 - } - - fn wait_until_task_is_blocked_in_poll(tid: libc::pid_t) { + fn wait_until_task_is_blocked_in_poll(tid: Pid) { let stat_path = format!("/proc/self/task/{tid}/stat"); let deadline = Instant::now() + Duration::from_secs(1); loop { @@ -248,9 +235,9 @@ mod tests { let source = RuntimeWakeSource::try_new().unwrap(); source.handle().wake().unwrap(); - assert!(poll_readable(&source, 0)); + assert!(source.is_readable_within(Duration::ZERO).unwrap()); assert!(source.drain().unwrap()); - assert!(!poll_readable(&source, 0)); + assert!(!source.is_readable_within(Duration::ZERO).unwrap()); } #[test] @@ -259,10 +246,7 @@ mod tests { let handle = source.handle(); let (poll_entry_tx, poll_entry_rx) = mpsc::channel(); let poller = thread::spawn(move || { - // SAFETY: gettid has no preconditions and returns the calling Linux - // thread's id, used only to observe its scheduler state in this test. - let tid = unsafe { libc::syscall(libc::SYS_gettid) as libc::pid_t }; - poll_entry_tx.send(tid).unwrap(); + poll_entry_tx.send(rustix_thread::gettid()).unwrap(); assert!(source.wait_readable(Some(Duration::from_secs(1))).unwrap()); }); @@ -281,7 +265,7 @@ mod tests { } assert!(source.drain().unwrap()); - assert!(!poll_readable(&source, 0)); + assert!(!source.is_readable_within(Duration::ZERO).unwrap()); } #[test] @@ -299,6 +283,15 @@ mod tests { assert!(!source.drain().unwrap()); } + #[test] + fn readiness_without_a_wake_is_an_error_rather_than_silence() { + let (read, write) = pipe::pipe().unwrap(); + drop(write); + let source = RuntimeWakeSource { fd: Arc::new(read) }; + + assert!(source.is_readable_within(Duration::ZERO).is_err()); + } + #[test] fn wait_readable_times_out_without_a_wake() { let source = RuntimeWakeSource::try_new().unwrap(); @@ -307,22 +300,18 @@ mod tests { #[test] fn terminal_readiness_policy_distinguishes_streams_from_wake_descriptors() { - let pollfd = libc::pollfd { - fd: 7, - events: libc::POLLIN, - revents: libc::POLLIN | libc::POLLHUP, - }; + let revents = PollFlags::IN | PollFlags::HUP; assert!( validate_poll_readiness( - &pollfd, + revents, "buffered stream", TerminalReadinessPolicy::ReadBuffered, ) .unwrap() ); assert!( - validate_poll_readiness(&pollfd, "runtime wake", TerminalReadinessPolicy::Reject,) + validate_poll_readiness(revents, "runtime wake", TerminalReadinessPolicy::Reject) .is_err() ); } diff --git a/src/backend/wayland/backend/signals.rs b/src/backend/wayland/backend/signals.rs index 637bb00c..4d18e240 100644 --- a/src/backend/wayland/backend/signals.rs +++ b/src/backend/wayland/backend/signals.rs @@ -114,21 +114,13 @@ pub(super) fn setup_signal_handlers( #[cfg(test)] mod tests { - use std::os::fd::AsRawFd; + use std::time::Duration; use super::*; use crate::backend::wayland::RuntimeWakeSource; fn wait_for_wake(source: &RuntimeWakeSource) { - let mut pollfd = libc::pollfd { - fd: source.poll_fd().as_raw_fd(), - events: libc::POLLIN, - revents: 0, - }; - // SAFETY: source retains the descriptor throughout this bounded wait. - let ready = unsafe { libc::poll(&raw mut pollfd, 1, 1_000) }; - assert_eq!(ready, 1); - assert_ne!(pollfd.revents & libc::POLLIN, 0); + assert!(source.is_readable_within(Duration::from_secs(1)).unwrap()); source.drain().unwrap(); } diff --git a/src/backend/wayland/clipboard/file_list.rs b/src/backend/wayland/clipboard/file_list.rs index df30b8a2..5920001d 100644 --- a/src/backend/wayland/clipboard/file_list.rs +++ b/src/backend/wayland/clipboard/file_list.rs @@ -205,12 +205,11 @@ fn is_gnome_copied_files_mime(mime_type: &str) -> bool { #[cfg(test)] mod tests { - use std::io; use std::os::unix; use super::*; use crate::backend::wayland::clipboard::image::choose_supported_mime; - use crate::test_temp::TempDir; + use crate::test_temp::{self, TempDir}; use std::fs; #[test] @@ -265,16 +264,9 @@ mod tests { #[test] #[cfg(unix)] fn uri_list_paste_rejects_fifo_without_blocking() { - use std::ffi::CString; - use std::os::unix::ffi::OsStrExt; - let temp = TempDir::new().unwrap(); let fifo_path = temp.path().join("not-an-image"); - let c_path = CString::new(fifo_path.as_os_str().as_bytes()).unwrap(); - // SAFETY: `c_path` is a NUL-terminated string that outlives the call, - // and mkfifo only reads it. - let result = unsafe { libc::mkfifo(c_path.as_ptr(), 0o600) }; - assert_eq!(result, 0, "mkfifo failed: {}", io::Error::last_os_error()); + test_temp::make_fifo(&fifo_path); let uri = file_uri_for_path(&fifo_path); let offered = vec![TEXT_URI_LIST_MIME.to_string()]; diff --git a/src/backend/wayland/input_monitor/monitor.rs b/src/backend/wayland/input_monitor/monitor.rs index 40ec2314..62022db9 100644 --- a/src/backend/wayland/input_monitor/monitor.rs +++ b/src/backend/wayland/input_monitor/monitor.rs @@ -16,7 +16,9 @@ //! device or keymap problem can never panic across the libinput FFI edge. use std::io; -use std::os::fd::{AsRawFd, OwnedFd}; +use std::os::fd::OwnedFd; + +use rustix::io as rustix_io; use std::sync::mpsc::{Receiver, TryRecvError, channel}; use std::thread::{self, JoinHandle}; use std::time::{Duration, Instant}; @@ -188,15 +190,10 @@ impl InputMonitor { /// A reader wedged inside libinput is left to finish on its own rather than /// blocking overlay shutdown; its descriptors close with the process. fn stop(&mut self) { - let byte = [1_u8]; - // SAFETY: the write end is owned and open for the duration of the call, - // and `byte` is a valid one-byte buffer. - let written = - unsafe { libc::write(self.stop.as_raw_fd(), byte.as_ptr().cast(), byte.len()) }; - if written < 0 { + if let Err(errno) = rustix_io::write(&self.stop, &[1]) { log::warn!( "Failed to signal the system input reader: {}", - io::Error::last_os_error() + io::Error::from(errno) ); } let Some(join) = self.join.take() else { diff --git a/src/backend/wayland/input_monitor/monitor/worker.rs b/src/backend/wayland/input_monitor/monitor/worker.rs index d15741a0..3649399d 100644 --- a/src/backend/wayland/input_monitor/monitor/worker.rs +++ b/src/backend/wayland/input_monitor/monitor/worker.rs @@ -13,9 +13,12 @@ use input::event::pointer::{Axis, ButtonState, PointerScrollEvent}; use input::event::tablet_tool::{TabletToolEvent, TipState}; use input::event::{Event, EventTrait, KeyboardEvent, PointerEvent}; use input::{DeviceCapability, Libinput, LibinputInterface}; +use rustix::event::{self, PollFd, PollFlags, Timespec}; +use rustix::io::Errno; +use rustix::pipe::{self, PipeFlags}; use std::fs::{File, OpenOptions}; use std::io; -use std::os::fd::{AsRawFd, FromRawFd, OwnedFd, RawFd}; +use std::os::fd::OwnedFd; use std::os::unix::fs::OpenOptionsExt; use std::path::Path; use std::sync::mpsc::Sender; @@ -28,17 +31,7 @@ use repeat::{KeyboardKeys, RepeatPress, RepeatState}; /// Non-blocking, close-on-exec pipe used only to interrupt the reader's poll. pub(super) fn stop_pipe() -> io::Result<(OwnedFd, OwnedFd)> { - let mut fds = [0 as RawFd; 2]; - // SAFETY: `fds` is a writable two-element array, the shape pipe2(2) expects. - let result = unsafe { libc::pipe2(fds.as_mut_ptr(), libc::O_CLOEXEC) }; - if result < 0 { - return Err(io::Error::last_os_error()); - } - // SAFETY: both descriptors were just returned by pipe2 and are unwrapped. - let read = unsafe { OwnedFd::from_raw_fd(fds[0]) }; - // SAFETY: as above for the write end. - let write = unsafe { OwnedFd::from_raw_fd(fds[1]) }; - Ok((read, write)) + pipe::pipe_with(PipeFlags::CLOEXEC).map_err(io::Error::from) } /// Open evdev nodes for libinput. Unprivileged `open(2)`: it succeeds when the @@ -239,47 +232,33 @@ fn wait_for_input( stop_read: &OwnedFd, timeout: Option, ) -> io::Result { - // A repeat deadline in the past polls with 0 and returns Timeout at once. - let timeout_ms = timeout.map_or(-1, |timeout| { - timeout.as_millis().min(i32::MAX as u128) as i32 - }); + // A repeat deadline in the past polls with zero and returns Timeout at once. + let timeout = timeout + .map(Timespec::try_from) + .transpose() + .map_err(io::Error::other)?; loop { let mut fds = [ - libc::pollfd { - fd: libinput.as_raw_fd(), - events: libc::POLLIN, - revents: 0, - }, - libc::pollfd { - fd: stop_read.as_raw_fd(), - events: libc::POLLIN, - revents: 0, - }, + PollFd::new(libinput, PollFlags::IN), + PollFd::new(stop_read, PollFlags::IN), ]; - // SAFETY: `fds` is a valid two-element array and both descriptors are - // owned (or borrowed from an owner) for the duration of the call. A - // negative timeout blocks until one becomes ready. - let ready = unsafe { libc::poll(fds.as_mut_ptr(), 2, timeout_ms) }; - if ready < 0 { - let err = io::Error::last_os_error(); - if err.kind() == io::ErrorKind::Interrupted { - continue; - } - return Err(err); + match event::poll(&mut fds, timeout.as_ref()) { + Ok(0) => return Ok(PollOutcome::Timeout), + Ok(_) => {} + Err(Errno::INTR) => continue, + Err(errno) => return Err(errno.into()), } - if ready == 0 { - return Ok(PollOutcome::Timeout); - } - if fds[1].revents != 0 { + if !fds[1].revents().is_empty() { return Ok(PollOutcome::Stop); } - if fds[0].revents & (libc::POLLERR | libc::POLLNVAL | libc::POLLHUP) != 0 { + let revents = fds[0].revents(); + if revents.intersects(PollFlags::ERR | PollFlags::NVAL | PollFlags::HUP) { return Err(io::Error::other(format!( "libinput poll descriptor failed with readiness {:#x}", - fds[0].revents + revents.bits() ))); } - if fds[0].revents & libc::POLLIN != 0 { + if revents.contains(PollFlags::IN) { return Ok(PollOutcome::Ready); } } diff --git a/src/backend/wayland/input_monitor/probe.rs b/src/backend/wayland/input_monitor/probe.rs index 8c4ef7d9..7d536e1c 100644 --- a/src/backend/wayland/input_monitor/probe.rs +++ b/src/backend/wayland/input_monitor/probe.rs @@ -5,14 +5,14 @@ //! silently pick overlay mode and `system` can warn with actionable guidance //! before anything is spawned. -use std::ffi::CString; -use std::os::unix::ffi::OsStrExt; +use rustix::fs::{self as rustix_fs, Access}; +use rustix::io::Errno; use std::path::Path; use crate::env_vars; use std::ffi::OsString; use std::io::ErrorKind; -use std::{env, fs, io}; +use std::{env, fs}; const DEV_INPUT_DIR: &str = "/dev/input"; /// Seat to follow when the session does not name one. libinput's udev backend @@ -211,18 +211,11 @@ fn classify_seat_nodes(seat: &str) -> EventNodeAccess { } fn node_path_access(path: &Path) -> NodePathAccess { - let Ok(c_path) = CString::new(path.as_os_str().as_bytes()) else { - return NodePathAccess::Unknown; - }; - // SAFETY: `c_path` is a valid NUL-terminated C string that outlives the - // call, and `access(2)` only reads it. - if unsafe { libc::access(c_path.as_ptr(), libc::R_OK) } == 0 { - return NodePathAccess::Readable; - } - match io::Error::last_os_error().raw_os_error() { - Some(libc::EACCES | libc::EPERM) => NodePathAccess::PermissionDenied, - Some(libc::ENOENT | libc::ENOTDIR | libc::ENODEV) => NodePathAccess::Absent, - _ => NodePathAccess::Unknown, + match rustix_fs::access(path, Access::READ_OK) { + Ok(()) => NodePathAccess::Readable, + Err(Errno::ACCESS | Errno::PERM) => NodePathAccess::PermissionDenied, + Err(Errno::NOENT | Errno::NOTDIR | Errno::NODEV) => NodePathAccess::Absent, + Err(_) => NodePathAccess::Unknown, } } diff --git a/src/backend/wayland/portal_task.rs b/src/backend/wayland/portal_task.rs index 398018fe..61fd2763 100644 --- a/src/backend/wayland/portal_task.rs +++ b/src/backend/wayland/portal_task.rs @@ -217,7 +217,6 @@ impl Drop for PortalTaskExitGuard { #[cfg(test)] mod tests { use std::future; - use std::os::fd::AsRawFd; use tokio::task; @@ -225,13 +224,7 @@ mod tests { use crate::backend::wayland::RuntimeWakeSource; fn wait_for_wake(wake: &RuntimeWakeSource) { - let mut pollfd = libc::pollfd { - fd: wake.poll_fd().as_raw_fd(), - events: libc::POLLIN, - revents: 0, - }; - // SAFETY: the descriptor and pollfd remain valid during the bounded wait. - assert_eq!(unsafe { libc::poll(&raw mut pollfd, 1, 1_000) }, 1); + assert!(wake.is_readable_within(Duration::from_secs(1)).unwrap()); } #[tokio::test(flavor = "multi_thread")] @@ -264,14 +257,7 @@ mod tests { }); drop(task); task::yield_now().await; - let mut pollfd = libc::pollfd { - fd: wake.poll_fd().as_raw_fd(), - events: libc::POLLIN, - revents: 0, - }; - // SAFETY: the descriptor and pollfd are valid for this non-blocking poll. - let ready = unsafe { libc::poll(&raw mut pollfd, 1, 0) }; - assert_eq!(ready, 0); + assert!(!wake.is_readable_within(Duration::ZERO).unwrap()); } #[tokio::test] diff --git a/src/backend/wayland/runtime_operation.rs b/src/backend/wayland/runtime_operation.rs index 4d4c98cf..71794a1a 100644 --- a/src/backend/wayland/runtime_operation.rs +++ b/src/backend/wayland/runtime_operation.rs @@ -379,12 +379,14 @@ impl Drop for RuntimeOperationExitGuard { #[cfg(test)] mod tests { - use std::os::fd::AsRawFd; use std::sync::atomic::{AtomicUsize, Ordering}; use std::sync::mpsc; use std::time::{Duration, Instant}; use std::{fs, thread}; + use rustix::process::Pid; + use rustix::thread as rustix_thread; + use super::*; use crate::backend::wayland::RuntimeWakeSource; @@ -396,17 +398,10 @@ mod tests { } fn wait_for_wake(wake: &RuntimeWakeSource) { - let mut pollfd = libc::pollfd { - fd: wake.poll_fd().as_raw_fd(), - events: libc::POLLIN, - revents: 0, - }; - // SAFETY: pollfd and the runtime wake descriptor remain valid for the bounded wait. - assert_eq!(unsafe { libc::poll(&raw mut pollfd, 1, 1_000) }, 1); - assert_ne!(pollfd.revents & libc::POLLIN, 0); + assert!(wake.is_readable_within(Duration::from_secs(1)).unwrap()); } - fn wait_until_thread_is_blocked_in_poll(tid: libc::pid_t) { + fn wait_until_thread_is_blocked_in_poll(tid: Pid) { let stat_path = format!("/proc/self/task/{tid}/stat"); let deadline = Instant::now() + Duration::from_secs(1); loop { @@ -516,9 +511,7 @@ mod tests { .unwrap(); let (tid_tx, tid_rx) = mpsc::channel(); let poller = thread::spawn(move || { - // SAFETY: gettid has no preconditions and is used only to observe this test thread. - let tid = unsafe { libc::syscall(libc::SYS_gettid) as libc::pid_t }; - tid_tx.send(tid).unwrap(); + tid_tx.send(rustix_thread::gettid()).unwrap(); wait_for_wake(&wake); }); let tid = tid_rx.recv().unwrap(); @@ -592,13 +585,7 @@ mod tests { ) ); assert!(!controller.is_active()); - let mut pollfd = libc::pollfd { - fd: wake.poll_fd().as_raw_fd(), - events: libc::POLLIN, - revents: 0, - }; - // SAFETY: pollfd and the runtime wake descriptor are valid for this non-blocking poll. - assert_eq!(unsafe { libc::poll(&raw mut pollfd, 1, 0) }, 0); + assert!(!wake.is_readable_within(Duration::ZERO).unwrap()); assert_eq!( controller .try_submit(42, "test-after-spawn-failure", || 2) diff --git a/src/backend/wayland/session/persistence.rs b/src/backend/wayland/session/persistence.rs index f26d2ee9..94d9bbc1 100644 --- a/src/backend/wayland/session/persistence.rs +++ b/src/backend/wayland/session/persistence.rs @@ -470,9 +470,7 @@ fn assert_send_static() {} #[cfg(test)] mod tests { - use std::io::ErrorKind; - use std::os::fd::AsRawFd; - use std::{fs, io}; + use std::fs; use crate::test_temp; @@ -503,27 +501,10 @@ mod tests { } fn wait_for_runtime_wake(wake: &RuntimeWakeSource) { - let mut pollfd = libc::pollfd { - fd: wake.poll_fd().as_raw_fd(), - events: libc::POLLIN, - revents: 0, - }; - loop { - // SAFETY: pollfd and the runtime-owned descriptor remain valid for - // this bounded test wait. - let ready = unsafe { libc::poll(&raw mut pollfd, 1, 1_000) }; - if ready > 0 { - assert_ne!(pollfd.revents & libc::POLLIN, 0); - return; - } - if ready == 0 { - panic!("persistence worker did not wake runtime within one second"); - } - let err = io::Error::last_os_error(); - if err.kind() != ErrorKind::Interrupted { - panic!("runtime wake poll failed: {err}"); - } - } + assert!( + wake.is_readable_within(Duration::from_secs(1)).unwrap(), + "persistence worker did not wake runtime within one second" + ); } #[test] diff --git a/src/backend/wayland/state/keybindings/implementation/tests/persistence.rs b/src/backend/wayland/state/keybindings/implementation/tests/persistence.rs index 63db8a1f..fcf75a95 100644 --- a/src/backend/wayland/state/keybindings/implementation/tests/persistence.rs +++ b/src/backend/wayland/state/keybindings/implementation/tests/persistence.rs @@ -1,3 +1,5 @@ +use rustix::process as rustix_process; + use crate::config::ConfigDocument; use crate::{config, test_temp}; @@ -387,8 +389,7 @@ fn a_read_only_config_fails_the_write_and_leaves_the_file_alone() { fs::set_permissions(temp.path(), fs::Permissions::from_mode(0o755)).expect("restore dir"); fs::set_permissions(&path, fs::Permissions::from_mode(0o644)).expect("restore file"); - // SAFETY: geteuid has no preconditions and cannot fail. - if unsafe { libc::geteuid() } == 0 { + if rustix_process::geteuid().is_root() { // Root ignores these modes; the scenario cannot be staged. return; } diff --git a/src/backend/wayland/state/toolbar/events/presets.rs b/src/backend/wayland/state/toolbar/events/presets.rs index 19324ed4..1ce43d72 100644 --- a/src/backend/wayland/state/toolbar/events/presets.rs +++ b/src/backend/wayland/state/toolbar/events/presets.rs @@ -136,6 +136,8 @@ impl WaylandState { mod tests { use std::path::{Path, PathBuf}; + use rustix::process as rustix_process; + use crate::config::ConfigDocument; use crate::input::Tool; use crate::{config, test_temp}; @@ -367,8 +369,7 @@ redo = [\"Ctrl+Alt+Shift+Q\"] fs::set_permissions(temp.path(), fs::Permissions::from_mode(0o755)).expect("restore dir"); fs::set_permissions(&path, fs::Permissions::from_mode(0o644)).expect("restore file"); - // SAFETY: geteuid has no preconditions and cannot fail. - if unsafe { libc::geteuid() } == 0 { + if rustix_process::geteuid().is_root() { return; } assert!(result.is_err(), "a read-only config must fail the write"); diff --git a/src/backend/wayland/state/toolbar/events/quick_colors.rs b/src/backend/wayland/state/toolbar/events/quick_colors.rs index 6fb19cbc..01f4e359 100644 --- a/src/backend/wayland/state/toolbar/events/quick_colors.rs +++ b/src/backend/wayland/state/toolbar/events/quick_colors.rs @@ -122,6 +122,8 @@ impl WaylandState { mod tests { use std::path::{Path, PathBuf}; + use rustix::process as rustix_process; + use crate::config::{ColorSpec, ConfigDocument}; use crate::{config, test_temp}; @@ -343,8 +345,7 @@ color = \"#33F54F\" fs::set_permissions(temp.path(), fs::Permissions::from_mode(0o755)).expect("restore dir"); fs::set_permissions(&path, fs::Permissions::from_mode(0o644)).expect("restore file"); - // SAFETY: geteuid has no preconditions and cannot fail. - if unsafe { libc::geteuid() } == 0 { + if rustix_process::geteuid().is_root() { return; } assert!(result.is_err(), "a read-only config must fail the write"); diff --git a/src/capture/tests/manager.rs b/src/capture/tests/manager.rs index e1edace2..623e9545 100644 --- a/src/capture/tests/manager.rs +++ b/src/capture/tests/manager.rs @@ -1,4 +1,3 @@ -use std::os::fd::AsRawFd; use std::path::PathBuf; use std::sync::atomic::{AtomicUsize, Ordering}; use std::sync::{Arc, Mutex, mpsc}; @@ -163,23 +162,19 @@ async fn completion_wakes_an_already_blocked_runtime_poll_after_publication() { .unwrap(); started.notified().await; - let wake_fd = runtime_wake.poll_fd().as_raw_fd(); let (polling_tx, polling_rx) = mpsc::sync_channel(0); let poller = thread::spawn(move || { - let mut pollfd = libc::pollfd { - fd: wake_fd, - events: libc::POLLIN, - revents: 0, - }; polling_tx.send(()).unwrap(); - // SAFETY: the runtime wake source outlives this bounded poll and owns - // the descriptor for the entire test. - assert_eq!(unsafe { libc::poll(&raw mut pollfd, 1, 1_000) }, 1); - assert_ne!(pollfd.revents & libc::POLLIN, 0); + assert!( + runtime_wake + .is_readable_within(Duration::from_secs(1)) + .unwrap() + ); + runtime_wake }); polling_rx.recv().unwrap(); release.notify_one(); - poller.join().unwrap(); + let runtime_wake = poller.join().unwrap(); runtime_wake.drain().unwrap(); assert!(matches!( diff --git a/src/cli.rs b/src/cli.rs index d58b8d8e..ec2a87ff 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -2,6 +2,8 @@ use std::env; use std::ffi::OsString; use std::path::PathBuf; +use lexopt::{Arg, Parser}; + use crate::domain::Language; use crate::tray_action::TrayAction; use crate::{build_info, i18n, runtime_capabilities}; @@ -105,96 +107,70 @@ impl Cli { I: IntoIterator, T: Into, { - let mut args = args - .into_iter() - .map(|arg| { - arg.into() - .into_string() - .map_err(|_| "arguments must be valid UTF-8".to_string()) - }) - .collect::, _>>()?; - - if args.is_empty() { - args.push("wayscriber".to_string()); - } - + let mut parser = Parser::from_iter(args.into_iter().map(Into::into)); let mut cli = Cli::default(); - let mut index = 1; - while index < args.len() { - let arg = &args[index]; - match arg.as_str() { - "-h" | "--help" => return Ok(CliOutcome::Help), - "-V" | "--version" => return Ok(CliOutcome::Version), - "-d" | "--daemon" => cli.daemon = true, - "--daemon-toggle" => cli.daemon_toggle = true, - "--daemon-action" => { - index += 1; - cli.daemon_action = Some(value_after(&args, index, "--daemon-action")?); + + while let Some(arg) = parser.next().map_err(|error| parse_error(&error))? { + match arg { + Arg::Short('h') => return Ok(CliOutcome::Help), + Arg::Short('V') => return Ok(CliOutcome::Version), + Arg::Long("help") => { + reject_attached_value(&mut parser, "--help")?; + return Ok(CliOutcome::Help); } - "--light-toggle" => cli.light_toggle = true, - "--light-draw-toggle" => cli.light_draw_toggle = true, - "--light-draw-on" => cli.light_draw_on = true, - "--light-draw-off" => cli.light_draw_off = true, - "-a" | "--active" => cli.active = true, - "-m" | "--mode" => { - index += 1; - cli.mode = Some(value_after(&args, index, "--mode")?); + Arg::Long("version") => { + reject_attached_value(&mut parser, "--version")?; + return Ok(CliOutcome::Version); } - "--no-tray" => cli.no_tray = true, - "--freeze-on-show" => cli.freeze_on_show = true, - "--clear-session" => cli.clear_session = true, - "--clear-tool-state" => cli.clear_tool_state = true, - "--session-info" => cli.session_info = true, - "--rename-session" => { - index += 1; - cli.rename_session = Some(value_after(&args, index, "--rename-session")?); + Arg::Short('d') | Arg::Long("daemon") => cli.daemon = true, + Arg::Long("daemon-toggle") => cli.daemon_toggle = true, + Arg::Long("daemon-action") => { + cli.daemon_action = Some(string_value(&mut parser, "--daemon-action")?); } - "--session-file" => { - index += 1; + Arg::Long("light-toggle") => cli.light_toggle = true, + Arg::Long("light-draw-toggle") => cli.light_draw_toggle = true, + Arg::Long("light-draw-on") => cli.light_draw_on = true, + Arg::Long("light-draw-off") => cli.light_draw_off = true, + Arg::Short('a') | Arg::Long("active") => cli.active = true, + Arg::Short('m') => cli.mode = Some(string_value(&mut parser, "-m")?), + Arg::Long("mode") => cli.mode = Some(string_value(&mut parser, "--mode")?), + Arg::Long("no-tray") => cli.no_tray = true, + Arg::Long("freeze-on-show") => cli.freeze_on_show = true, + Arg::Long("clear-session") => cli.clear_session = true, + Arg::Long("clear-tool-state") => cli.clear_tool_state = true, + Arg::Long("session-info") => cli.session_info = true, + Arg::Long("rename-session") => { + cli.rename_session = Some(string_value(&mut parser, "--rename-session")?); + } + Arg::Long("session-file") => { cli.session_file = - Some(PathBuf::from(value_after(&args, index, "--session-file")?)); + Some(PathBuf::from(option_value(&mut parser, "--session-file")?)); } - "--freeze" => cli.freeze = true, - "--exit-after-capture" => cli.exit_after_capture = true, - "--no-exit-after-capture" => cli.no_exit_after_capture = true, - "--resume-session" => cli.resume_session = true, - "--no-resume-session" => cli.no_resume_session = true, - "--about" => cli.about = true, - "--check-update" => cli.check_update = true, - "--language" => { - index += 1; - cli.language = Some(parse_language(&value_after(&args, index, "--language")?)?); + Arg::Long("freeze") => cli.freeze = true, + Arg::Long("exit-after-capture") => cli.exit_after_capture = true, + Arg::Long("no-exit-after-capture") => cli.no_exit_after_capture = true, + Arg::Long("resume-session") => cli.resume_session = true, + Arg::Long("no-resume-session") => cli.no_resume_session = true, + Arg::Long("about") => cli.about = true, + Arg::Long("check-update") => cli.check_update = true, + Arg::Long("language") => { + cli.language = Some(parse_language(&string_value(&mut parser, "--language")?)?); } - "--list-languages" => cli.list_languages = true, - runtime_capabilities::RUNTIME_CAPABILITIES_FLAG => { + Arg::Long("list-languages") => cli.list_languages = true, + Arg::Long(name) + if runtime_capabilities::RUNTIME_CAPABILITIES_FLAG.strip_prefix("--") + == Some(name) => + { cli.runtime_capabilities = true; } - _ if arg.starts_with("--daemon-action=") => { - cli.daemon_action = Some(value_from_equals(arg, "--daemon-action")?); + Arg::Long(name) => { + let option = format!("--{name}"); + reject_attached_value(&mut parser, &option)?; + return Err(unknown_argument(&option)); } - _ if arg.starts_with("--mode=") => { - cli.mode = Some(value_from_equals(arg, "--mode")?); - } - _ if arg.starts_with("--session-file=") => { - cli.session_file = - Some(PathBuf::from(value_from_equals(arg, "--session-file")?)); - } - _ if arg.starts_with("--rename-session=") => { - cli.rename_session = Some(value_from_equals(arg, "--rename-session")?); - } - _ if arg.starts_with("--language=") => { - cli.language = Some(parse_language(&value_from_equals(arg, "--language")?)?); - } - _ if is_short_option_cluster(arg) => { - match parse_short_option_cluster(&args, index, &mut cli)? { - ShortOptionOutcome::Continue(next_index) => index = next_index, - ShortOptionOutcome::Help => return Ok(CliOutcome::Help), - ShortOptionOutcome::Version => return Ok(CliOutcome::Version), - } - } - _ => return Err(format!("unknown argument '{arg}'")), + Arg::Short(flag) => return Err(format!("unknown short option '-{flag}'")), + Arg::Value(value) => return Err(unknown_argument(&value.display().to_string())), } - index += 1; } cli.validate()?; @@ -525,77 +501,53 @@ fn parse_language(value: &str) -> Result { }) } -fn is_short_option_cluster(arg: &str) -> bool { - arg.starts_with('-') && !arg.starts_with("--") && arg.len() > 2 +/// Reads an option's value, either attached (`--mode=x`, `-mx`, `-m=x`) or as +/// the next argument. A next argument starting with `-` is the next option, so +/// a value that starts with `-` has to be attached. +fn option_value(parser: &mut Parser, name: &str) -> Result { + let requires_value = || format!("{name} requires a value"); + + if let Some(value) = parser.optional_value() { + return if value.is_empty() { + Err(requires_value()) + } else { + Ok(value) + }; + } + + let value = parser.value().map_err(|_| requires_value())?; + if value.as_encoded_bytes().starts_with(b"-") { + return Err(requires_value()); + } + Ok(value) } -enum ShortOptionOutcome { - Continue(usize), - Help, - Version, +fn string_value(parser: &mut Parser, name: &str) -> Result { + option_value(parser, name)? + .into_string() + .map_err(|_| "arguments must be valid UTF-8".to_string()) } -fn parse_short_option_cluster( - args: &[String], - index: usize, - cli: &mut Cli, -) -> Result { - let arg = &args[index]; - for (offset, flag) in arg[1..].char_indices() { - match flag { - 'h' => return Ok(ShortOptionOutcome::Help), - 'V' => return Ok(ShortOptionOutcome::Version), - 'd' => cli.daemon = true, - 'a' => cli.active = true, - 'm' => { - let value_start = 1 + offset + flag.len_utf8(); - let value = if value_start < arg.len() { - attached_short_value(arg, value_start, "-m")? - } else { - value_after(args, index + 1, "-m")? - }; - cli.mode = Some(value); - return Ok(ShortOptionOutcome::Continue(if value_start < arg.len() { - index - } else { - index + 1 - })); - } - _ => return Err(format!("unknown short option '-{flag}'")), +/// Rejects `--flag=value` for an option that is about to stop parsing, which +/// would otherwise skip the check `Parser::next` makes on its following call. +fn reject_attached_value(parser: &mut Parser, name: &str) -> Result<(), String> { + match parser.optional_value() { + Some(value) => Err(unknown_argument(&format!("{name}={}", value.display()))), + None => Ok(()), + } +} + +fn parse_error(error: &lexopt::Error) -> String { + match error { + lexopt::Error::UnexpectedValue { option, value } => { + unknown_argument(&format!("{option}={}", value.display())) } + other => other.to_string(), } - Ok(ShortOptionOutcome::Continue(index)) } -fn value_after(args: &[String], index: usize, name: &str) -> Result { - let value = args - .get(index) - .ok_or_else(|| format!("{name} requires a value"))?; - if value.starts_with('-') { - return Err(format!("{name} requires a value")); - } - Ok(value.clone()) -} - -fn attached_short_value(arg: &str, value_start: usize, name: &str) -> Result { - let value = arg[value_start..] - .strip_prefix('=') - .unwrap_or_else(|| &arg[value_start..]); - if value.is_empty() { - return Err(format!("{name} requires a value")); - } - Ok(value.to_string()) -} - -fn value_from_equals(arg: &str, name: &str) -> Result { - let value = arg - .split_once('=') - .map(|(_, value)| value) - .unwrap_or_default(); - if value.is_empty() { - return Err(format!("{name} requires a value")); - } - Ok(value.to_string()) +fn unknown_argument(arg: &str) -> String { + format!("unknown argument '{arg}'") } fn conflict(left: &str, right: &str) -> String { diff --git a/src/cli/tests.rs b/src/cli/tests.rs index 3d28a639..cbe12ea9 100644 --- a/src/cli/tests.rs +++ b/src/cli/tests.rs @@ -1,3 +1,5 @@ +use std::ffi::OsString; +use std::os::unix::ffi::OsStringExt; use std::path::PathBuf; use super::{Cli, CliOutcome}; @@ -601,3 +603,104 @@ fn list_languages_is_a_print_and_exit_command() { ); } } + +#[test] +fn a_value_starting_with_a_dash_must_be_attached() { + for args in [ + &["wayscriber", "--mode", "-x"][..], + &["wayscriber", "-m", "-x"], + &["wayscriber", "--mode", "--active"], + ] { + assert!( + Cli::try_parse_from(args.iter().copied()) + .unwrap_err() + .ends_with("requires a value"), + "{args:?}" + ); + } + + assert_eq!( + parse_cli(["wayscriber", "--mode=-x"]).mode.as_deref(), + Some("-x") + ); + assert_eq!( + parse_cli(["wayscriber", "-m=-x"]).mode.as_deref(), + Some("-x") + ); + assert_eq!( + parse_cli(["wayscriber", "-m-x"]).mode.as_deref(), + Some("-x") + ); +} + +#[test] +fn an_empty_attached_value_is_missing_but_a_separate_one_is_not() { + for (args, name) in [ + (&["wayscriber", "--mode="][..], "--mode"), + (&["wayscriber", "-m="], "-m"), + (&["wayscriber", "--language="], "--language"), + ] { + assert_eq!( + Cli::try_parse_from(args.iter().copied()).unwrap_err(), + format!("{name} requires a value") + ); + } + + assert_eq!( + parse_cli(["wayscriber", "--mode", ""]).mode.as_deref(), + Some("") + ); +} + +#[test] +fn unknown_and_misused_options_are_rejected() { + for (args, expected) in [ + (&["wayscriber", "--nope"][..], "unknown argument '--nope'"), + (&["wayscriber", "--nope=1"], "unknown argument '--nope=1'"), + ( + &["wayscriber", "--active=1"], + "unknown argument '--active=1'", + ), + (&["wayscriber", "--help=1"], "unknown argument '--help=1'"), + ( + &["wayscriber", "--version=1"], + "unknown argument '--version=1'", + ), + (&["wayscriber", "stray"], "unknown argument 'stray'"), + (&["wayscriber", "-"], "unknown argument '-'"), + (&["wayscriber", "--", "stray"], "unknown argument 'stray'"), + (&["wayscriber", "-x"], "unknown short option '-x'"), + (&["wayscriber", "-dx"], "unknown short option '-x'"), + ] { + assert_eq!( + Cli::try_parse_from(args.iter().copied()).unwrap_err(), + expected, + "{args:?}" + ); + } +} + +#[test] +fn a_session_path_keeps_its_bytes_but_other_values_must_be_utf8() { + let raw_path = OsString::from_vec(b"/tmp/s\xff.wayscriber-session".to_vec()); + + let outcome = Cli::try_parse_from([ + OsString::from("wayscriber"), + OsString::from("--active"), + OsString::from("--session-file"), + raw_path.clone(), + ]) + .unwrap(); + let CliOutcome::Run(cli) = outcome else { + panic!("expected CLI run outcome, got {outcome:?}"); + }; + assert_eq!(cli.session_file, Some(PathBuf::from(raw_path))); + + let error = Cli::try_parse_from([ + OsString::from("wayscriber"), + OsString::from("--mode"), + OsString::from_vec(b"\xff".to_vec()), + ]) + .unwrap_err(); + assert_eq!(error, "arguments must be valid UTF-8"); +} diff --git a/src/daemon/control/runtime.rs b/src/daemon/control/runtime.rs index 01923084..22b6331c 100644 --- a/src/daemon/control/runtime.rs +++ b/src/daemon/control/runtime.rs @@ -6,6 +6,8 @@ use crate::paths::{daemon_lock_file, daemon_pid_file}; use crate::session::try_lock_exclusive; use anyhow::{Context, Result, anyhow}; use log::warn; +#[cfg(unix)] +use rustix::process::{self as rustix_process, Pid, Signal}; use std::fs; use std::fs::{File, OpenOptions}; use std::io::ErrorKind; @@ -136,16 +138,13 @@ pub(super) fn signal_daemon_pid(pid: u32) -> Result<()> { return Err(anyhow!("invalid daemon pid {pid}")); } - // SAFETY: `pid` has been checked to be a positive Unix process id and - // `SIGUSR1` is a valid signal constant. - if unsafe { libc::kill(pid, libc::SIGUSR1) } != 0 { - return Err(anyhow!( - "failed to signal wayscriber daemon {}: {}", - pid, - io::Error::last_os_error() - )); - } - Ok(()) + let target = Pid::from_raw(pid).ok_or_else(|| anyhow!("invalid daemon pid {pid}"))?; + rustix_process::kill_process(target, Signal::USR1).map_err(|errno| { + anyhow!( + "failed to signal wayscriber daemon {pid}: {}", + io::Error::from(errno) + ) + }) } #[cfg(not(unix))] diff --git a/src/daemon/core.rs b/src/daemon/core.rs index 2daa4b31..b1a84568 100644 --- a/src/daemon/core.rs +++ b/src/daemon/core.rs @@ -3,10 +3,12 @@ use std::{io, mem, process, thread}; use anyhow::{Context, Result}; use log::{info, warn}; +use rustix::event::{self, PollFd, PollFlags}; +use rustix::io::Errno; use std::fs; use std::fs::OpenOptions; use std::io::ErrorKind; -use std::os::fd::{AsRawFd, BorrowedFd}; +use std::os::fd::BorrowedFd; use std::path::PathBuf; use std::sync::Arc; use std::sync::atomic::{AtomicBool, AtomicU8, Ordering}; @@ -922,48 +924,31 @@ fn wait_for_daemon_lifecycle( deadline_source: Option<&BootDeadlineSource>, overlay_exit_fd: Option>, ) -> Result { - let mut pollfds = vec![libc::pollfd { - fd: daemon_wake.poll_fd().as_raw_fd(), - events: libc::POLLIN, - revents: 0, - }]; + let mut pollfds = vec![PollFd::from_borrowed_fd( + daemon_wake.poll_fd(), + PollFlags::IN, + )]; if let Some(watcher) = command_watcher { - pollfds.push(libc::pollfd { - fd: watcher.poll_fd().as_raw_fd(), - events: libc::POLLIN, - revents: 0, - }); + pollfds.push(PollFd::from_borrowed_fd(watcher.poll_fd(), PollFlags::IN)); } let command_index = command_watcher.map(|_| 1); let deadline_index = deadline_source.map(|source| { let index = pollfds.len(); - pollfds.push(libc::pollfd { - fd: source.poll_fd().as_raw_fd(), - events: libc::POLLIN, - revents: 0, - }); + pollfds.push(PollFd::from_borrowed_fd(source.poll_fd(), PollFlags::IN)); index }); let child_index = overlay_exit_fd.map(|fd| { let index = pollfds.len(); - pollfds.push(libc::pollfd { - fd: fd.as_raw_fd(), - events: libc::POLLIN, - revents: 0, - }); + pollfds.push(PollFd::from_borrowed_fd(fd, PollFlags::IN)); index }); loop { - // SAFETY: the descriptor remains owned by `daemon_wake` throughout poll. - let ready = unsafe { - libc::poll( - pollfds.as_mut_ptr(), - pollfds - .len() - .try_into() - .expect("poll descriptor count fits"), - -1, - ) + let ready = match event::poll(&mut pollfds, None) { + Ok(ready) => ready, + Err(Errno::INTR) => continue, + Err(errno) => { + return Err(io::Error::from(errno)).context("daemon lifecycle poll failed"); + } }; if ready == 0 { return Ok(DaemonLifecycleReadiness { @@ -971,36 +956,28 @@ fn wait_for_daemon_lifecycle( deadline: false, }); } - if ready < 0 { - let err = io::Error::last_os_error(); - if err.kind() == ErrorKind::Interrupted { - continue; - } - return Err(err).context("daemon lifecycle poll failed"); - } for pollfd in &pollfds { - let terminal = pollfd.revents & (libc::POLLERR | libc::POLLHUP | libc::POLLNVAL); - if terminal != 0 { + let revents = pollfd.revents(); + if revents.intersects(PollFlags::ERR | PollFlags::HUP | PollFlags::NVAL) { return Err(anyhow::anyhow!( "daemon lifecycle descriptor returned terminal readiness {:#x}", - pollfd.revents + revents.bits() )); } } - let daemon_ready = pollfds[0].revents & libc::POLLIN != 0; - let command_ready = command_index - .and_then(|index| pollfds.get(index)) - .is_some_and(|pollfd| pollfd.revents & libc::POLLIN != 0); - let deadline_ready = deadline_index - .and_then(|index| pollfds.get(index)) - .is_some_and(|pollfd| pollfd.revents & libc::POLLIN != 0); - let child_ready = child_index - .and_then(|index| pollfds.get(index)) - .is_some_and(|pollfd| pollfd.revents & libc::POLLIN != 0); + let is_readable = |index: Option| { + index + .and_then(|index| pollfds.get(index)) + .is_some_and(|pollfd| pollfd.revents().contains(PollFlags::IN)) + }; + let daemon_ready = is_readable(Some(0)); + let command_ready = is_readable(command_index); + let deadline_ready = is_readable(deadline_index); + let child_ready = is_readable(child_index); if !daemon_ready && !command_ready && !deadline_ready && !child_ready { return Err(anyhow::anyhow!( "daemon wake descriptor returned invalid readiness {:#x}", - pollfds[0].revents + pollfds[0].revents().bits() )); } if daemon_ready { diff --git a/src/daemon/overlay/lifecycle.rs b/src/daemon/overlay/lifecycle.rs index faee91c4..7f673e72 100644 --- a/src/daemon/overlay/lifecycle.rs +++ b/src/daemon/overlay/lifecycle.rs @@ -1,4 +1,4 @@ -use std::os::fd::{AsRawFd, BorrowedFd}; +use std::os::fd::{AsFd, BorrowedFd}; use std::path::{Path, PathBuf}; use std::sync::Arc; use std::sync::atomic::{AtomicBool, AtomicU8, Ordering}; @@ -108,7 +108,7 @@ impl OverlayLifecycle { &candidate.program, &launch.arguments, launch.environment, - daemon_watchdog.as_raw_fd(), + daemon_watchdog.as_fd(), )?; self.child.start(child)?; self.child diff --git a/src/daemon/protocol_v2/action.rs b/src/daemon/protocol_v2/action.rs index ee637e91..bb378674 100644 --- a/src/daemon/protocol_v2/action.rs +++ b/src/daemon/protocol_v2/action.rs @@ -2,7 +2,6 @@ use std::collections::BTreeMap; use std::fs::{self, File, OpenOptions}; use std::io::{self, ErrorKind}; -use std::os::fd::AsRawFd; use std::os::unix::fs::{MetadataExt, OpenOptionsExt}; use std::path::{Path, PathBuf}; #[cfg(test)] @@ -11,6 +10,7 @@ use std::sync::LazyLock; use std::sync::Mutex; use anyhow::{Context, Result, anyhow, bail}; +use rustix::fs::{self as rustix_fs, FlockOperation}; use serde::de::DeserializeOwned; use serde::{Deserialize, Serialize}; @@ -224,10 +224,13 @@ fn try_open_journal_lock(root: &Path, nonblocking: bool) -> Result> if !file.metadata()?.is_file() { bail!("action journal lock is not a regular file"); } - let operation = libc::LOCK_EX | if nonblocking { libc::LOCK_NB } else { 0 }; - // SAFETY: file owns the descriptor; flock retains no pointer. - if unsafe { libc::flock(file.as_raw_fd(), operation) } != 0 { - let error = io::Error::last_os_error(); + let operation = if nonblocking { + FlockOperation::NonBlockingLockExclusive + } else { + FlockOperation::LockExclusive + }; + if let Err(errno) = rustix_fs::flock(&file, operation) { + let error = io::Error::from(errno); if nonblocking && error.kind() == ErrorKind::WouldBlock { return Ok(None); } @@ -237,12 +240,9 @@ fn try_open_journal_lock(root: &Path, nonblocking: bool) -> Result> } fn unlock(file: &File) -> Result<()> { - // SAFETY: file owns the descriptor; flock retains no pointer. - if unsafe { libc::flock(file.as_raw_fd(), libc::LOCK_UN) } == 0 { - Ok(()) - } else { - Err(io::Error::last_os_error()).context("failed to unlock action journal") - } + rustix_fs::flock(file, FlockOperation::Unlock) + .map_err(io::Error::from) + .context("failed to unlock action journal") } fn write_record(path: &Path, value: &T) -> Result<()> { diff --git a/src/daemon/protocol_v2/action/tests.rs b/src/daemon/protocol_v2/action/tests.rs index 8ba46acb..181ebeb3 100644 --- a/src/daemon/protocol_v2/action/tests.rs +++ b/src/daemon/protocol_v2/action/tests.rs @@ -1,5 +1,7 @@ use std::{env, thread}; +use rustix::fs::{self as rustix_fs, FlockOperation}; + use crate::{test_env, test_temp}; use super::*; @@ -179,21 +181,11 @@ fn event_loop_claim_and_finish_defer_instead_of_waiting_for_locks() { .write(true) .open(decision_path) .unwrap(); - assert_eq!( - // SAFETY: `held_decision` keeps the descriptor open for the call, - // and flock retains no pointer. - unsafe { libc::flock(held_decision.as_raw_fd(), libc::LOCK_EX) }, - 0 - ); + rustix_fs::flock(&held_decision, FlockOperation::LockExclusive).unwrap(); let ActionFinishOutcome::Deferred(action) = action.try_finish(true, None).unwrap() else { panic!("contended command finish should defer"); }; - assert_eq!( - // SAFETY: `held_decision` keeps the descriptor open for the call, - // and flock retains no pointer. - unsafe { libc::flock(held_decision.as_raw_fd(), libc::LOCK_UN) }, - 0 - ); + rustix_fs::flock(&held_decision, FlockOperation::Unlock).unwrap(); assert!(matches!( action.try_finish(true, None).unwrap(), ActionFinishOutcome::Complete diff --git a/src/daemon/protocol_v2/child.rs b/src/daemon/protocol_v2/child.rs index 092ae279..42b20265 100644 --- a/src/daemon/protocol_v2/child.rs +++ b/src/daemon/protocol_v2/child.rs @@ -1,5 +1,5 @@ use std::io::ErrorKind; -use std::os::fd::{AsFd, AsRawFd, BorrowedFd, FromRawFd, OwnedFd}; +use std::os::fd::{AsFd, BorrowedFd, FromRawFd, OwnedFd}; use std::os::unix::fs::PermissionsExt; use std::path::{Path, PathBuf}; use std::thread::Builder; @@ -7,6 +7,8 @@ use std::time::Duration; use std::{env, fs, io, process, thread}; use anyhow::{Context, Result, anyhow, bail}; +use rustix::event::{self, PollFd, PollFlags}; +use rustix::io::{self as rustix_io, Errno, FdFlags}; use serde::{Deserialize, Serialize}; use crate::durable_io::{AtomicWriteOptions, OverwriteMode, PermissionPolicy, SymlinkPolicy}; @@ -535,11 +537,9 @@ pub(crate) fn start_daemon_watchdog_from_environment() -> Result<()> { // inherited descriptor. The environment marker is removed immediately so // no later exec can reinterpret the descriptor number. let descriptor = unsafe { OwnedFd::from_raw_fd(raw) }; - // SAFETY: descriptor is owned and F_SETFD changes only exec inheritance. - if unsafe { libc::fcntl(descriptor.as_raw_fd(), libc::F_SETFD, libc::FD_CLOEXEC) } != 0 { - return Err(io::Error::last_os_error()) - .context("failed to protect daemon watchdog descriptor"); - } + rustix_io::fcntl_setfd(&descriptor, FdFlags::CLOEXEC) + .map_err(io::Error::from) + .context("failed to protect daemon watchdog descriptor")?; super::linux::validate_pidfd(descriptor.as_fd()) .context("inherited daemon watchdog is not a live pidfd")?; // SAFETY: application startup has not launched runtime worker threads yet; @@ -550,27 +550,12 @@ pub(crate) fn start_daemon_watchdog_from_environment() -> Result<()> { Builder::new() .name("wayscriber-daemon-watchdog".into()) .spawn(move || { - let mut pollfd = libc::pollfd { - fd: descriptor.as_raw_fd(), - events: libc::POLLIN, - revents: 0, - }; - loop { - // SAFETY: pollfd points to one initialized entry and the - // descriptor remains owned by this thread. - let result = unsafe { libc::poll(&raw mut pollfd, 1, -1) }; - if result > 0 { - // SAFETY: loss of the owning daemon is an irrevocable - // fail-stop condition for its internal overlay child. - unsafe { super::linux::fail_stop(70) } - } - if result < 0 && io::Error::last_os_error().kind() == ErrorKind::Interrupted { - continue; - } - // SAFETY: an unusable parent-death channel cannot safely - // preserve overlay ownership. - unsafe { super::linux::fail_stop(70) } - } + let mut pollfds = [PollFd::new(&descriptor, PollFlags::IN)]; + while event::poll(&mut pollfds, None) == Err(Errno::INTR) {} + // SAFETY: readiness means the owning daemon is gone, and any other + // result is an unusable parent-death channel. Neither can safely + // preserve overlay ownership, so this is an irrevocable fail-stop. + unsafe { super::linux::fail_stop(70) } }) .context("failed to start daemon watchdog")?; Ok(()) diff --git a/src/daemon/protocol_v2/command/action_link.rs b/src/daemon/protocol_v2/command/action_link.rs index f91e706f..e474792e 100644 --- a/src/daemon/protocol_v2/command/action_link.rs +++ b/src/daemon/protocol_v2/command/action_link.rs @@ -2,6 +2,7 @@ use std::io::ErrorKind; use std::time::Duration; use anyhow::{Context, Result, bail}; +use rustix::fs::FlockOperation; use super::super::BootClock; use super::super::action::PreparedAction; @@ -37,14 +38,14 @@ fn claim_command_action_inner( let path = control_path(&command_root(), command_identity); let decision = open_lock(&path.join("decision.lock"), false)?; if nonblocking { - match flock(&decision, libc::LOCK_EX | libc::LOCK_NB) { + match flock(&decision, FlockOperation::NonBlockingLockExclusive) { Ok(()) => {} Err(error) if error.kind() == ErrorKind::WouldBlock => return Ok(None), Err(error) => return Err(error).context("failed to try command-action decision lock"), } } else { let deadline = BootClock::now()?.checked_add(Duration::from_millis(200))?; - lock_until(&decision, libc::LOCK_EX, deadline)?; + lock_until(&decision, FlockOperation::LockExclusive, deadline)?; } let mut control = read_control(&path)?; let exact = |status: &ActionStatus| match status { @@ -193,14 +194,14 @@ fn finish_command_action_inner( let path = control_path(&command_root(), command_identity); let decision = open_lock(&path.join("decision.lock"), false)?; if nonblocking { - match flock(&decision, libc::LOCK_EX | libc::LOCK_NB) { + match flock(&decision, FlockOperation::NonBlockingLockExclusive) { Ok(()) => {} Err(error) if error.kind() == ErrorKind::WouldBlock => return Ok(false), Err(error) => return Err(error).context("failed to try command-action decision lock"), } } else { let deadline = BootClock::now()?.checked_add(Duration::from_millis(200))?; - lock_until(&decision, libc::LOCK_EX, deadline)?; + lock_until(&decision, FlockOperation::LockExclusive, deadline)?; } let mut control = read_control(&path)?; let applied = result == CommandActionResult::Applied; diff --git a/src/daemon/protocol_v2/command/claimed.rs b/src/daemon/protocol_v2/command/claimed.rs index 1c386649..3fb2bfd5 100644 --- a/src/daemon/protocol_v2/command/claimed.rs +++ b/src/daemon/protocol_v2/command/claimed.rs @@ -1,6 +1,7 @@ use std::time::Duration; use anyhow::{Result, anyhow, bail}; +use rustix::fs::FlockOperation; #[cfg(test)] use super::super::wire::CommandControl; @@ -61,7 +62,8 @@ impl ClaimedCommand { journal.prepare_command(&expected.identity, &expected.target_daemon_token, action)?; let deadline = BootDeadline::from_nanos(expected.submission_clock.authorization_deadline_boottime_ns); - if let Err(error) = lock_until(&self.decision_lock, libc::LOCK_EX, deadline) { + if let Err(error) = lock_until(&self.decision_lock, FlockOperation::LockExclusive, deadline) + { let _ = journal.abandon(&prepared, "command decision lock was not reacquired"); return Err(error); } @@ -71,7 +73,11 @@ impl ClaimedCommand { let reason = "command was canceled during action preparation"; journal.abandon(&prepared, reason)?; let recovery_deadline = BootClock::now()?.checked_add(Duration::from_millis(200))?; - lock_until(&self.decision_lock, libc::LOCK_EX, recovery_deadline)?; + lock_until( + &self.decision_lock, + FlockOperation::LockExclusive, + recovery_deadline, + )?; let mut latest = read_control(&self.control_path)?; if latest != current || !matches!( @@ -221,6 +227,6 @@ impl ClaimedCommand { impl Drop for ClaimedCommand { fn drop(&mut self) { - let _ = flock(&self.decision_lock, libc::LOCK_UN); + let _ = flock(&self.decision_lock, FlockOperation::Unlock); } } diff --git a/src/daemon/protocol_v2/command/client.rs b/src/daemon/protocol_v2/command/client.rs index 08ed3847..21c03c2d 100644 --- a/src/daemon/protocol_v2/command/client.rs +++ b/src/daemon/protocol_v2/command/client.rs @@ -3,6 +3,7 @@ use std::time::Duration; use std::{fs, process, thread}; use anyhow::{Context, Result, bail}; +use rustix::fs::FlockOperation; #[cfg(test)] use anyhow::anyhow; @@ -62,8 +63,16 @@ impl ClientCommand { create_private_directory(&stage)?; let decision_lock = open_lock(&stage.join("decision.lock"), true)?; let caller_lease = open_lock(&stage.join("caller.lease"), true)?; - lock_until(&caller_lease, libc::LOCK_SH, authorization_deadline)?; - lock_until(&decision_lock, libc::LOCK_EX, authorization_deadline)?; + lock_until( + &caller_lease, + FlockOperation::LockShared, + authorization_deadline, + )?; + lock_until( + &decision_lock, + FlockOperation::LockExclusive, + authorization_deadline, + )?; let mut control = CommandControl { protocol_version: DAEMON_COMMAND_PROTOCOL_VERSION, @@ -154,7 +163,7 @@ impl ClientCommand { let slice = now .checked_add(Duration::from_millis(20))? .min(self.response_deadline); - if !try_lock_until(&self.decision_lock, libc::LOCK_EX, slice)? { + if !try_lock_until(&self.decision_lock, FlockOperation::LockExclusive, slice)? { if slice < self.response_deadline { continue; } @@ -201,7 +210,11 @@ impl ClientCommand { let _ = unlock(&self.caller_lease); return Ok(TerminalCommandResult::AdmittedIndeterminate(reason)); } - lock_until(&self.decision_lock, libc::LOCK_EX, self.response_deadline)?; + lock_until( + &self.decision_lock, + FlockOperation::LockExclusive, + self.response_deadline, + )?; let mut control = read_control(&self.control_path)?; if cancel_open(&mut control)? { write_control(&self.control_path, &control)?; diff --git a/src/daemon/protocol_v2/command/layout.rs b/src/daemon/protocol_v2/command/layout.rs index 00e956c2..67efc6f9 100644 --- a/src/daemon/protocol_v2/command/layout.rs +++ b/src/daemon/protocol_v2/command/layout.rs @@ -1,13 +1,13 @@ use std::collections::BTreeSet; use std::fs::{self, File, OpenOptions}; use std::io::{self, ErrorKind}; -use std::os::fd::AsRawFd; use std::os::unix::fs::{MetadataExt, OpenOptionsExt}; use std::path::{Path, PathBuf}; use std::thread; use std::time::Duration; use anyhow::{Context, Result, anyhow, bail}; +use rustix::fs::{self as rustix_fs, FlockOperation}; use serde::de::DeserializeOwned; use crate::durable_io::AtomicWriteOptions; @@ -279,22 +279,25 @@ pub(super) fn open_lock(path: &Path, create_new: bool) -> Result { Ok(file) } -pub(super) fn flock(file: &File, operation: libc::c_int) -> io::Result<()> { - // SAFETY: file owns a valid descriptor and flock does not retain it. - if unsafe { libc::flock(file.as_raw_fd(), operation) } == 0 { - Ok(()) - } else { - Err(io::Error::last_os_error()) +pub(super) fn flock(file: &File, operation: FlockOperation) -> io::Result<()> { + rustix_fs::flock(file, operation).map_err(io::Error::from) +} + +fn nonblocking(operation: FlockOperation) -> FlockOperation { + match operation { + FlockOperation::LockShared => FlockOperation::NonBlockingLockShared, + FlockOperation::LockExclusive => FlockOperation::NonBlockingLockExclusive, + other => other, } } pub(super) fn unlock(file: &File) -> Result<()> { - flock(file, libc::LOCK_UN).context("failed to unlock protocol descriptor") + flock(file, FlockOperation::Unlock).context("failed to unlock protocol descriptor") } pub(super) fn lock_until( file: &File, - operation: libc::c_int, + operation: FlockOperation, deadline: BootDeadline, ) -> Result<()> { if try_lock_until(file, operation, deadline)? { @@ -306,11 +309,11 @@ pub(super) fn lock_until( pub(super) fn try_lock_until( file: &File, - operation: libc::c_int, + operation: FlockOperation, deadline: BootDeadline, ) -> Result { loop { - match flock(file, operation | libc::LOCK_NB) { + match flock(file, nonblocking(operation)) { Ok(()) => return Ok(true), Err(error) if error.kind() == ErrorKind::WouldBlock => { if BootClock::now()? >= deadline { @@ -341,7 +344,7 @@ pub(super) fn try_admission_lock(root: &Path, deadline: BootDeadline) -> Result< } Err(error) => return Err(error), }; - if try_lock_until(&file, libc::LOCK_EX, deadline)? { + if try_lock_until(&file, FlockOperation::LockExclusive, deadline)? { Ok(Some(file)) } else { Ok(None) diff --git a/src/daemon/protocol_v2/command/owner.rs b/src/daemon/protocol_v2/command/owner.rs index 62b0771a..07e7d001 100644 --- a/src/daemon/protocol_v2/command/owner.rs +++ b/src/daemon/protocol_v2/command/owner.rs @@ -5,6 +5,7 @@ use std::path::{Path, PathBuf}; use std::time::Duration; use anyhow::{Context, Result, bail}; +use rustix::fs::FlockOperation; use super::super::wire::{ ActionStatus, CallerDisposition, ChildReportStatus, CommandDecision, CommandResponse, @@ -191,7 +192,11 @@ impl CommandOwner { Err(error) => return Err(error), }; let retry_deadline = BootClock::now()?.checked_add(Duration::from_millis(200))?; - if !try_lock_until(&decision_lock, libc::LOCK_EX, retry_deadline)? { + if !try_lock_until( + &decision_lock, + FlockOperation::LockExclusive, + retry_deadline, + )? { return Ok(None); } let Ok(mut control) = read_control(&path) else { @@ -316,7 +321,7 @@ impl CommandOwner { Err(error) => return Err(error), }; let deadline = BootClock::now()?.checked_add(Duration::from_millis(200))?; - lock_until(&decision, libc::LOCK_EX, deadline)?; + lock_until(&decision, FlockOperation::LockExclusive, deadline)?; let mut control = read_control(&path)?; if control.identity == reference.command_identity && control.queue_order == reference.queue_order @@ -341,13 +346,13 @@ impl CommandOwner { observed: &super::super::wire::CommandControl, ) -> Result<()> { let caller_lease = open_lock(&path.join("caller.lease"), false)?; - match flock(&caller_lease, libc::LOCK_EX | libc::LOCK_NB) { + match flock(&caller_lease, FlockOperation::NonBlockingLockExclusive) { Ok(()) => {} Err(error) if error.kind() == ErrorKind::WouldBlock => return Ok(()), Err(error) => return Err(error).context("failed to inspect unqueued caller lease"), } let decision = open_lock(&path.join("decision.lock"), false)?; - match flock(&decision, libc::LOCK_EX | libc::LOCK_NB) { + match flock(&decision, FlockOperation::NonBlockingLockExclusive) { Ok(()) => {} Err(error) if error.kind() == ErrorKind::WouldBlock => { unlock(&caller_lease)?; @@ -390,7 +395,7 @@ impl CommandOwner { } let decision_lock = open_lock(&path.join("decision.lock"), false)?; let deadline = BootClock::now()?.checked_add(Duration::from_millis(200))?; - if !try_lock_until(&decision_lock, libc::LOCK_EX, deadline)? { + if !try_lock_until(&decision_lock, FlockOperation::LockExclusive, deadline)? { return Ok(None); } let mut control = read_control(&path)?; @@ -440,11 +445,11 @@ impl CommandOwner { for entry in read_dir_bounded(&controls_dir(&self.root), MAX_DIRECTORY_ENTRIES_PER_SCAN)? { let path = entry.path(); let lease = open_lock(&path.join("caller.lease"), false)?; - if flock(&lease, libc::LOCK_EX | libc::LOCK_NB).is_err() { + if flock(&lease, FlockOperation::NonBlockingLockExclusive).is_err() { continue; } let decision = open_lock(&path.join("decision.lock"), false)?; - if flock(&decision, libc::LOCK_EX | libc::LOCK_NB).is_err() { + if flock(&decision, FlockOperation::NonBlockingLockExclusive).is_err() { unlock(&lease)?; continue; } diff --git a/src/daemon/protocol_v2/command/recovery.rs b/src/daemon/protocol_v2/command/recovery.rs index d5639540..4a6f8524 100644 --- a/src/daemon/protocol_v2/command/recovery.rs +++ b/src/daemon/protocol_v2/command/recovery.rs @@ -3,6 +3,7 @@ use std::path::Path; use std::time::Duration; use anyhow::{Result, anyhow, bail}; +use rustix::fs::FlockOperation; use super::super::BootClock; use super::super::wire::{ @@ -32,7 +33,7 @@ pub(super) fn recover_previous_generation(root: &Path, daemon_token: &str) -> Re continue; }; let deadline = BootClock::now()?.checked_add(Duration::from_millis(200))?; - lock_until(&decision_lock, libc::LOCK_EX, deadline)?; + lock_until(&decision_lock, FlockOperation::LockExclusive, deadline)?; let Ok(mut control) = read_control(&path) else { unlock(&decision_lock)?; quarantine_entry(root, &path, QuarantineKind::Control)?; diff --git a/src/daemon/protocol_v2/command/staging.rs b/src/daemon/protocol_v2/command/staging.rs index 5b6cba77..48449536 100644 --- a/src/daemon/protocol_v2/command/staging.rs +++ b/src/daemon/protocol_v2/command/staging.rs @@ -3,6 +3,7 @@ use std::path::Path; use std::time::Duration; use anyhow::Result; +use rustix::fs::FlockOperation; use super::super::{BootClock, BootIdentity}; use super::MAX_COMMAND_STAGING_DIRECTORIES; @@ -24,7 +25,7 @@ pub(super) fn recover_staging(root: &Path) -> Result<()> { let _ = fs::remove_dir_all(path); continue; }; - if flock(&lease, libc::LOCK_EX | libc::LOCK_NB).is_ok() { + if flock(&lease, FlockOperation::NonBlockingLockExclusive).is_ok() { fs::remove_dir_all(path)?; } } diff --git a/src/daemon/protocol_v2/command/tests.rs b/src/daemon/protocol_v2/command/tests.rs index fdf5d264..4418c3bb 100644 --- a/src/daemon/protocol_v2/command/tests.rs +++ b/src/daemon/protocol_v2/command/tests.rs @@ -1,6 +1,8 @@ use std::env; use std::time::Duration; +use rustix::fs::FlockOperation; + use crate::{test_env, test_temp}; use super::super::BootClock; @@ -208,7 +210,7 @@ fn contended_published_command_claim_is_deferred_and_retried() { &token, ) .unwrap(); - flock(&client.decision_lock, libc::LOCK_EX).unwrap(); + flock(&client.decision_lock, FlockOperation::LockExclusive).unwrap(); assert!(owner.claim_next().unwrap().is_none()); let now = BootClock::now().unwrap(); @@ -252,7 +254,7 @@ fn contended_ref_less_recovery_is_deferred_and_retried() { &control.identity, )) .unwrap(); - flock(&client.decision_lock, libc::LOCK_EX).unwrap(); + flock(&client.decision_lock, FlockOperation::LockExclusive).unwrap(); assert!(owner.claim_next().unwrap().is_none()); unlock(&client.decision_lock).unwrap(); @@ -287,7 +289,7 @@ fn terminal_collection_defers_while_admission_is_busy() { drop(client); let admission = open_lock(&owner.root.join("admission.lock"), false).unwrap(); - flock(&admission, libc::LOCK_EX).unwrap(); + flock(&admission, FlockOperation::LockExclusive).unwrap(); assert_eq!(owner.collect_terminal().unwrap(), 0); let now = BootClock::now().unwrap(); let retry = owner @@ -715,20 +717,20 @@ fn lock_deadline_is_reported_without_parsing_error_text() { let path = root.join("structured-timeout.lock"); let held = open_lock(&path, true).unwrap(); let contender = open_lock(&path, false).unwrap(); - flock(&held, libc::LOCK_EX).unwrap(); + flock(&held, FlockOperation::LockExclusive).unwrap(); let deadline = BootClock::now() .unwrap() .checked_add(Duration::from_millis(5)) .unwrap(); - assert!(!try_lock_until(&contender, libc::LOCK_EX, deadline).unwrap()); + assert!(!try_lock_until(&contender, FlockOperation::LockExclusive, deadline).unwrap()); unlock(&held).unwrap(); let deadline = BootClock::now() .unwrap() .checked_add(Duration::from_millis(5)) .unwrap(); - assert!(try_lock_until(&contender, libc::LOCK_EX, deadline).unwrap()); + assert!(try_lock_until(&contender, FlockOperation::LockExclusive, deadline).unwrap()); unlock(&contender).unwrap(); }); } diff --git a/src/daemon/protocol_v2/linux.rs b/src/daemon/protocol_v2/linux.rs index 58790f1d..d4cc9faf 100644 --- a/src/daemon/protocol_v2/linux.rs +++ b/src/daemon/protocol_v2/linux.rs @@ -1,12 +1,21 @@ use std::ffi::CString; use std::fs::{File, Metadata, OpenOptions, Permissions}; use std::io::{self, Read}; -use std::os::fd::{AsFd, AsRawFd, BorrowedFd, FromRawFd, OwnedFd}; +use std::os::fd::{AsFd, AsRawFd, BorrowedFd, OwnedFd}; use std::os::unix::fs::{MetadataExt, OpenOptionsExt}; use std::path::{Path, PathBuf}; use std::time::{Duration, Instant}; use std::{fmt, fs, process, ptr}; +use rustix::event::{self, PollFd, PollFlags, Timespec}; +use rustix::fs::inotify::{self, CreateFlags, WatchFlags}; +use rustix::io::{self as rustix_io, Errno}; +use rustix::process::{self as rustix_process, Pid, PidfdFlags}; +use rustix::time::{ + self as rustix_time, DynamicClockId, Itimerspec, TimerfdClockId, TimerfdFlags, + TimerfdTimerFlags, +}; + const BOOT_ID_PATH: &str = "/proc/sys/kernel/random/boot_id"; const TIME_NAMESPACE_PATH: &str = "/proc/self/ns/time"; const PID_NAMESPACE_PATH: &str = "/proc/self/ns/pid"; @@ -63,33 +72,6 @@ pub(crate) struct CommandQueueWatcher { buffered_offset: usize, } -fn fill_random(bytes: &mut [u8]) -> io::Result<()> { - let mut filled = 0; - while filled < bytes.len() { - // SAFETY: the remaining slice is writable for its full length. getrandom - // writes at most that length and does not retain the pointer. - let result = unsafe { - libc::getrandom(bytes[filled..].as_mut_ptr().cast(), bytes.len() - filled, 0) - }; - if result > 0 { - filled += result as usize; - continue; - } - if result == 0 { - return Err(io::Error::new( - io::ErrorKind::UnexpectedEof, - "getrandom returned zero bytes", - )); - } - let error = io::Error::last_os_error(); - if error.kind() == io::ErrorKind::Interrupted { - continue; - } - return Err(error); - } - Ok(()) -} - fn lowercase_hex(bytes: &[u8], output: &mut fmt::Formatter<'_>) -> fmt::Result { for byte in bytes { write!(output, "{byte:02x}")?; @@ -100,7 +82,7 @@ fn lowercase_hex(bytes: &[u8], output: &mut fmt::Formatter<'_>) -> fmt::Result { impl ProtocolId { pub(crate) fn generate() -> io::Result { let mut bytes = [0; 16]; - fill_random(&mut bytes)?; + getrandom::fill(&mut bytes)?; Ok(Self(bytes)) } } @@ -120,7 +102,7 @@ impl fmt::Display for ProtocolId { impl ProtocolToken { pub(crate) fn generate() -> io::Result { let mut bytes = [0; 32]; - fill_random(&mut bytes)?; + getrandom::fill(&mut bytes)?; Ok(Self(bytes)) } } @@ -198,8 +180,7 @@ pub(crate) fn read_bounded_private_file(path: &Path, cap: usize) -> io::Result bool { - // SAFETY: geteuid has no preconditions and cannot fail. - metadata.uid() == unsafe { libc::geteuid() } && metadata.mode() & 0o077 == 0 + metadata.uid() == rustix_process::geteuid().as_raw() && metadata.mode() & 0o077 == 0 } /// Creates `path` as a directory only this user may use, or makes an existing @@ -215,8 +196,7 @@ pub(crate) fn create_private_directory(path: &Path) -> anyhow::Result<()> { } let metadata = fs::symlink_metadata(path)?; - // SAFETY: geteuid has no preconditions and cannot fail. - if !metadata.is_dir() || metadata.uid() != unsafe { libc::geteuid() } { + if !metadata.is_dir() || metadata.uid() != rustix_process::geteuid().as_raw() { anyhow::bail!("{} is not a no-follow private directory", path.display()); } fs::set_permissions(path, Permissions::from_mode(0o700))?; @@ -389,15 +369,10 @@ pub(crate) fn process_start_ticks(pid: u32) -> io::Result { pub(crate) fn open_pidfd(pid: u32) -> io::Result { let pid = i32::try_from(pid) - .map_err(|_| io::Error::new(io::ErrorKind::InvalidInput, "pid does not fit pid_t"))?; - // SAFETY: pidfd_open has no pointer arguments. The returned descriptor is - // owned by the caller on success. - let raw = unsafe { libc::syscall(libc::SYS_pidfd_open, pid, 0) }; - if raw < 0 { - return Err(io::Error::last_os_error()); - } - // SAFETY: raw is a new pidfd not wrapped elsewhere. - Ok(unsafe { OwnedFd::from_raw_fd(raw as i32) }) + .ok() + .and_then(Pid::from_raw) + .ok_or_else(|| io::Error::new(io::ErrorKind::InvalidInput, "pid does not fit pid_t"))?; + Ok(rustix_process::pidfd_open(pid, PidfdFlags::empty())?) } /// Blocks until the process behind `fd` exits or `timeout` elapses, returning @@ -413,27 +388,14 @@ pub(crate) fn wait_for_pidfd_exit(fd: BorrowedFd<'_>, timeout: Duration) -> io:: if remaining.is_zero() { return Ok(false); } - let mut poll_fd = libc::pollfd { - fd: fd.as_raw_fd(), - events: libc::POLLIN, - revents: 0, - }; - let millis = i32::try_from(remaining.as_millis()).unwrap_or(i32::MAX); - // SAFETY: one owned pollfd, valid for the call; the descriptor is - // borrowed for at least this long. - let ready = unsafe { libc::poll(&raw mut poll_fd, 1, millis) }; - if ready < 0 { - let error = io::Error::last_os_error(); + let mut pollfds = [PollFd::from_borrowed_fd(fd, PollFlags::IN)]; + let remaining = Timespec::try_from(remaining).map_err(io::Error::other)?; + match event::poll(&mut pollfds, Some(&remaining)) { + Ok(ready) => return Ok(ready != 0), // A signal arriving mid-wait is expected on this thread. - if error.kind() == io::ErrorKind::Interrupted { - continue; - } - return Err(error); + Err(Errno::INTR) => {} + Err(errno) => return Err(errno.into()), } - if ready == 0 { - return Ok(false); - } - return Ok(true); } } @@ -459,15 +421,7 @@ pub(crate) fn validate_pidfd(fd: BorrowedFd<'_>) -> io::Result<()> { impl BootClock { pub(crate) fn now() -> io::Result { - let mut value = libc::timespec { - tv_sec: 0, - tv_nsec: 0, - }; - // SAFETY: value is a valid writable timespec and CLOCK_BOOTTIME has no - // additional preconditions. - if unsafe { libc::clock_gettime(libc::CLOCK_BOOTTIME, &raw mut value) } != 0 { - return Err(io::Error::last_os_error()); - } + let value = rustix_time::clock_gettime_dynamic(DynamicClockId::Boottime)?; if value.tv_sec < 0 || !(0..1_000_000_000).contains(&value.tv_nsec) { return Err(io::Error::new( io::ErrorKind::InvalidData, @@ -508,17 +462,11 @@ impl BootDeadline { impl BootDeadlineSource { pub(crate) fn try_new() -> io::Result { - // SAFETY: timerfd_create returns a new owned descriptor on success. - let raw = unsafe { - libc::timerfd_create(libc::CLOCK_BOOTTIME, libc::TFD_NONBLOCK | libc::TFD_CLOEXEC) - }; - if raw < 0 { - return Err(io::Error::last_os_error()); - } - Ok(Self { - // SAFETY: raw is a newly created descriptor not wrapped elsewhere. - fd: unsafe { OwnedFd::from_raw_fd(raw) }, - }) + let fd = rustix_time::timerfd_create( + TimerfdClockId::Boottime, + TimerfdFlags::NONBLOCK | TimerfdFlags::CLOEXEC, + )?; + Ok(Self { fd }) } pub(crate) fn poll_fd(&self) -> BorrowedFd<'_> { @@ -528,82 +476,45 @@ impl BootDeadlineSource { pub(crate) fn arm(&self, deadline: BootDeadline) -> io::Result<()> { let seconds = deadline.0 / 1_000_000_000; let nanos = deadline.0 % 1_000_000_000; - let value = libc::itimerspec { - it_interval: libc::timespec { - tv_sec: 0, - tv_nsec: 0, - }, - it_value: libc::timespec { - tv_sec: libc::time_t::try_from(seconds).map_err(|_| { + let value = Itimerspec { + it_interval: Timespec::default(), + it_value: Timespec { + tv_sec: i64::try_from(seconds).map_err(|_| { io::Error::new(io::ErrorKind::InvalidInput, "deadline seconds overflow") })?, - tv_nsec: libc::c_long::try_from(nanos).map_err(|_| { + tv_nsec: i64::try_from(nanos).map_err(|_| { io::Error::new(io::ErrorKind::InvalidInput, "deadline nanos overflow") })?, }, }; - // SAFETY: value is a fully initialized itimerspec and fd owns a timerfd. - if unsafe { - libc::timerfd_settime( - self.fd.as_raw_fd(), - libc::TFD_TIMER_ABSTIME, - &raw const value, - ptr::null_mut(), - ) - } != 0 - { - return Err(io::Error::last_os_error()); - } + rustix_time::timerfd_settime(&self.fd, TimerfdTimerFlags::ABSTIME, &value)?; Ok(()) } pub(crate) fn disarm(&self) -> io::Result<()> { - let value = libc::itimerspec { - it_interval: libc::timespec { - tv_sec: 0, - tv_nsec: 0, - }, - it_value: libc::timespec { - tv_sec: 0, - tv_nsec: 0, - }, + let value = Itimerspec { + it_interval: Timespec::default(), + it_value: Timespec::default(), }; - // SAFETY: value is initialized and fd owns a timerfd. - if unsafe { - libc::timerfd_settime(self.fd.as_raw_fd(), 0, &raw const value, ptr::null_mut()) - } != 0 - { - return Err(io::Error::last_os_error()); - } + rustix_time::timerfd_settime(&self.fd, TimerfdTimerFlags::empty(), &value)?; Ok(()) } pub(crate) fn drain(&self) -> io::Result { - let mut expirations = 0_u64; + let mut expirations = [0_u8; size_of::()]; loop { - // SAFETY: expirations is writable and fd remains valid for the read. - let result = unsafe { - libc::read( - self.fd.as_raw_fd(), - (&raw mut expirations).cast(), - size_of::(), - ) - }; - if result == size_of::() as isize { - return Ok(true); + match rustix_io::read(&self.fd, &mut expirations) { + Ok(read) if read == expirations.len() => return Ok(true), + Ok(read) => { + return Err(io::Error::new( + io::ErrorKind::UnexpectedEof, + format!("timerfd returned a short read ({read} bytes)"), + )); + } + Err(Errno::INTR) => {} + Err(Errno::WOULDBLOCK) => return Ok(false), + Err(errno) => return Err(errno.into()), } - if result < 0 { - let error = io::Error::last_os_error(); - return match error.kind() { - io::ErrorKind::Interrupted => continue, - io::ErrorKind::WouldBlock => Ok(false), - _ => Err(error), - }; - } - return Err(io::Error::new( - io::ErrorKind::UnexpectedEof, - format!("timerfd returned a short read ({result} bytes)"), - )); } } } @@ -611,27 +522,15 @@ impl BootDeadlineSource { impl CommandQueueWatcher { pub(crate) fn try_new(queue_path: &Path) -> io::Result { let (queue_fd, queue_identity) = open_nofollow_directory(queue_path)?; - // SAFETY: inotify_init1 returns a newly owned descriptor on success. - let raw = unsafe { libc::inotify_init1(libc::IN_NONBLOCK | libc::IN_CLOEXEC) }; - if raw < 0 { - return Err(io::Error::last_os_error()); - } - // SAFETY: raw is a new descriptor not wrapped elsewhere. - let fd = unsafe { OwnedFd::from_raw_fd(raw) }; + let fd = inotify::init(CreateFlags::NONBLOCK | CreateFlags::CLOEXEC)?; let path = CString::new(queue_path.as_os_str().as_encoded_bytes()) .map_err(|_| io::Error::new(io::ErrorKind::InvalidInput, "queue path contains NUL"))?; - let mask = libc::IN_MOVED_TO - | libc::IN_CREATE - | libc::IN_DELETE_SELF - | libc::IN_MOVE_SELF - | libc::IN_IGNORED - | libc::IN_Q_OVERFLOW; - // SAFETY: path is NUL-terminated and fd owns an inotify instance. - let watch_descriptor = - unsafe { libc::inotify_add_watch(fd.as_raw_fd(), path.as_ptr(), mask) }; - if watch_descriptor < 0 { - return Err(io::Error::last_os_error()); - } + // IN_IGNORED and IN_Q_OVERFLOW always arrive, so they are not requested here. + let mask = WatchFlags::MOVED_TO + | WatchFlags::CREATE + | WatchFlags::DELETE_SELF + | WatchFlags::MOVE_SELF; + let watch_descriptor = inotify::add_watch(&fd, &path, mask)?; revalidate_path_identity(queue_path, queue_identity)?; Ok(Self { fd, @@ -657,30 +556,20 @@ impl CommandQueueWatcher { self.buffered_length = 0; self.buffered_offset = 0; loop { - // SAFETY: buffer is writable for its full fixed length and fd is - // a nonblocking inotify descriptor. - let read = unsafe { - libc::read( - self.fd.as_raw_fd(), - self.buffer.as_mut_ptr().cast(), - self.buffer.len(), - ) - }; - if read > 0 { - self.buffered_length = read as usize; - return Ok(true); - } - if read == 0 { - return Err(io::Error::new( - io::ErrorKind::UnexpectedEof, - "inotify returned EOF", - )); - } - let error = io::Error::last_os_error(); - match error.kind() { - io::ErrorKind::Interrupted => continue, - io::ErrorKind::WouldBlock => return Ok(false), - _ => return Err(error), + match rustix_io::read(&self.fd, &mut self.buffer[..]) { + Ok(0) => { + return Err(io::Error::new( + io::ErrorKind::UnexpectedEof, + "inotify returned EOF", + )); + } + Ok(read) => { + self.buffered_length = read; + return Ok(true); + } + Err(Errno::INTR) => {} + Err(Errno::WOULDBLOCK) => return Ok(false), + Err(errno) => return Err(errno.into()), } } } diff --git a/src/daemon/protocol_v2/tests.rs b/src/daemon/protocol_v2/tests.rs index 4598d6f9..ae44eef4 100644 --- a/src/daemon/protocol_v2/tests.rs +++ b/src/daemon/protocol_v2/tests.rs @@ -1,13 +1,30 @@ -use std::os::fd::AsRawFd; +use std::os::fd::{AsFd, BorrowedFd}; use std::process::Command; use std::time::{Duration, Instant}; use std::{env, process}; +use rustix::event::{self, PollFd, PollFlags, Timespec}; + use crate::tray_action::TrayAction; use crate::{env_vars, test_env, test_temp}; use super::*; +/// Polls for input. Any other readiness fails the test, so a broken descriptor +/// cannot pass for silence. +fn is_readable(fd: BorrowedFd<'_>, timeout: Duration) -> bool { + let mut pollfds = [PollFd::from_borrowed_fd(fd, PollFlags::IN)]; + let timeout = Timespec::try_from(timeout).unwrap(); + let ready = event::poll(&mut pollfds, Some(&timeout)).unwrap(); + let revents = pollfds[0].revents(); + assert!( + ready == 0 || revents == PollFlags::IN, + "unexpected readiness {:#x}", + revents.bits() + ); + ready == 1 +} + #[test] fn production_mode_is_closed_to_published_v2_after_cutover() { assert_eq!( @@ -71,15 +88,7 @@ fn boottime_deadline_source_arms_absolutely_and_drains_once() { .unwrap(); source.arm(deadline).unwrap(); - let mut poll_fd = libc::pollfd { - fd: source.poll_fd().as_raw_fd(), - events: libc::POLLIN, - revents: 0, - }; - // SAFETY: poll_fd remains valid for this bounded poll. - let ready = unsafe { libc::poll(&raw mut poll_fd, 1, 500) }; - assert_eq!(ready, 1); - assert_ne!(poll_fd.revents & libc::POLLIN, 0); + assert!(is_readable(source.poll_fd(), Duration::from_millis(500))); assert!(source.drain().unwrap()); assert!(!source.drain().unwrap()); source.disarm().unwrap(); @@ -128,14 +137,7 @@ fn pidfd_wait_times_out_while_alive_and_returns_once_exited() { #[test] fn self_pidfd_is_not_readable_while_process_is_alive() { let pidfd = linux::open_self_pidfd().unwrap(); - let mut poll_fd = libc::pollfd { - fd: pidfd.as_raw_fd(), - events: libc::POLLIN, - revents: 0, - }; - // SAFETY: poll_fd remains valid for the zero-timeout observation. - let ready = unsafe { libc::poll(&raw mut poll_fd, 1, 0) }; - assert_eq!(ready, 0); + assert!(!is_readable(pidfd.as_fd(), Duration::ZERO)); } #[test] @@ -189,13 +191,7 @@ fn command_queue_watch_reports_atomic_publication_and_rejects_replacement() { ) .unwrap(); - let mut poll_fd = libc::pollfd { - fd: watcher.poll_fd().as_raw_fd(), - events: libc::POLLIN, - revents: 0, - }; - // SAFETY: watcher owns the descriptor throughout the bounded poll. - assert_eq!(unsafe { libc::poll(&raw mut poll_fd, 1, 500) }, 1); + assert!(is_readable(watcher.poll_fd(), Duration::from_millis(500))); assert!(watcher.drain().unwrap().scan_pending); fs::rename(&queue, temp.path().join("old-queue")).unwrap(); diff --git a/src/durable_io.rs b/src/durable_io.rs index c6c6a5c7..e611a20c 100644 --- a/src/durable_io.rs +++ b/src/durable_io.rs @@ -8,6 +8,9 @@ use std::path::{Path, PathBuf}; use std::sync::atomic::{AtomicU64, Ordering}; use std::time::{SystemTime, UNIX_EPOCH}; +#[cfg(target_os = "linux")] +use rustix::fs::{self as rustix_fs, RenameFlags}; + use std::ffi::{CString, OsStr, OsString}; #[cfg(unix)] use std::os::unix::fs::{OpenOptionsExt, PermissionsExt}; @@ -594,23 +597,14 @@ fn metadata_mode(_metadata: &fs::Metadata) -> Option { pub fn rename_no_replace(source: &Path, target: &Path) -> io::Result<()> { let source = path_to_cstring(source)?; let target = path_to_cstring(target)?; - // SAFETY: The C strings are valid, NUL-terminated paths. AT_FDCWD makes both - // paths relative to the process cwd, matching std::fs::rename path semantics. - let result = unsafe { - libc::syscall( - libc::SYS_renameat2, - libc::AT_FDCWD, - source.as_ptr(), - libc::AT_FDCWD, - target.as_ptr(), - libc::RENAME_NOREPLACE, - ) - }; - if result == 0 { - Ok(()) - } else { - Err(io::Error::last_os_error()) - } + rustix_fs::renameat_with( + rustix_fs::CWD, + &source, + rustix_fs::CWD, + &target, + RenameFlags::NOREPLACE, + ) + .map_err(io::Error::from) } #[cfg(target_os = "linux")] diff --git a/src/durable_io/lock.rs b/src/durable_io/lock.rs index f8dab3f7..a236e4fd 100644 --- a/src/durable_io/lock.rs +++ b/src/durable_io/lock.rs @@ -4,19 +4,11 @@ use std::fs::File; use std::io; #[cfg(unix)] -use std::os::unix::io::AsRawFd; +use rustix::fs::{self as rustix_fs, FlockOperation}; #[cfg(unix)] -fn flock(file: &File, op: libc::c_int) -> io::Result<()> { - let fd = file.as_raw_fd(); - // SAFETY: `fd` stays open for the borrowed `file`, and flock takes no - // pointers or ownership of the descriptor. - let result = unsafe { libc::flock(fd, op) }; - if result == 0 { - Ok(()) - } else { - Err(io::Error::last_os_error()) - } +fn flock(file: &File, operation: FlockOperation) -> io::Result<()> { + rustix_fs::flock(file, operation).map_err(io::Error::from) } #[cfg(not(unix))] @@ -30,7 +22,7 @@ fn flock(_file: &File, _op: i32) -> io::Result<()> { pub fn lock_shared(file: &File) -> io::Result<()> { #[cfg(unix)] { - flock(file, libc::LOCK_SH) + flock(file, FlockOperation::LockShared) } #[cfg(not(unix))] { @@ -41,7 +33,7 @@ pub fn lock_shared(file: &File) -> io::Result<()> { pub fn lock_exclusive(file: &File) -> io::Result<()> { #[cfg(unix)] { - flock(file, libc::LOCK_EX) + flock(file, FlockOperation::LockExclusive) } #[cfg(not(unix))] { @@ -52,7 +44,7 @@ pub fn lock_exclusive(file: &File) -> io::Result<()> { pub fn try_lock_exclusive(file: &File) -> io::Result<()> { #[cfg(unix)] { - flock(file, libc::LOCK_EX | libc::LOCK_NB) + flock(file, FlockOperation::NonBlockingLockExclusive) } #[cfg(not(unix))] { @@ -63,7 +55,7 @@ pub fn try_lock_exclusive(file: &File) -> io::Result<()> { pub fn unlock(file: &File) -> io::Result<()> { #[cfg(unix)] { - flock(file, libc::LOCK_UN) + flock(file, FlockOperation::Unlock) } #[cfg(not(unix))] { diff --git a/src/i18n/tests.rs b/src/i18n/tests.rs index 3b11ce0a..40f80b98 100644 --- a/src/i18n/tests.rs +++ b/src/i18n/tests.rs @@ -1,11 +1,10 @@ use std::collections::BTreeMap; -use std::ffi::CString; use std::os::unix; use std::sync::mpsc; use std::time::Duration; use std::{fs, thread}; -use crate::test_env; +use crate::{test_env, test_temp}; use super::*; @@ -314,9 +313,7 @@ fn a_symlinked_override_is_refused() { fn a_fifo_override_is_refused_without_waiting_for_a_writer() { let directory = tempfile::tempdir().expect("override directory"); let path = directory.path().join("zh-CN.toml"); - let raw_path = CString::new(path.as_os_str().as_encoded_bytes()).expect("path"); - // SAFETY: `raw_path` is a valid NUL-terminated path for the call. - assert_eq!(unsafe { libc::mkfifo(raw_path.as_ptr(), 0o600) }, 0); + test_temp::make_fifo(&path); let (sender, receiver) = mpsc::channel(); thread::spawn(move || { diff --git a/src/process_broker/bootstrap.rs b/src/process_broker/bootstrap.rs index 0368352f..611e8ab5 100644 --- a/src/process_broker/bootstrap.rs +++ b/src/process_broker/bootstrap.rs @@ -1,5 +1,5 @@ use std::io; -use std::os::fd::{AsRawFd, FromRawFd, OwnedFd}; +use std::os::fd::{AsRawFd, OwnedFd}; use std::sync::atomic::AtomicBool; use std::sync::{Arc, Mutex}; @@ -17,6 +17,13 @@ use std::thread::Builder; use std::time::{Duration, Instant}; use anyhow::{Context, Result}; +#[cfg(not(test))] +use rustix::io as rustix_io; +use rustix::io::Errno; +use rustix::net::{self, AddressFamily, SocketFlags, SocketType}; +#[cfg(not(test))] +use rustix::process::Signal; +use rustix::process::{self as rustix_process, Pid, WaitOptions}; use crate::daemon::protocol_v2::ProtocolToken; #[cfg(not(test))] @@ -152,9 +159,8 @@ pub(super) fn start() -> Result { }), }; if let Err(error) = broker.request(BrokerOperation::Ping) { - // SAFETY: pid is the raw-clone broker child created above. - unsafe { - libc::kill(pid, libc::SIGKILL); + if let Some(broker_pid) = Pid::from_raw(pid) { + let _ = rustix_process::kill_process(broker_pid, Signal::KILL); } wait_for_broker_process(pid); return Err(error).context("process broker exec/authentication handshake failed"); @@ -164,37 +170,20 @@ pub(super) fn start() -> Result { #[cfg(not(test))] fn duplicate_for_exec(descriptor: &OwnedFd) -> Result { - // SAFETY: F_DUPFD_CLOEXEC duplicates the live descriptor at or above five. - let duplicate = unsafe { libc::fcntl(descriptor.as_raw_fd(), libc::F_DUPFD_CLOEXEC, 5) }; - if duplicate < 0 { - return Err(io::Error::last_os_error()).context("failed to stage broker descriptor"); - } - // SAFETY: fcntl returned a fresh owned descriptor. - Ok(unsafe { OwnedFd::from_raw_fd(duplicate) }) + rustix_io::fcntl_dupfd_cloexec(descriptor, 5) + .map_err(io::Error::from) + .context("failed to stage broker descriptor") } fn socket_pair(label: &str) -> Result<(OwnedFd, OwnedFd)> { - let mut sockets = [0; 2]; - // SAFETY: sockets has room for the returned descriptor pair. - if unsafe { - libc::socketpair( - libc::AF_UNIX, - libc::SOCK_SEQPACKET | libc::SOCK_CLOEXEC, - 0, - sockets.as_mut_ptr(), - ) - } != 0 - { - return Err(io::Error::last_os_error()) - .with_context(|| format!("failed to create {label} socketpair")); - } - // SAFETY: socketpair returned two new descriptors. - Ok(unsafe { - ( - OwnedFd::from_raw_fd(sockets[0]), - OwnedFd::from_raw_fd(sockets[1]), - ) - }) + net::socketpair( + AddressFamily::UNIX, + SocketType::SEQPACKET, + SocketFlags::CLOEXEC, + None, + ) + .map_err(io::Error::from) + .with_context(|| format!("failed to create {label} socketpair")) } /// Whether the broker process exits within `grace`, reaping it if it does. @@ -203,20 +192,17 @@ pub(super) fn broker_exits_within(child_pid: libc::pid_t, grace: Duration) -> bo return true; } + let Some(pid) = Pid::from_raw(child_pid) else { + return true; + }; let deadline = Instant::now() + grace; - let mut status = 0; loop { - // SAFETY: child_pid names the raw-clone broker child owned by its guard. - let result = unsafe { libc::waitpid(child_pid, &raw mut status, libc::WNOHANG) }; - if result == child_pid { - return true; - } - if result < 0 { - if io::Error::last_os_error().kind() == io::ErrorKind::Interrupted { - continue; - } + match rustix_process::waitpid(Some(pid), WaitOptions::NOHANG) { + Ok(Some(_)) => return true, + Ok(None) => {} + Err(Errno::INTR) => continue, // Already reaped: nothing is left to wait for. - return true; + Err(_) => return true, } if Instant::now() >= deadline { return false; @@ -229,16 +215,11 @@ pub(super) fn wait_for_broker_process(child_pid: libc::pid_t) { if child_pid <= 0 { return; } - let mut status = 0; - loop { - // SAFETY: child_pid names the raw-clone broker child owned by its guard. - let result = unsafe { libc::waitpid(child_pid, &raw mut status, 0) }; - if result == child_pid { - break; - } - if result < 0 && io::Error::last_os_error().kind() == io::ErrorKind::Interrupted { - continue; - } - break; - } + let Some(pid) = Pid::from_raw(child_pid) else { + return; + }; + while matches!( + rustix_process::waitpid(Some(pid), WaitOptions::empty()), + Err(Errno::INTR) + ) {} } diff --git a/src/process_broker/client.rs b/src/process_broker/client.rs index 0cfdd0a8..fbfc0313 100644 --- a/src/process_broker/client.rs +++ b/src/process_broker/client.rs @@ -2,7 +2,7 @@ use std::collections::VecDeque; use std::ffi::{OsStr, OsString}; use std::io; use std::io::ErrorKind; -use std::os::fd::{AsRawFd, OwnedFd, RawFd}; +use std::os::fd::{AsFd, BorrowedFd, OwnedFd}; use std::sync::atomic::{AtomicBool, Ordering}; use std::sync::{Arc, Mutex, MutexGuard, OnceLock, TryLockError, Weak}; #[cfg(test)] @@ -10,6 +10,9 @@ use std::thread::JoinHandle; use std::time::Duration; use anyhow::{Context, Result, anyhow, bail}; +use rustix::io::Errno; +use rustix::net::{self, SendFlags}; +use rustix::process::{self as rustix_process, Pid, Signal}; use crate::daemon::protocol_v2::ProtocolId; @@ -87,8 +90,8 @@ enum ExchangeWait { /// The transport policy and watchdog a spawn request carries, kept together so /// the private spawn path stays inside the argument-count lint. #[derive(Debug, Clone, Copy)] -struct SpawnOptions { - watchdog: Option, +struct SpawnOptions<'descriptor> { + watchdog: Option>, wait: ExchangeWait, } @@ -119,7 +122,7 @@ pub(crate) fn current() -> Result { impl Drop for ProcessBrokerGuard { fn drop(&mut self) { let was_healthy = self.broker.inner.healthy.swap(false, Ordering::AcqRel); - let asked_to_stop = signal_shutdown(self.broker.inner.shutdown.as_raw_fd()).is_ok(); + let asked_to_stop = signal_shutdown(self.broker.inner.shutdown.as_fd()).is_ok(); // A broker whose exchange failed may be stuck and never read the // shutdown request. Waiting on it without a bound would hang the very // exit that lets the service manager start a fresh daemon. @@ -129,11 +132,11 @@ impl Drop for ProcessBrokerGuard { self.broker.inner.child_pid, UNHEALTHY_BROKER_EXIT_GRACE, )); - if must_kill && self.broker.inner.child_pid > 0 { - // SAFETY: child_pid is the broker process owned by this guard. - unsafe { - libc::kill(self.broker.inner.child_pid, libc::SIGKILL); - } + if must_kill + && self.broker.inner.child_pid > 0 + && let Some(pid) = Pid::from_raw(self.broker.inner.child_pid) + { + let _ = rustix_process::kill_process(pid, Signal::KILL); } #[cfg(test)] if let Some(thread) = self @@ -163,29 +166,20 @@ impl Drop for ProcessBrokerGuard { } } -fn signal_shutdown(descriptor: RawFd) -> io::Result<()> { +fn signal_shutdown(descriptor: BorrowedFd<'_>) -> io::Result<()> { let byte = [GRACEFUL_SHUTDOWN_BYTE]; loop { - // SAFETY: descriptor is the broker shutdown socket and byte is readable. - let written = unsafe { - libc::send( - descriptor, - byte.as_ptr().cast(), - byte.len(), - libc::MSG_NOSIGNAL, - ) - }; - if written == 1 { - return Ok(()); + match net::send(descriptor, &byte, SendFlags::NOSIGNAL) { + Ok(1) => return Ok(()), + Ok(_) => { + return Err(io::Error::new( + ErrorKind::WriteZero, + "short broker shutdown write", + )); + } + Err(Errno::INTR) => {} + Err(errno) => return Err(errno.into()), } - if written < 0 && io::Error::last_os_error().kind() == ErrorKind::Interrupted { - continue; - } - return Err(if written < 0 { - io::Error::last_os_error() - } else { - io::Error::new(ErrorKind::WriteZero, "short broker shutdown write") - }); } } @@ -253,7 +247,7 @@ impl ProcessBroker { fn request_with_descriptors( &self, operation: BrokerOperation, - descriptors: &[RawFd], + descriptors: &[BorrowedFd<'_>], wait: ExchangeWait, ) -> Result<(BrokerOutcome, Vec)> { let request_id = ProtocolId::generate()?.to_string(); @@ -271,9 +265,9 @@ impl ProcessBroker { bail!("process broker transport is no longer usable"); } let exchange = (|| -> Result<(BrokerResponse, Vec)> { - set_socket_timeout(self.inner.socket.as_raw_fd(), exchange_timeout)?; - send_packet(self.inner.socket.as_raw_fd(), &packet, descriptors)?; - let (packet, descriptors) = recv_packet(self.inner.socket.as_raw_fd())?; + set_socket_timeout(self.inner.socket.as_fd(), exchange_timeout)?; + send_packet(self.inner.socket.as_fd(), &packet, descriptors)?; + let (packet, descriptors) = recv_packet(self.inner.socket.as_fd())?; let response: BrokerResponse = serde_json::from_slice(&packet)?; if response.request_id != request_id { bail!("broker response identity mismatch"); @@ -368,7 +362,7 @@ impl ProcessBroker { let (input, input_descriptor) = encode_blob(input, MAX_INPUT_BYTES)?; let request_descriptors = input_descriptor .as_ref() - .map(|descriptor| vec![descriptor.as_raw_fd()]) + .map(|descriptor| vec![descriptor.as_fd()]) .unwrap_or_default(); let (outcome, descriptors) = self.request_with_descriptors( BrokerOperation::Run { @@ -428,7 +422,7 @@ impl ProcessBroker { let (input, input_descriptor) = encode_blob(input, super::manifest::input_cap(kind))?; let request_descriptors = input_descriptor .as_ref() - .map(|descriptor| vec![descriptor.as_raw_fd()]) + .map(|descriptor| vec![descriptor.as_fd()]) .unwrap_or_default(); let (outcome, descriptors) = self.request_with_descriptors( BrokerOperation::Publish { @@ -530,7 +524,7 @@ impl ProcessBroker { program: &OsStr, arguments: I, environment: Vec<(OsString, Option)>, - watchdog: RawFd, + watchdog: BorrowedFd<'_>, ) -> Result where I: IntoIterator, @@ -556,7 +550,7 @@ impl ProcessBroker { program: &OsStr, arguments: I, environment: Vec<(OsString, Option)>, - options: SpawnOptions, + options: SpawnOptions<'_>, ) -> Result where I: IntoIterator, diff --git a/src/process_broker/execution.rs b/src/process_broker/execution.rs index 7700d216..0444ceb6 100644 --- a/src/process_broker/execution.rs +++ b/src/process_broker/execution.rs @@ -1,7 +1,6 @@ use std::collections::BTreeMap; use std::io::{self, Read, Write}; -use std::mem::MaybeUninit; -use std::os::fd::{AsRawFd, RawFd}; +use std::os::fd::{AsFd, BorrowedFd}; use std::os::unix::process::CommandExt; use std::process::{Child, ChildStdin, Command, ExitStatus, Stdio}; use std::sync::Arc; @@ -10,6 +9,10 @@ use std::time::Duration; use std::{mem, thread}; use anyhow::{Context, Result, anyhow}; +use rustix::event::{self, PollFd, PollFlags, Timespec}; +use rustix::fs::{self as rustix_fs, OFlags}; +use rustix::io::Errno; +use rustix::process::{self as rustix_process, Pid, Signal, WaitId, WaitIdOptions}; use crate::daemon::protocol_v2::{BootClock, BootDeadline}; @@ -25,7 +28,10 @@ use super::wire::{HelperKind, MAX_STDERR_BYTES, OutputMode, STDOUT_CAP_EXCEEDED} const PIPE_DRAIN_GRACE: Duration = Duration::from_secs(2); /// How often a pipe thread waiting on an idle pipe checks whether to give up. -const PIPE_POLL_INTERVAL_MS: libc::c_int = 20; +const PIPE_POLL_INTERVAL: Timespec = Timespec { + tv_sec: 0, + tv_nsec: 20_000_000, +}; const PIPE_HELD_OPEN: &str = "broker helper pipe is still open after the helper exited"; @@ -63,12 +69,9 @@ pub(super) fn terminate_owned_children(children: &mut BTreeMap) { } pub(super) fn kill_child_process_group(child: &mut Child) { - let pid = child.id(); - if let Ok(group) = i32::try_from(pid) { - // SAFETY: broker children use a fresh process group equal to their PID. - if unsafe { libc::kill(-group, libc::SIGKILL) } == 0 { - return; - } + // Broker children use a fresh process group equal to their PID. + if rustix_process::kill_process_group(Pid::from_child(child), Signal::KILL).is_ok() { + return; } let _ = child.kill(); } @@ -145,48 +148,28 @@ impl Drop for OwnedProcess { } fn child_status_unreaped(child: &Child) -> io::Result> { - let mut info = MaybeUninit::::zeroed(); // WNOWAIT pins the zombie leader and therefore its process-group identity. - // SAFETY: info is writable and child is owned by this broker. - if unsafe { - libc::waitid( - libc::P_PID, - child.id(), - info.as_mut_ptr(), - libc::WEXITED | libc::WNOHANG | libc::WNOWAIT, - ) - } != 0 - { - return Err(io::Error::last_os_error()); - } - // SAFETY: waitid initialized info after success. - let info = unsafe { info.assume_init() }; - // SAFETY: info was zeroed before the call and waitid only writes a SIGCHLD - // record into it, so the pid field it reads is initialized; it stays zero - // when WNOHANG found no waitable child. - if unsafe { info.si_pid() } == 0 { + let options = WaitIdOptions::EXITED | WaitIdOptions::NOHANG | WaitIdOptions::NOWAIT; + let Some(status) = rustix_process::waitid(WaitId::Pid(Pid::from_child(child)), options)? else { return Ok(None); - } - // SAFETY: CLD terminal records initialize si_status. - let detail = unsafe { info.si_status() }; - let status = match info.si_code { - libc::CLD_EXITED => detail, - libc::CLD_KILLED | libc::CLD_DUMPED => 128_i32.saturating_add(detail), - _ => { - return Err(io::Error::new( - io::ErrorKind::InvalidData, - "waitid returned a nonterminal child status", - )); - } }; - Ok(Some(status)) + if let Some(code) = status.exit_status() { + return Ok(Some(code)); + } + if let Some(signal) = status.terminating_signal() { + return Ok(Some(128_i32.saturating_add(signal))); + } + Err(io::Error::new( + io::ErrorKind::InvalidData, + "waitid returned a nonterminal child status", + )) } pub(super) fn publish_bounded( mut command: Command, input: Vec, timeout: Duration, - shutdown_fd: RawFd, + shutdown_fd: BorrowedFd<'static>, ) -> Result { if shutdown_requested(shutdown_fd)? { return Err(anyhow!("broker publication cancelled during shutdown")); @@ -254,17 +237,10 @@ fn write_input_until( mut stdin: ChildStdin, input: &[u8], deadline: BootDeadline, - shutdown_fd: RawFd, + shutdown_fd: BorrowedFd<'static>, ) -> io::Result<()> { - let descriptor = stdin.as_raw_fd(); - // SAFETY: fcntl reads and updates descriptor-local flags. - let flags = unsafe { libc::fcntl(descriptor, libc::F_GETFL) }; - // SAFETY: `stdin` keeps the descriptor open, and F_SETFL only adds - // O_NONBLOCK to the flags just read from it. - if flags < 0 || unsafe { libc::fcntl(descriptor, libc::F_SETFL, flags | libc::O_NONBLOCK) } != 0 - { - return Err(io::Error::last_os_error()); - } + let flags = rustix_fs::fcntl_getfl(&stdin)?; + rustix_fs::fcntl_setfl(&stdin, flags | OFlags::NONBLOCK)?; let mut offset = 0; while offset < input.len() { if shutdown_requested(shutdown_fd)? { @@ -298,7 +274,7 @@ pub(super) fn run_bounded( timeout: Duration, output_cap: usize, output_mode: OutputMode, - shutdown_fd: RawFd, + shutdown_fd: BorrowedFd<'static>, ) -> Result { if shutdown_requested(shutdown_fd)? { return Err(anyhow!("broker helper cancelled during shutdown")); @@ -421,7 +397,7 @@ struct AbandonablePipe

{ abandon: Arc, } -impl AbandonablePipe

{ +impl AbandonablePipe

{ fn new(pipe: P, abandon: &Arc) -> Self { Self { pipe, @@ -429,42 +405,31 @@ impl AbandonablePipe

{ } } - fn wait_until_ready(&self, events: libc::c_short) -> io::Result<()> { - let mut pollfd = libc::pollfd { - fd: self.pipe.as_raw_fd(), - events, - revents: 0, - }; + fn wait_until_ready(&self, events: PollFlags) -> io::Result<()> { + let mut pollfds = [PollFd::new(&self.pipe, events)]; loop { if self.abandon.load(Ordering::Acquire) { return Err(io::Error::new(io::ErrorKind::TimedOut, PIPE_HELD_OPEN)); } - // SAFETY: pollfd points to one initialized entry, and the pipe - // stays open for as long as `self` owns it. - let result = unsafe { libc::poll(&raw mut pollfd, 1, PIPE_POLL_INTERVAL_MS) }; - if result > 0 { - return Ok(()); - } - if result < 0 { - let error = io::Error::last_os_error(); - if error.kind() != io::ErrorKind::Interrupted { - return Err(error); - } + match event::poll(&mut pollfds, Some(&PIPE_POLL_INTERVAL)) { + Ok(0) | Err(Errno::INTR) => {} + Ok(_) => return Ok(()), + Err(errno) => return Err(errno.into()), } } } } -impl Read for AbandonablePipe

{ +impl Read for AbandonablePipe

{ fn read(&mut self, buffer: &mut [u8]) -> io::Result { - self.wait_until_ready(libc::POLLIN)?; + self.wait_until_ready(PollFlags::IN)?; self.pipe.read(buffer) } } -impl Write for AbandonablePipe

{ +impl Write for AbandonablePipe

{ fn write(&mut self, buffer: &[u8]) -> io::Result { - self.wait_until_ready(libc::POLLOUT)?; + self.wait_until_ready(PollFlags::OUT)?; // A writable pipe has room for at least PIPE_BUF bytes, so a write of // no more than that cannot block on a reader that stopped reading. self.pipe.write(&buffer[..buffer.len().min(libc::PIPE_BUF)]) diff --git a/src/process_broker/server.rs b/src/process_broker/server.rs index a9ab880e..1b41bb21 100644 --- a/src/process_broker/server.rs +++ b/src/process_broker/server.rs @@ -1,5 +1,5 @@ use std::collections::{BTreeMap, VecDeque}; -use std::os::fd::{AsRawFd, OwnedFd, RawFd}; +use std::os::fd::{AsFd, AsRawFd, BorrowedFd, OwnedFd, RawFd}; use std::os::unix::process::CommandExt; use std::process::{Child, ExitCode, Stdio}; use std::thread::Builder; @@ -7,6 +7,9 @@ use std::time::Duration; use std::{env, io, mem}; use anyhow::{Context, Result, anyhow, bail}; +use rustix::event::{self, PollFd, PollFlags}; +use rustix::io::{self as rustix_io, Errno, FdFlags}; +use rustix::process::{self as rustix_process, Pid, Signal}; use crate::daemon::protocol_v2::ProtocolId; use crate::env_vars; @@ -41,10 +44,17 @@ pub(crate) fn run_internal_broker_if_requested() -> Option { { return Some(ExitCode::from(126)); } + // SAFETY: both numbers were just checked to be open broker sockets, and + // nothing in this process closes them before it exits. + let (fd, shutdown_fd) = unsafe { + ( + BorrowedFd::borrow_raw(fd), + BorrowedFd::borrow_raw(shutdown_fd), + ) + }; // Restore CLOEXEC before any runtime helper can inherit broker internals. for descriptor in [fd, shutdown_fd] { - // SAFETY: both descriptors were validated as inherited broker channels. - if unsafe { libc::fcntl(descriptor, libc::F_SETFD, libc::FD_CLOEXEC) } != 0 { + if rustix_io::fcntl_setfd(descriptor, FdFlags::CLOEXEC).is_err() { return Some(ExitCode::from(126)); } } @@ -63,7 +73,9 @@ pub(crate) fn run_internal_broker_if_requested() -> Option { fn validate_broker_socket(fd: RawFd) -> io::Result<()> { let mut socket_type = 0_i32; let mut length = mem::size_of::() as libc::socklen_t; - // SAFETY: the output slots are valid for getsockopt. + // SAFETY: the output slots are valid for getsockopt, which also rejects a + // number that is not an open descriptor. A borrowed descriptor cannot be + // used here because it requires the number to be open already. if unsafe { libc::getsockopt( fd, @@ -85,7 +97,11 @@ fn validate_broker_socket(fd: RawFd) -> io::Result<()> { Ok(()) } -fn broker_loop(socket: RawFd, shutdown_fd: RawFd, token: &str) -> Result<()> { +fn broker_loop( + socket: BorrowedFd<'static>, + shutdown_fd: BorrowedFd<'static>, + token: &str, +) -> Result<()> { let mut ownership = BrokerOwnership::default(); loop { if wait_for_request(socket, shutdown_fd)? == BrokerWake::Shutdown { @@ -139,7 +155,7 @@ fn broker_loop(socket: RawFd, shutdown_fd: RawFd, token: &str) -> Result<()> { let response_descriptors = wire_response .descriptors .iter() - .map(AsRawFd::as_raw_fd) + .map(AsFd::as_fd) .collect::>(); send_packet(socket, &bytes, &response_descriptors)?; if take_graceful_shutdown_signal(shutdown_fd)? { @@ -155,39 +171,27 @@ enum BrokerWake { Shutdown, } -fn wait_for_request(socket: RawFd, shutdown_fd: RawFd) -> io::Result { +fn wait_for_request(socket: BorrowedFd<'_>, shutdown_fd: BorrowedFd<'_>) -> io::Result { let mut descriptors = [ - libc::pollfd { - fd: socket, - events: libc::POLLIN, - revents: 0, - }, - libc::pollfd { - fd: shutdown_fd, - events: libc::POLLIN, - revents: 0, - }, + PollFd::from_borrowed_fd(socket, PollFlags::IN), + PollFd::from_borrowed_fd(shutdown_fd, PollFlags::IN), ]; + let unusable = PollFlags::HUP | PollFlags::ERR | PollFlags::NVAL; loop { - // SAFETY: descriptors points to two initialized pollfd entries. - let result = unsafe { libc::poll(descriptors.as_mut_ptr(), descriptors.len() as _, -1) }; - if result < 0 { - let error = io::Error::last_os_error(); - if error.kind() == io::ErrorKind::Interrupted { - continue; - } - return Err(error); + match event::poll(&mut descriptors, None) { + Ok(_) => {} + Err(Errno::INTR) => continue, + Err(errno) => return Err(errno.into()), } - if descriptors[1].revents & libc::POLLIN != 0 && take_graceful_shutdown_signal(shutdown_fd)? - { + let socket_events = descriptors[0].revents(); + let shutdown_events = descriptors[1].revents(); + if shutdown_events.contains(PollFlags::IN) && take_graceful_shutdown_signal(shutdown_fd)? { return Ok(BrokerWake::Shutdown); } - if descriptors[0].revents & libc::POLLIN != 0 { + if socket_events.contains(PollFlags::IN) { return Ok(BrokerWake::Request); } - if descriptors[0].revents & (libc::POLLHUP | libc::POLLERR | libc::POLLNVAL) != 0 - || descriptors[1].revents & (libc::POLLHUP | libc::POLLERR | libc::POLLNVAL) != 0 - { + if socket_events.intersects(unusable) || shutdown_events.intersects(unusable) { return Err(io::Error::new( io::ErrorKind::BrokenPipe, "broker control channel became unusable", @@ -198,6 +202,14 @@ fn wait_for_request(socket: RawFd, shutdown_fd: RawFd) -> io::Result #[cfg(test)] pub(super) fn run_loop_for_test(socket: RawFd, shutdown_fd: RawFd, token: &str) -> Result<()> { + // SAFETY: the test broker thread owns both descriptors and keeps them open + // until this loop returns. + let (socket, shutdown_fd) = unsafe { + ( + BorrowedFd::borrow_raw(socket), + BorrowedFd::borrow_raw(shutdown_fd), + ) + }; broker_loop(socket, shutdown_fd, token) } @@ -242,7 +254,7 @@ fn handle_operation( operation: BrokerOperation, descriptors: &mut VecDeque, ownership: &mut BrokerOwnership, - shutdown_fd: RawFd, + shutdown_fd: BorrowedFd<'static>, ) -> Result { if shutdown_requested(shutdown_fd)? { bail!("broker operation cancelled during shutdown"); @@ -350,20 +362,20 @@ fn handle_operation( ), BrokerOperation::Signal { handle, signal } => { reject_descriptors(descriptors)?; - if !matches!( - signal, - libc::SIGUSR1 | libc::SIGUSR2 | libc::SIGTERM | libc::SIGKILL - ) { - bail!("signal is not allowed by broker manifest"); - } + let signal = match signal { + libc::SIGUSR1 => Signal::USR1, + libc::SIGUSR2 => Signal::USR2, + libc::SIGTERM => Signal::TERM, + libc::SIGKILL => Signal::KILL, + _ => bail!("signal is not allowed by broker manifest"), + }; let child = ownership .children .get(&handle) .ok_or_else(|| anyhow!("unknown broker child handle"))?; - // SAFETY: the broker retains the exact unreaped child handle. - if unsafe { libc::kill(child.id() as i32, signal) } != 0 { - return Err(io::Error::last_os_error()).context("broker child signal failed"); - } + rustix_process::kill_process(Pid::from_child(child), signal) + .map_err(io::Error::from) + .context("broker child signal failed")?; Ok(wire_outcome(BrokerOutcome::Acknowledged)) } BrokerOperation::TryWait { handle } => { @@ -409,7 +421,7 @@ fn spawn_helper( request: SpawnRequest, descriptors: &mut VecDeque, children: &mut BTreeMap, - shutdown_fd: RawFd, + shutdown_fd: BorrowedFd<'static>, ) -> Result { let SpawnRequest { kind, @@ -504,22 +516,17 @@ fn reject_descriptors(descriptors: &VecDeque) -> Result<()> { } fn set_cloexec(descriptor: &OwnedFd, enabled: bool) -> Result<()> { - // SAFETY: fcntl reads and updates descriptor-local flags. - let current = unsafe { libc::fcntl(descriptor.as_raw_fd(), libc::F_GETFD) }; - if current < 0 { - return Err(io::Error::last_os_error()).context("failed to read descriptor flags"); - } + let current = rustix_io::fcntl_getfd(descriptor) + .map_err(io::Error::from) + .context("failed to read descriptor flags")?; let updated = if enabled { - current | libc::FD_CLOEXEC + current | FdFlags::CLOEXEC } else { - current & !libc::FD_CLOEXEC + current - FdFlags::CLOEXEC }; - // SAFETY: `descriptor` is borrowed open for the call, and `updated` only - // changes FD_CLOEXEC in the flags just read from it. - if unsafe { libc::fcntl(descriptor.as_raw_fd(), libc::F_SETFD, updated) } != 0 { - return Err(io::Error::last_os_error()).context("failed to update descriptor flags"); - } - Ok(()) + rustix_io::fcntl_setfd(descriptor, updated) + .map_err(io::Error::from) + .context("failed to update descriptor flags") } fn wire_outcome(outcome: BrokerOutcome) -> BrokerWireResponse { @@ -551,30 +558,18 @@ fn canonical_lower_hex(value: &str, length: usize) -> bool { mod tests { use std::process::Command; + use rustix::net::{self, AddressFamily, SendFlags, SocketFlags, SocketType}; + use super::*; - use std::os::fd::FromRawFd; fn test_socket_pair() -> (OwnedFd, OwnedFd) { - let mut sockets = [0; 2]; - assert_eq!( - // SAFETY: sockets has room for both returned descriptors. - unsafe { - libc::socketpair( - libc::AF_UNIX, - libc::SOCK_SEQPACKET | libc::SOCK_CLOEXEC, - 0, - sockets.as_mut_ptr(), - ) - }, - 0 - ); - // SAFETY: socketpair returned two fresh owned descriptors. - unsafe { - ( - OwnedFd::from_raw_fd(sockets[0]), - OwnedFd::from_raw_fd(sockets[1]), - ) - } + net::socketpair( + AddressFamily::UNIX, + SocketType::SEQPACKET, + SocketFlags::CLOEXEC, + None, + ) + .unwrap() } #[test] @@ -583,8 +578,7 @@ mod tests { let (shutdown_writer, shutdown_reader) = test_socket_pair(); drop(shutdown_writer); - let Err(error) = wait_for_request(control_reader.as_raw_fd(), shutdown_reader.as_raw_fd()) - else { + let Err(error) = wait_for_request(control_reader.as_fd(), shutdown_reader.as_fd()) else { panic!("hangup must unwind through destructive broker cleanup"); }; @@ -597,21 +591,13 @@ mod tests { let (shutdown_writer, shutdown_reader) = test_socket_pair(); let signal = [super::super::transport::GRACEFUL_SHUTDOWN_BYTE]; assert_eq!( - // SAFETY: both the descriptor and one-byte buffer are valid. - unsafe { - libc::send( - shutdown_writer.as_raw_fd(), - signal.as_ptr().cast(), - signal.len(), - libc::MSG_NOSIGNAL, - ) - }, + net::send(&shutdown_writer, &signal, SendFlags::NOSIGNAL).unwrap(), 1 ); drop(shutdown_writer); assert!(matches!( - wait_for_request(control_reader.as_raw_fd(), shutdown_reader.as_raw_fd()), + wait_for_request(control_reader.as_fd(), shutdown_reader.as_fd()), Ok(BrokerWake::Shutdown) )); } @@ -622,20 +608,11 @@ mod tests { let (shutdown_writer, shutdown_reader) = test_socket_pair(); let invalid = [2_u8]; assert_eq!( - // SAFETY: both the descriptor and one-byte buffer are valid. - unsafe { - libc::send( - shutdown_writer.as_raw_fd(), - invalid.as_ptr().cast(), - invalid.len(), - libc::MSG_NOSIGNAL, - ) - }, + net::send(&shutdown_writer, &invalid, SendFlags::NOSIGNAL).unwrap(), 1 ); - let Err(error) = wait_for_request(control_reader.as_raw_fd(), shutdown_reader.as_raw_fd()) - else { + let Err(error) = wait_for_request(control_reader.as_fd(), shutdown_reader.as_fd()) else { panic!("invalid packet must not authorize ownership release"); }; @@ -648,20 +625,11 @@ mod tests { let (shutdown_writer, shutdown_reader) = test_socket_pair(); let invalid = [super::super::transport::GRACEFUL_SHUTDOWN_BYTE; 2]; assert_eq!( - // SAFETY: both the descriptor and two-byte buffer are valid. - unsafe { - libc::send( - shutdown_writer.as_raw_fd(), - invalid.as_ptr().cast(), - invalid.len(), - libc::MSG_NOSIGNAL, - ) - }, + net::send(&shutdown_writer, &invalid, SendFlags::NOSIGNAL).unwrap(), 2 ); - let Err(error) = wait_for_request(control_reader.as_raw_fd(), shutdown_reader.as_raw_fd()) - else { + let Err(error) = wait_for_request(control_reader.as_fd(), shutdown_reader.as_fd()) else { panic!("oversized packet must not authorize ownership release"); }; @@ -681,7 +649,6 @@ mod tests { drop(ownership); - // SAFETY: signal zero only probes the test-owned provider after teardown. - assert_ne!(unsafe { libc::kill(pid, 0) }, 0); + assert!(rustix_process::test_kill_process(Pid::from_raw(pid).unwrap()).is_err()); } } diff --git a/src/process_broker/tests.rs b/src/process_broker/tests.rs index acd5cd15..15005482 100644 --- a/src/process_broker/tests.rs +++ b/src/process_broker/tests.rs @@ -1,5 +1,5 @@ use std::ffi::OsStr; -use std::os::fd::AsRawFd; +use std::os::fd::AsFd; use std::os::unix::process::CommandExt; use std::path::Path; use std::process::Command; @@ -7,10 +7,23 @@ use std::sync::mpsc; use std::time::{Duration, Instant}; use std::{env, fs, iter, thread}; +use rustix::net::{self, Shutdown}; +use rustix::process::{self as rustix_process, Pid, Signal}; + use crate::daemon::protocol_v2; use crate::test_fake_helper::Role; use crate::{env_vars, process_broker, test_env, test_fake_helper, test_temp}; +fn process_exists(pid: libc::pid_t) -> bool { + Pid::from_raw(pid).is_some_and(|pid| rustix_process::test_kill_process(pid).is_ok()) +} + +fn kill_for_cleanup(pid: libc::pid_t) { + if let Some(pid) = Pid::from_raw(pid) { + let _ = rustix_process::kill_process(pid, Signal::KILL); + } +} + use super::*; fn wire_arguments(arguments: &[&str]) -> Vec { @@ -28,10 +41,7 @@ fn release_test_provider(release_path: &Path, proof_path: &Path, provider_pid: i return; } if Instant::now() >= deadline { - // SAFETY: cleanup keeps a failing regression test from leaking its provider. - unsafe { - libc::kill(provider_pid, libc::SIGKILL); - } + kill_for_cleanup(provider_pid); panic!("successful provider could not act after normal broker shutdown"); } thread::sleep(Duration::from_millis(5)); @@ -540,8 +550,7 @@ fn process_group_guard_cleans_up_before_ownership_transfer() { drop(child); - // SAFETY: signal zero only probes the test-owned helper PID after guard cleanup. - assert_ne!(unsafe { libc::kill(pid, 0) }, 0); + assert!(!process_exists(pid)); } #[test] @@ -762,8 +771,7 @@ fn broker_guard_preempts_an_active_operation_and_kills_its_group() { "broker shutdown waited for the active operation" ); assert!(run.join().unwrap().is_err()); - // SAFETY: signal zero only probes the test-owned helper PID. - assert_ne!(unsafe { libc::kill(helper_pid, 0) }, 0); + assert!(!process_exists(helper_pid)); } #[test] @@ -778,7 +786,7 @@ fn owned_child_inherits_daemon_pidfd_without_leaking_broker_copy() { OsStr::new("sleep"), [OsStr::new("30")], Vec::new(), - watchdog.as_raw_fd(), + watchdog.as_fd(), ) .unwrap(); let inherited_pidfd = fs::read_dir(format!("/proc/{}/fd", child.id())) @@ -891,10 +899,7 @@ fn a_descendant_outside_the_helper_group_cannot_hold_the_broker_on_its_pipes() { .ok() .and_then(|pid| pid.trim().parse::().ok()) { - // SAFETY: cleanup of the descendant this test started in its own session. - unsafe { - libc::kill(pid, libc::SIGKILL); - } + kill_for_cleanup(pid); } let error = result.expect_err("output still held open must not read as complete"); @@ -950,8 +955,7 @@ fn normal_broker_shutdown_releases_successful_provider_descendant() { .trim() .parse::() .unwrap(); - // SAFETY: signal zero only checks the test-owned provider. - assert_eq!(unsafe { libc::kill(provider_pid, 0) }, 0); + assert!(process_exists(provider_pid)); drop(guard); @@ -984,28 +988,19 @@ fn shutdown_channel_peer_loss_kills_retained_provider() { .trim() .parse::() .unwrap(); - // SAFETY: signal zero only checks the test-owned provider. - assert_eq!(unsafe { libc::kill(provider_pid, 0) }, 0); + assert!(process_exists(provider_pid)); // Simulate abrupt parent loss without writing the graceful-shutdown packet. - assert_eq!( - // SAFETY: the descriptor belongs to this test's live broker guard. - unsafe { libc::shutdown(guard.broker().inner.shutdown.as_raw_fd(), libc::SHUT_RDWR) }, - 0 - ); + net::shutdown(&guard.broker().inner.shutdown, Shutdown::Both).unwrap(); drop(guard); let deadline = Instant::now() + Duration::from_secs(1); loop { - // SAFETY: signal zero only probes the recorded test provider PID. - if unsafe { libc::kill(provider_pid, 0) } != 0 { + if !process_exists(provider_pid) { break; } if Instant::now() >= deadline { - // SAFETY: cleanup keeps a failing regression test from leaking its provider. - unsafe { - libc::kill(provider_pid, libc::SIGKILL); - } + kill_for_cleanup(provider_pid); panic!("provider survived abnormal broker channel loss"); } thread::sleep(Duration::from_millis(5)); @@ -1069,22 +1064,17 @@ fn retained_publication_replacement_disposes_the_previous_provider() { .unwrap(); let deadline = Instant::now() + Duration::from_secs(1); loop { - // SAFETY: signal zero only probes the recorded test provider PID. - if unsafe { libc::kill(first_pid, 0) } != 0 { + if !process_exists(first_pid) { break; } if Instant::now() >= deadline { - // SAFETY: cleanup keeps a failing regression test from leaking its helpers. - unsafe { - libc::kill(first_pid, libc::SIGKILL); - libc::kill(second_pid, libc::SIGKILL); - } + kill_for_cleanup(first_pid); + kill_for_cleanup(second_pid); panic!("replaced publication provider remained alive"); } thread::sleep(Duration::from_millis(5)); } - // SAFETY: signal zero only checks the current test-owned provider. - assert_eq!(unsafe { libc::kill(second_pid, 0) }, 0); + assert!(process_exists(second_pid)); drop(guard); release_test_provider(&second_release_path, &second_proof_path, second_pid); @@ -1146,20 +1136,15 @@ fn failed_publication_replacement_preserves_the_current_provider() { .trim() .parse::() .unwrap(); - // SAFETY: signal zero only checks the current test-owned provider. - assert_eq!(unsafe { libc::kill(current_pid, 0) }, 0); + assert!(process_exists(current_pid)); let deadline = Instant::now() + Duration::from_secs(1); loop { - // SAFETY: signal zero only probes the failed test provider PID. - if unsafe { libc::kill(failed_pid, 0) } != 0 { + if !process_exists(failed_pid) { break; } if Instant::now() >= deadline { - // SAFETY: cleanup keeps a failing regression test from leaking its helpers. - unsafe { - libc::kill(current_pid, libc::SIGKILL); - libc::kill(failed_pid, libc::SIGKILL); - } + kill_for_cleanup(current_pid); + kill_for_cleanup(failed_pid); panic!("failed replacement provider survived cleanup"); } thread::sleep(Duration::from_millis(5)); @@ -1205,8 +1190,7 @@ fn retained_publication_kills_failed_or_input_stalled_provider_groups() { } let deadline = Instant::now() + Duration::from_secs(1); loop { - // SAFETY: signal zero only probes the recorded test child PID. - if unsafe { libc::kill(provider_pid, 0) } != 0 { + if !process_exists(provider_pid) { break; } assert!( @@ -1307,8 +1291,7 @@ fn broker_shutdown_preempts_retained_publication_stdin_writer() { release_test_provider(¤t_release_path, ¤t_proof_path, current_pid); let deadline = Instant::now() + Duration::from_secs(1); loop { - // SAFETY: signal zero only probes the recorded test child PID. - if unsafe { libc::kill(provider_pid, 0) } != 0 { + if !process_exists(provider_pid) { break; } assert!( diff --git a/src/process_broker/transport.rs b/src/process_broker/transport.rs index d3dca23b..a21048dd 100644 --- a/src/process_broker/transport.rs +++ b/src/process_broker/transport.rs @@ -1,10 +1,18 @@ use std::collections::VecDeque; +use std::io::{self, IoSlice, IoSliceMut}; use std::mem::MaybeUninit; -use std::os::fd::{AsRawFd, FromRawFd, OwnedFd, RawFd}; +use std::os::fd::{BorrowedFd, OwnedFd}; use std::time::Duration; -use std::{io, mem, ptr}; use anyhow::{Context, Result, anyhow, bail}; +use rustix::event::{self, PollFd, PollFlags, Timespec}; +use rustix::fs::{self as rustix_fs, FileType, MemfdFlags}; +use rustix::io::{self as rustix_io, Errno}; +use rustix::net::sockopt::{self, Timeout}; +use rustix::net::{ + self, RecvAncillaryBuffer, RecvAncillaryMessage, RecvFlags, ReturnFlags, SendAncillaryBuffer, + SendAncillaryMessage, SendFlags, +}; use super::wire::{ BlobWire, INLINE_BLOB_BYTES, MAX_OUTPUT_BYTES, MAX_PACKET_BYTES, MAX_PACKET_DESCRIPTORS, @@ -13,80 +21,47 @@ use super::wire::{ pub(super) const GRACEFUL_SHUTDOWN_BYTE: u8 = 1; -pub(super) fn set_socket_timeout(fd: RawFd, timeout: Duration) -> io::Result<()> { +pub(super) fn set_socket_timeout(fd: BorrowedFd<'_>, timeout: Duration) -> io::Result<()> { let timeout = timeout.max(Duration::from_millis(1)); - let value = libc::timeval { - tv_sec: libc::time_t::try_from(timeout.as_secs()) - .map_err(|_| io::Error::new(io::ErrorKind::InvalidInput, "socket timeout overflow"))?, - tv_usec: libc::suseconds_t::from(timeout.subsec_micros()), - }; - for option in [libc::SO_SNDTIMEO, libc::SO_RCVTIMEO] { - // SAFETY: value is initialized and setsockopt copies it. - if unsafe { - libc::setsockopt( - fd, - libc::SOL_SOCKET, - option, - (&raw const value).cast(), - mem::size_of::() as libc::socklen_t, - ) - } != 0 - { - return Err(io::Error::last_os_error()); - } + for option in [Timeout::Send, Timeout::Recv] { + sockopt::set_socket_timeout(fd, option, Some(timeout))?; } Ok(()) } -pub(super) fn shutdown_requested(descriptor: RawFd) -> io::Result { - let mut pollfd = libc::pollfd { - fd: descriptor, - events: libc::POLLIN, - revents: 0, - }; - // SAFETY: pollfd points to one initialized entry. - let result = unsafe { libc::poll(&raw mut pollfd, 1, 0) }; - if result < 0 { - return Err(io::Error::last_os_error()); - } - if pollfd.revents & libc::POLLNVAL != 0 { +pub(super) fn shutdown_requested(descriptor: BorrowedFd<'_>) -> io::Result { + let mut pollfds = [PollFd::from_borrowed_fd(descriptor, PollFlags::IN)]; + event::poll(&mut pollfds, Some(&Timespec::default()))?; + let revents = pollfds[0].revents(); + if revents.contains(PollFlags::NVAL) { return Err(io::Error::new( io::ErrorKind::BrokenPipe, "broker shutdown channel became invalid", )); } - Ok(pollfd.revents & (libc::POLLIN | libc::POLLHUP | libc::POLLERR) != 0) + Ok(revents.intersects(PollFlags::IN | PollFlags::HUP | PollFlags::ERR)) } -pub(super) fn take_graceful_shutdown_signal(descriptor: RawFd) -> io::Result { - let mut pollfd = libc::pollfd { - fd: descriptor, - events: libc::POLLIN, - revents: 0, - }; +pub(super) fn take_graceful_shutdown_signal(descriptor: BorrowedFd<'_>) -> io::Result { + let mut pollfds = [PollFd::from_borrowed_fd(descriptor, PollFlags::IN)]; loop { - // SAFETY: pollfd points to one initialized entry. - let result = unsafe { libc::poll(&raw mut pollfd, 1, 0) }; - if result < 0 { - let error = io::Error::last_os_error(); - if error.kind() == io::ErrorKind::Interrupted { - continue; - } - return Err(error); + match event::poll(&mut pollfds, Some(&Timespec::default())) { + Ok(0) => return Ok(false), + Ok(_) => {} + Err(Errno::INTR) => continue, + Err(errno) => return Err(errno.into()), } - if result == 0 { - return Ok(false); - } - if pollfd.revents & libc::POLLNVAL != 0 { + let revents = pollfds[0].revents(); + if revents.contains(PollFlags::NVAL) { return Err(io::Error::new( io::ErrorKind::BrokenPipe, "broker shutdown channel became invalid", )); } - if pollfd.revents & libc::POLLIN != 0 { + if revents.contains(PollFlags::IN) { return receive_graceful_shutdown_signal(descriptor); } - if pollfd.revents & (libc::POLLHUP | libc::POLLERR) != 0 { + if revents.intersects(PollFlags::HUP | PollFlags::ERR) { return Err(io::Error::new( io::ErrorKind::BrokenPipe, "broker shutdown channel closed without a graceful signal", @@ -96,27 +71,17 @@ pub(super) fn take_graceful_shutdown_signal(descriptor: RawFd) -> io::Result io::Result { - let mut byte = 0_u8; +fn receive_graceful_shutdown_signal(descriptor: BorrowedFd<'_>) -> io::Result { + let mut byte = [0_u8]; loop { - // MSG_TRUNC makes an oversized SOCK_SEQPACKET message report its full length. - // SAFETY: byte is a valid one-byte destination for this live broker socket. - let received = unsafe { - libc::recv( - descriptor, - (&raw mut byte).cast(), - 1, - libc::MSG_DONTWAIT | libc::MSG_TRUNC, - ) + // TRUNC makes an oversized SOCK_SEQPACKET message report its full length. + let flags = RecvFlags::DONTWAIT | RecvFlags::TRUNC; + let received = match net::recv(descriptor, &mut byte, flags) { + Ok((_, received)) => received, + Err(Errno::INTR) => continue, + Err(errno) => return Err(errno.into()), }; - if received < 0 { - let error = io::Error::last_os_error(); - if error.kind() == io::ErrorKind::Interrupted { - continue; - } - return Err(error); - } - if received == 1 && byte == GRACEFUL_SHUTDOWN_BYTE { + if received == 1 && byte[0] == GRACEFUL_SHUTDOWN_BYTE { return Ok(true); } return Err(io::Error::new( @@ -166,28 +131,11 @@ pub(super) fn decode_blob( let mut bytes = vec![0_u8; length]; let mut offset = 0; while offset < bytes.len() { - // SAFETY: the destination range is writable and the validated - // memfd has exactly the declared immutable length. - let read = unsafe { - libc::pread( - descriptor.as_raw_fd(), - bytes[offset..].as_mut_ptr().cast(), - bytes.len() - offset, - offset as libc::off_t, - ) - }; - if read > 0 { - offset += read as usize; - } else if read < 0 - && io::Error::last_os_error().kind() == io::ErrorKind::Interrupted - { - continue; - } else { - return Err(if read == 0 { - anyhow!("broker memfd ended before its declared length") - } else { - io::Error::last_os_error().into() - }); + match rustix_io::pread(&descriptor, &mut bytes[offset..], offset as u64) { + Ok(0) => bail!("broker memfd ended before its declared length"), + Ok(read) => offset += read, + Err(Errno::INTR) => {} + Err(errno) => return Err(io::Error::from(errno).into()), } } Ok(bytes) @@ -196,206 +144,120 @@ pub(super) fn decode_blob( } fn sealed_memfd(bytes: &[u8]) -> Result { - let name = c"wayscriber-broker-payload"; - // SAFETY: name is a valid C string and flags request a private, sealable fd. - let raw = - unsafe { libc::memfd_create(name.as_ptr(), libc::MFD_CLOEXEC | libc::MFD_ALLOW_SEALING) }; - if raw < 0 { - return Err(io::Error::last_os_error()).context("broker memfd creation failed"); - } - // SAFETY: memfd_create returned a new owned descriptor. - let descriptor = unsafe { OwnedFd::from_raw_fd(raw) }; + let descriptor = rustix_fs::memfd_create( + c"wayscriber-broker-payload", + MemfdFlags::CLOEXEC | MemfdFlags::ALLOW_SEALING, + ) + .map_err(io::Error::from) + .context("broker memfd creation failed")?; let mut offset = 0; while offset < bytes.len() { - // SAFETY: the source range is readable for the requested length. - let written = unsafe { - libc::write( - descriptor.as_raw_fd(), - bytes[offset..].as_ptr().cast(), - bytes.len() - offset, - ) - }; - if written > 0 { - offset += written as usize; - } else if written < 0 && io::Error::last_os_error().kind() == io::ErrorKind::Interrupted { - continue; - } else { - return Err(if written == 0 { - anyhow!("short write while creating broker memfd") - } else { - io::Error::last_os_error().into() - }); + match rustix_io::write(&descriptor, &bytes[offset..]) { + Ok(0) => bail!("short write while creating broker memfd"), + Ok(written) => offset += written, + Err(Errno::INTR) => {} + Err(errno) => return Err(io::Error::from(errno).into()), } } - // SAFETY: fcntl changes only seal metadata on the owned memfd. - if unsafe { - libc::fcntl( - descriptor.as_raw_fd(), - libc::F_ADD_SEALS, - REQUIRED_MEMFD_SEALS, - ) - } < 0 - { - return Err(io::Error::last_os_error()).context("failed to seal broker memfd"); - } + rustix_fs::fcntl_add_seals(&descriptor, REQUIRED_MEMFD_SEALS) + .map_err(io::Error::from) + .context("failed to seal broker memfd")?; Ok(descriptor) } fn validate_sealed_memfd(descriptor: &OwnedFd, length: usize) -> Result<()> { - let mut stat = MaybeUninit::::uninit(); - // SAFETY: stat points to writable storage for fstat. - if unsafe { libc::fstat(descriptor.as_raw_fd(), stat.as_mut_ptr()) } != 0 { - return Err(io::Error::last_os_error()).context("failed to inspect broker memfd"); - } - // SAFETY: fstat initialized stat after success. - let stat = unsafe { stat.assume_init() }; - if (stat.st_mode & libc::S_IFMT) != libc::S_IFREG || stat.st_size != length as libc::off_t { + let stat = rustix_fs::fstat(descriptor) + .map_err(io::Error::from) + .context("failed to inspect broker memfd")?; + if FileType::from_raw_mode(stat.st_mode) != FileType::RegularFile + || u64::try_from(stat.st_size).ok() != Some(length as u64) + { bail!("broker memfd shape does not match its declaration"); } - // SAFETY: F_GET_SEALS reads descriptor metadata. - let seals = unsafe { libc::fcntl(descriptor.as_raw_fd(), libc::F_GET_SEALS) }; - if seals < 0 || seals & REQUIRED_MEMFD_SEALS != REQUIRED_MEMFD_SEALS { + let sealed = rustix_fs::fcntl_get_seals(descriptor) + .is_ok_and(|seals| seals.contains(REQUIRED_MEMFD_SEALS)); + if !sealed { bail!("broker memfd is not immutably sealed"); } Ok(()) } -pub(super) fn send_packet(fd: RawFd, packet: &[u8], descriptors: &[RawFd]) -> io::Result<()> { +pub(super) fn send_packet( + fd: BorrowedFd<'_>, + packet: &[u8], + descriptors: &[BorrowedFd<'_>], +) -> io::Result<()> { if descriptors.len() > MAX_PACKET_DESCRIPTORS { return Err(io::Error::new( io::ErrorKind::InvalidInput, "too many broker packet descriptors", )); } + let mut space = [MaybeUninit::uninit(); rustix::cmsg_space!(ScmRights(MAX_PACKET_DESCRIPTORS))]; + let mut control = SendAncillaryBuffer::new(&mut space); + if !descriptors.is_empty() && !control.push(SendAncillaryMessage::ScmRights(descriptors)) { + return Err(io::Error::new( + io::ErrorKind::InvalidInput, + "broker packet descriptors do not fit their control buffer", + )); + } + let payload = [IoSlice::new(packet)]; loop { - let mut iovec = libc::iovec { - iov_base: packet.as_ptr().cast_mut().cast(), - iov_len: packet.len(), - }; - let control_len = if descriptors.is_empty() { - 0 - } else { - // SAFETY: CMSG_SPACE only performs arithmetic on its argument. - unsafe { libc::CMSG_SPACE(mem::size_of_val(descriptors) as libc::c_uint) as usize } - }; - let mut control = vec![0_usize; control_len.div_ceil(mem::size_of::())]; - // SAFETY: zero is a valid initial state for msghdr. - let mut message: libc::msghdr = unsafe { mem::zeroed() }; - message.msg_iov = &raw mut iovec; - message.msg_iovlen = 1; - if !descriptors.is_empty() { - message.msg_control = control.as_mut_ptr().cast(); - message.msg_controllen = control_len; - // SAFETY: the control buffer has CMSG_SPACE bytes. - unsafe { - let header = libc::CMSG_FIRSTHDR(&raw const message); - (*header).cmsg_level = libc::SOL_SOCKET; - (*header).cmsg_type = libc::SCM_RIGHTS; - (*header).cmsg_len = - libc::CMSG_LEN(mem::size_of_val(descriptors) as libc::c_uint) as usize; - ptr::copy_nonoverlapping( - descriptors.as_ptr(), - libc::CMSG_DATA(header).cast::(), - descriptors.len(), - ); + match net::sendmsg(fd, &payload, &mut control, SendFlags::NOSIGNAL) { + Ok(sent) if sent == packet.len() => return Ok(()), + Ok(_) => { + return Err(io::Error::new( + io::ErrorKind::WriteZero, + "short seqpacket send", + )); } + Err(Errno::INTR) => {} + Err(errno) => return Err(errno.into()), } - // SAFETY: message references live packet/control buffers. - let sent = unsafe { libc::sendmsg(fd, &raw const message, libc::MSG_NOSIGNAL) }; - if sent == packet.len() as isize { - return Ok(()); - } - if sent < 0 && io::Error::last_os_error().kind() == io::ErrorKind::Interrupted { - continue; - } - return Err(if sent < 0 { - io::Error::last_os_error() - } else { - io::Error::new(io::ErrorKind::WriteZero, "short seqpacket send") - }); } } -pub(super) fn recv_packet(fd: RawFd) -> io::Result<(Vec, Vec)> { +pub(super) fn recv_packet(fd: BorrowedFd<'_>) -> io::Result<(Vec, Vec)> { let mut buffer = vec![0_u8; MAX_PACKET_BYTES + 1]; - loop { - let mut iovec = libc::iovec { - iov_base: buffer.as_mut_ptr().cast(), - iov_len: buffer.len(), - }; - // SAFETY: CMSG_SPACE only performs arithmetic on its argument. - let control_len = unsafe { - libc::CMSG_SPACE((MAX_PACKET_DESCRIPTORS * mem::size_of::()) as libc::c_uint) - as usize - }; - let mut control = vec![0_usize; control_len.div_ceil(mem::size_of::())]; - // SAFETY: zero is a valid initial state for msghdr. - let mut message: libc::msghdr = unsafe { mem::zeroed() }; - message.msg_iov = &raw mut iovec; - message.msg_iovlen = 1; - message.msg_control = control.as_mut_ptr().cast(); - message.msg_controllen = control_len; - // SAFETY: message references writable packet/control buffers. - let read = unsafe { libc::recvmsg(fd, &raw mut message, libc::MSG_CMSG_CLOEXEC) }; - if read > 0 { - let read = read as usize; - let mut descriptors = Vec::new(); - // SAFETY: recvmsg initialized ancillary headers in the buffer. - unsafe { - let mut header = libc::CMSG_FIRSTHDR(&raw const message); - while !header.is_null() { - if (*header).cmsg_level != libc::SOL_SOCKET - || (*header).cmsg_type != libc::SCM_RIGHTS - { - return Err(io::Error::new( - io::ErrorKind::InvalidData, - "unexpected broker packet ancillary data", - )); - } - let base_len = libc::CMSG_LEN(0) as usize; - if (*header).cmsg_len < base_len { - return Err(io::Error::new( - io::ErrorKind::InvalidData, - "malformed broker descriptor payload", - )); - } - let data_len = (*header).cmsg_len - base_len; - if !data_len.is_multiple_of(mem::size_of::()) { - return Err(io::Error::new( - io::ErrorKind::InvalidData, - "misaligned broker descriptor payload", - )); - } - let count = data_len / mem::size_of::(); - let data = libc::CMSG_DATA(header).cast::(); - for index in 0..count { - descriptors.push(OwnedFd::from_raw_fd(*data.add(index))); - } - header = libc::CMSG_NXTHDR(&raw const message, header); - } - } - if read > MAX_PACKET_BYTES - || message.msg_flags & (libc::MSG_TRUNC | libc::MSG_CTRUNC) != 0 - || descriptors.len() > MAX_PACKET_DESCRIPTORS - { - return Err(io::Error::new( - io::ErrorKind::InvalidData, - "broker packet exceeds its data or descriptor cap", - )); - } - buffer.truncate(read); - return Ok((buffer, descriptors)); + let mut space = [MaybeUninit::uninit(); rustix::cmsg_space!(ScmRights(MAX_PACKET_DESCRIPTORS))]; + let mut control = RecvAncillaryBuffer::new(&mut space); + let received = loop { + let mut payload = [IoSliceMut::new(&mut buffer)]; + match net::recvmsg(fd, &mut payload, &mut control, RecvFlags::CMSG_CLOEXEC) { + Ok(received) => break received, + Err(Errno::INTR) => {} + Err(errno) => return Err(errno.into()), } - if read == 0 { - return Err(io::Error::new( - io::ErrorKind::UnexpectedEof, - "broker socket closed", - )); - } - let error = io::Error::last_os_error(); - if error.kind() == io::ErrorKind::Interrupted { - continue; - } - return Err(error); + }; + if received.bytes == 0 { + return Err(io::Error::new( + io::ErrorKind::UnexpectedEof, + "broker socket closed", + )); } + // Descriptors left in `control` close when it drops, so every rejection + // below releases what the packet carried. + let mut descriptors = Vec::new(); + for message in control.drain() { + let RecvAncillaryMessage::ScmRights(passed) = message else { + return Err(io::Error::new( + io::ErrorKind::InvalidData, + "unexpected broker packet ancillary data", + )); + }; + descriptors.extend(passed); + } + if received.bytes > MAX_PACKET_BYTES + || received + .flags + .intersects(ReturnFlags::TRUNC | ReturnFlags::CTRUNC) + || descriptors.len() > MAX_PACKET_DESCRIPTORS + { + return Err(io::Error::new( + io::ErrorKind::InvalidData, + "broker packet exceeds its data or descriptor cap", + )); + } + buffer.truncate(received.bytes); + Ok((buffer, descriptors)) } diff --git a/src/process_broker/wire.rs b/src/process_broker/wire.rs index a920ac3e..262c0296 100644 --- a/src/process_broker/wire.rs +++ b/src/process_broker/wire.rs @@ -3,6 +3,7 @@ use std::os::fd::OwnedFd; use std::os::unix::ffi::OsStrExt; use anyhow::{Result, bail}; +use rustix::fs::SealFlags; use serde::{Deserialize, Serialize}; pub(super) const BROKER_FD: i32 = 3; @@ -19,8 +20,10 @@ pub(super) const MAX_OUTPUT_BYTES: usize = 256 * 1024 * 1024; pub(super) const MAX_STDERR_BYTES: usize = 64 * 1024; pub(super) const MAX_OWNED_CHILDREN: usize = 64; pub(super) const MAX_PACKET_DESCRIPTORS: usize = 2; -pub(super) const REQUIRED_MEMFD_SEALS: i32 = - libc::F_SEAL_WRITE | libc::F_SEAL_GROW | libc::F_SEAL_SHRINK | libc::F_SEAL_SEAL; +pub(super) const REQUIRED_MEMFD_SEALS: SealFlags = SealFlags::WRITE + .union(SealFlags::GROW) + .union(SealFlags::SHRINK) + .union(SealFlags::SEAL); /// Failure text used when a `Complete`-mode helper writes past its stdout cap. /// diff --git a/src/runtime_ui_state/store/fs.rs b/src/runtime_ui_state/store/fs.rs index ff9749cc..f44fa043 100644 --- a/src/runtime_ui_state/store/fs.rs +++ b/src/runtime_ui_state/store/fs.rs @@ -1,8 +1,7 @@ use std::ffi::{CString, OsStr, OsString}; use std::fs::{self, File, OpenOptions}; use std::io::{self, Write}; -use std::mem::MaybeUninit; -use std::os::fd::{AsRawFd, FromRawFd}; +use std::os::fd::{AsFd, AsRawFd, BorrowedFd}; use std::os::unix::ffi::OsStrExt; use std::os::unix::fs::{MetadataExt, OpenOptionsExt}; use std::path::PathBuf; @@ -10,6 +9,11 @@ use std::process; use std::sync::Arc; use std::sync::atomic::{AtomicU64, Ordering}; +#[cfg(target_os = "linux")] +use rustix::fs::RenameFlags; +use rustix::fs::{self as rustix_fs, AtFlags, Mode, OFlags}; +use rustix::io::Errno; + use crate::runtime_ui_state::{RuntimeStateFileIdentity, RuntimeStateResolvedParent}; static NEXT_SIBLING_ID: AtomicU64 = AtomicU64::new(1); @@ -62,8 +66,8 @@ impl PinnedParent { }) } - fn fd(&self) -> libc::c_int { - self.inner.file.as_raw_fd() + fn fd(&self) -> BorrowedFd<'_> { + self.inner.file.as_fd() } fn sync(&self) -> io::Result<()> { @@ -77,7 +81,7 @@ impl PinnedParent { return Ok(self.inner.observed_path.clone()); } - let proc_path = PathBuf::from(format!("/proc/self/fd/{}", self.fd())); + let proc_path = PathBuf::from(format!("/proc/self/fd/{}", self.fd().as_raw_fd())); let current = fs::read_link(proc_path)?; if !fs::metadata(¤t) .is_ok_and(|metadata| file_identity(&metadata) == self.inner.identity) @@ -100,15 +104,13 @@ pub(super) struct PinnedPath { impl PinnedPath { pub(super) fn open_read(&self) -> io::Result { let leaf = os_str_c_string(&self.leaf)?; - // SAFETY: the parent fd and CString remain valid for the duration of the call. - let fd = unsafe { - libc::openat( - self.parent.fd(), - leaf.as_ptr(), - libc::O_RDONLY | libc::O_CLOEXEC | libc::O_NOFOLLOW | libc::O_NONBLOCK, - ) - }; - file_from_fd(fd) + let fd = rustix_fs::openat( + self.parent.fd(), + &leaf, + OFlags::RDONLY | OFlags::CLOEXEC | OFlags::NOFOLLOW | OFlags::NONBLOCK, + Mode::empty(), + )?; + Ok(File::from(fd)) } pub(super) fn reported_path(&self) -> io::Result { @@ -119,40 +121,23 @@ impl PinnedPath { self.parent.sync() } - fn create_new(&self, mode: libc::mode_t) -> io::Result { + fn create_new(&self, mode: Mode) -> io::Result { let leaf = os_str_c_string(&self.leaf)?; - // SAFETY: the parent fd and CString remain valid for the duration of the call. - let fd = unsafe { - libc::openat( - self.parent.fd(), - leaf.as_ptr(), - libc::O_WRONLY | libc::O_CREAT | libc::O_EXCL | libc::O_CLOEXEC | libc::O_NOFOLLOW, - mode, - ) - }; - file_from_fd(fd) + let fd = rustix_fs::openat( + self.parent.fd(), + &leaf, + OFlags::WRONLY | OFlags::CREATE | OFlags::EXCL | OFlags::CLOEXEC | OFlags::NOFOLLOW, + mode, + )?; + Ok(File::from(fd)) } fn exists_nofollow(&self) -> io::Result { let leaf = os_str_c_string(&self.leaf)?; - let mut stat = MaybeUninit::::uninit(); - // SAFETY: the pointers are valid and stat points to writable storage. - let result = unsafe { - libc::fstatat( - self.parent.fd(), - leaf.as_ptr(), - stat.as_mut_ptr(), - libc::AT_SYMLINK_NOFOLLOW, - ) - }; - if result == 0 { - return Ok(true); - } - let error = io::Error::last_os_error(); - if error.kind() == io::ErrorKind::NotFound { - Ok(false) - } else { - Err(error) + match rustix_fs::statat(self.parent.fd(), &leaf, AtFlags::SYMLINK_NOFOLLOW) { + Ok(_) => Ok(true), + Err(Errno::NOENT) => Ok(false), + Err(errno) => Err(errno.into()), } } } @@ -184,18 +169,20 @@ impl Drop for CleanupPath { } pub(super) fn create_synced_temp(path: &PinnedPath, bytes: &[u8]) -> io::Result { - let path = create_unique_sibling(path, "tmp", |candidate| match candidate.create_new(0o600) { - Ok(mut file) => { - let result = file.write_all(bytes).and_then(|()| file.sync_all()); - drop(file); - if let Err(error) = result { - let _ = remove_file(candidate); - return Err(error); + let path = create_unique_sibling(path, "tmp", |candidate| { + match candidate.create_new(Mode::RUSR | Mode::WUSR) { + Ok(mut file) => { + let result = file.write_all(bytes).and_then(|()| file.sync_all()); + drop(file); + if let Err(error) = result { + let _ = remove_file(candidate); + return Err(error); + } + Ok(true) } - Ok(true) + Err(error) if error.kind() == io::ErrorKind::AlreadyExists => Ok(false), + Err(error) => Err(error), } - Err(error) if error.kind() == io::ErrorKind::AlreadyExists => Ok(false), - Err(error) => Err(error), })?; Ok(CleanupPath::new(path)) } @@ -236,21 +223,14 @@ pub(super) fn rename_noreplace(source: &PinnedPath, destination: &PinnedPath) -> { let source_leaf = os_str_c_string(&source.leaf)?; let destination_leaf = os_str_c_string(&destination.leaf)?; - // SAFETY: both parent fds and CStrings remain valid for the call. - let result = unsafe { - libc::renameat2( - source.parent.fd(), - source_leaf.as_ptr(), - destination.parent.fd(), - destination_leaf.as_ptr(), - libc::RENAME_NOREPLACE, - ) - }; - if result == 0 { - Ok(()) - } else { - Err(io::Error::last_os_error()) - } + rustix_fs::renameat_with( + source.parent.fd(), + &source_leaf, + destination.parent.fd(), + &destination_leaf, + RenameFlags::NOREPLACE, + ) + .map_err(io::Error::from) } #[cfg(not(target_os = "linux"))] { @@ -264,13 +244,7 @@ pub(super) fn rename_noreplace(source: &PinnedPath, destination: &PinnedPath) -> pub(super) fn remove_file(path: &PinnedPath) -> io::Result<()> { let leaf = os_str_c_string(&path.leaf)?; - // SAFETY: the parent fd and CString remain valid for the duration of the call. - let result = unsafe { libc::unlinkat(path.parent.fd(), leaf.as_ptr(), 0) }; - if result == 0 { - Ok(()) - } else { - Err(io::Error::last_os_error()) - } + rustix_fs::unlinkat(path.parent.fd(), &leaf, AtFlags::empty()).map_err(io::Error::from) } fn validate_leaf(leaf: &OsStr) -> io::Result<()> { @@ -288,14 +262,6 @@ fn os_str_c_string(value: &OsStr) -> io::Result { .map_err(|_| io::Error::new(io::ErrorKind::InvalidInput, "path contains a NUL byte")) } -fn file_from_fd(fd: libc::c_int) -> io::Result { - if fd < 0 { - return Err(io::Error::last_os_error()); - } - // SAFETY: openat returned a new owned descriptor on success. - Ok(unsafe { File::from_raw_fd(fd) }) -} - fn file_identity(metadata: &fs::Metadata) -> RuntimeStateFileIdentity { RuntimeStateFileIdentity { device: metadata.dev(), diff --git a/src/runtime_ui_state/store/tests.rs b/src/runtime_ui_state/store/tests.rs index 3bde53fb..785a886a 100644 --- a/src/runtime_ui_state/store/tests.rs +++ b/src/runtime_ui_state/store/tests.rs @@ -1,6 +1,4 @@ -use std::ffi::CString; use std::fs; -use std::os::unix::ffi::OsStrExt; use std::os::unix::fs::{PermissionsExt, symlink}; use std::time::{Duration, Instant}; @@ -257,9 +255,7 @@ fn target_symlink_is_rejected_and_neither_path_is_modified() { fn fifo_is_rejected_without_blocking_for_a_writer() { let temp = test_temp::tempdir().unwrap(); let path = temp.path().join("runtime-ui.toml"); - let c_path = CString::new(path.as_os_str().as_bytes()).unwrap(); - // SAFETY: the CString is NUL-terminated and remains live for the call. - assert_eq!(unsafe { libc::mkfifo(c_path.as_ptr(), 0o600) }, 0); + test_temp::make_fifo(&path); let started = Instant::now(); assert!(RuntimeUiStateStore::new(path).inspect().is_err()); assert!(started.elapsed() < Duration::from_secs(1)); diff --git a/src/session/artifacts/move_file.rs b/src/session/artifacts/move_file.rs index 35d93de2..74bee470 100644 --- a/src/session/artifacts/move_file.rs +++ b/src/session/artifacts/move_file.rs @@ -1,6 +1,8 @@ use std::ffi::CString; use anyhow::{Context, Result, anyhow}; +#[cfg(target_os = "linux")] +use rustix::fs::{self as rustix_fs, RenameFlags}; use std::ffi::{OsStr, OsString}; use std::fs::{self, File}; use std::io; @@ -386,23 +388,14 @@ fn rename_artifacts_with_rollback(artifacts: &[MoveArtifact]) -> Result<()> { pub(crate) fn rename_artifact_no_replace(source: &Path, target: &Path) -> io::Result<()> { let source = path_to_cstring(source)?; let target = path_to_cstring(target)?; - // SAFETY: The C strings are valid, NUL-terminated paths. AT_FDCWD makes both paths - // relative to the current process, matching std::fs::rename path semantics. - let result = unsafe { - libc::syscall( - libc::SYS_renameat2, - libc::AT_FDCWD, - source.as_ptr(), - libc::AT_FDCWD, - target.as_ptr(), - libc::RENAME_NOREPLACE, - ) - }; - if result == 0 { - Ok(()) - } else { - Err(io::Error::last_os_error()) - } + rustix_fs::renameat_with( + rustix_fs::CWD, + &source, + rustix_fs::CWD, + &target, + RenameFlags::NOREPLACE, + ) + .map_err(io::Error::from) } #[cfg(target_os = "linux")] diff --git a/src/session/options/tests.rs b/src/session/options/tests.rs index 9ad7d8f9..4ede8366 100644 --- a/src/session/options/tests.rs +++ b/src/session/options/tests.rs @@ -1,11 +1,7 @@ #[cfg(unix)] -use std::ffi::CString; -#[cfg(unix)] -use std::os::unix::ffi::OsStrExt; -#[cfg(unix)] use std::os::unix::fs::{PermissionsExt, symlink}; use std::path::{Path, PathBuf}; -use std::{fs, io, process}; +use std::{fs, process}; use crate::{test_env, test_temp}; @@ -250,7 +246,7 @@ fn named_file_validation_rejects_missing_directory_shaped_path() { fn named_file_validation_rejects_fifo_target() { let temp = test_temp::tempdir().unwrap(); let path = temp.path().join("session-fifo.wayscriber-session"); - make_fifo(&path); + test_temp::make_fifo(&path); for (label, result) in [ ( @@ -394,17 +390,3 @@ fn can_create_probe(parent: &Path) -> bool { Err(_) => false, } } - -#[cfg(unix)] -fn make_fifo(path: &Path) { - let raw_path = CString::new(path.as_os_str().as_bytes()).expect("fifo path has no NUL bytes"); - // SAFETY: raw_path is a valid, NUL-terminated filesystem path for this process. - let result = unsafe { libc::mkfifo(raw_path.as_ptr(), 0o600) }; - assert_eq!( - result, - 0, - "mkfifo {} failed: {}", - path.display(), - io::Error::last_os_error() - ); -} diff --git a/src/session/snapshot/generation/tests.rs b/src/session/snapshot/generation/tests.rs index 483dfd5d..30e945e1 100644 --- a/src/session/snapshot/generation/tests.rs +++ b/src/session/snapshot/generation/tests.rs @@ -13,7 +13,7 @@ use super::*; use crate::session::snapshot::tests::sample_snapshot; use crate::session::snapshot::{load, save}; use crate::session::{CompressionMode, SessionOptions}; -use crate::test_temp::tempdir; +use crate::test_temp::{self, tempdir}; use std::fs::{self, File}; use std::io::{Seek, Write}; use std::path::Path; @@ -427,7 +427,7 @@ fn save_as_and_gzip_preserve_the_allocated_header_generation() { fn fifo_marker_probe_never_blocks_and_oversized_generations_do_not_raise_the_sequence() { let temp = tempdir().unwrap(); let options = options(temp.path()); - super::super::tests::make_fifo(&options.clear_marker_file_path()); + test_temp::make_fifo(&options.clear_marker_file_path()); assert_eq!( ArtifactSetView::probe(&options) .next_generation(UnreadablePolicy::Refuse) diff --git a/src/session/snapshot/tests.rs b/src/session/snapshot/tests.rs index b75ceed5..46ece0f9 100644 --- a/src/session/snapshot/tests.rs +++ b/src/session/snapshot/tests.rs @@ -22,19 +22,15 @@ use crate::draw::frame::{MAX_COMPOUND_DEPTH, ShapeSnapshot, UndoAction}; use crate::draw::{ArrowStyle, Color, FontDescriptor, Frame, Shape}; use crate::input::EraserMode; use crate::session::options::{CompressionMode, SessionOptions}; -use crate::test_temp::tempdir; +use crate::test_temp::{make_fifo, tempdir}; use crate::time_utils::now_rfc3339; -#[cfg(unix)] -use std::ffi::CString; use std::fs::{File, Permissions}; #[cfg(unix)] -use std::os::unix::ffi::OsStrExt; -#[cfg(unix)] use std::os::unix::fs::symlink; use std::path::{Path, PathBuf}; use std::sync::mpsc; use std::time::{Duration, SystemTime}; -use std::{fs, io, thread}; +use std::{fs, thread}; fn sample_frame() -> Frame { let mut frame = Frame::new(); @@ -281,20 +277,6 @@ fn write_contentless_session(path: &Path) { .expect("contentless session write"); } -#[cfg(unix)] -pub(super) fn make_fifo(path: &Path) { - let raw_path = CString::new(path.as_os_str().as_bytes()).expect("fifo path has no NUL bytes"); - // SAFETY: raw_path is a valid, NUL-terminated filesystem path for this process. - let result = unsafe { libc::mkfifo(raw_path.as_ptr(), 0o600) }; - assert_eq!( - result, - 0, - "mkfifo {} failed: {}", - path.display(), - io::Error::last_os_error() - ); -} - pub(super) fn set_modified(path: &Path, modified: SystemTime) { File::options() .write(true) diff --git a/src/session/storage/tests.rs b/src/session/storage/tests.rs index 6a9e4e80..8f81f7ab 100644 --- a/src/session/storage/tests.rs +++ b/src/session/storage/tests.rs @@ -8,14 +8,9 @@ use crate::session::snapshot::{BoardPagesSnapshot, BoardSnapshot}; use crate::session::{ CompressionMode, SessionOptions, SessionSnapshot, ToolStateSnapshot, save_snapshot, }; -#[cfg(unix)] -use std::ffi::CString; -#[cfg(unix)] -use std::os::unix::ffi::OsStrExt; +use std::fs; #[cfg(unix)] use std::os::unix::fs::symlink; -use std::path::Path; -use std::{fs, io}; fn transparent_line_snapshot() -> SessionSnapshot { let mut frame = Frame::new(); @@ -632,7 +627,7 @@ fn inspect_named_file_rejects_special_lock_without_blocking() { save_snapshot(&snapshot, &options).expect("save_snapshot should succeed"); let lock_path = options.lock_file_path(); fs::remove_file(&lock_path).expect("remove named session lock"); - make_fifo(&lock_path); + test_temp::make_fifo(&lock_path); let err = inspect_session(&options).expect_err("special lock should be rejected"); @@ -722,17 +717,3 @@ fn clear_named_file_rejects_symlink_primary_without_removing_artifacts() { "clear must not remove selected sidecars after primary validation fails" ); } - -#[cfg(unix)] -fn make_fifo(path: &Path) { - let raw_path = CString::new(path.as_os_str().as_bytes()).expect("fifo path has no NUL bytes"); - // SAFETY: raw_path is a valid, NUL-terminated filesystem path for this process. - let result = unsafe { libc::mkfifo(raw_path.as_ptr(), 0o600) }; - assert_eq!( - result, - 0, - "mkfifo {} failed: {}", - path.display(), - io::Error::last_os_error() - ); -} diff --git a/src/test_fake_helper.rs b/src/test_fake_helper.rs index 6eac2008..61f7c441 100644 --- a/src/test_fake_helper.rs +++ b/src/test_fake_helper.rs @@ -15,6 +15,8 @@ use std::os::unix::fs as unix_fs; use std::path::{Path, PathBuf}; use std::{env, fs, io, process}; +use rustix::process as rustix_process; + use anyhow::{Context, Result}; use crate::durable_io; @@ -124,8 +126,7 @@ fn play(arguments: &[OsString]) -> Result<()> { let report: Vec = match role { Role::Exit => return Ok(()), Role::ReportProcessGroup => { - // SAFETY: getpgrp has no preconditions and cannot fail. - let leader = unsafe { libc::getpgrp() } == process::id() as libc::pid_t; + let leader = rustix_process::getpgrp().as_raw_pid() == process::id() as libc::pid_t; if leader { "process-group-leader" } else { diff --git a/src/test_temp.rs b/src/test_temp.rs index 15be825c..6ca085ce 100644 --- a/src/test_temp.rs +++ b/src/test_temp.rs @@ -1 +1,19 @@ +#[cfg(unix)] +use std::path::Path; + +#[cfg(unix)] +use rustix::fs::{self as rustix_fs, FileType, Mode}; pub(crate) use tempfile::{TempDir, tempdir}; + +/// Creates a FIFO that only the current user can open. +#[cfg(unix)] +pub(crate) fn make_fifo(path: &Path) { + rustix_fs::mknodat( + rustix_fs::CWD, + path, + FileType::Fifo, + Mode::RUSR | Mode::WUSR, + 0, + ) + .unwrap_or_else(|errno| panic!("mkfifo {} failed: {errno}", path.display())); +} diff --git a/src/toolbar_gtk/bridge.rs b/src/toolbar_gtk/bridge.rs index abe214f9..5e40a9b9 100644 --- a/src/toolbar_gtk/bridge.rs +++ b/src/toolbar_gtk/bridge.rs @@ -527,7 +527,8 @@ mod tests { use std::fs; use glib::MainContext; - use std::os::fd::AsRawFd; + use rustix::process::Pid; + use rustix::thread as rustix_thread; use std::sync::mpsc; use super::*; @@ -570,18 +571,10 @@ mod tests { } fn wake_is_readable(source: &RuntimeWakeSource) -> bool { - let mut pollfd = libc::pollfd { - fd: source.poll_fd().as_raw_fd(), - events: libc::POLLIN, - revents: 0, - }; - // SAFETY: pollfd and the source descriptor are valid for this non-blocking poll. - let ready = unsafe { libc::poll(&raw mut pollfd, 1, 0) }; - assert!(ready >= 0); - ready > 0 && pollfd.revents & libc::POLLIN != 0 + source.is_readable_within(Duration::ZERO).unwrap() } - fn wait_until_thread_is_blocked_in_poll(tid: libc::pid_t) { + fn wait_until_thread_is_blocked_in_poll(tid: Pid) { let stat_path = format!("/proc/self/task/{tid}/stat"); let deadline = Instant::now() + Duration::from_secs(1); loop { @@ -814,18 +807,8 @@ mod tests { let (wake, _health, publisher) = channel(); let (tid_tx, tid_rx) = mpsc::channel(); let poller = thread::spawn(move || { - // SAFETY: gettid has no preconditions and is used only to observe this test thread. - let tid = unsafe { libc::syscall(libc::SYS_gettid) as libc::pid_t }; - tid_tx.send(tid).unwrap(); - let mut pollfd = libc::pollfd { - fd: wake.poll_fd().as_raw_fd(), - events: libc::POLLIN, - revents: 0, - }; - // SAFETY: pollfd and the source descriptor remain valid for the bounded wait. - let ready = unsafe { libc::poll(&raw mut pollfd, 1, 1_000) }; - assert_eq!(ready, 1); - assert_ne!(pollfd.revents & libc::POLLIN, 0); + tid_tx.send(rustix_thread::gettid()).unwrap(); + assert!(wake.is_readable_within(Duration::from_secs(1)).unwrap()); }); let tid = tid_rx.recv().unwrap(); diff --git a/src/toolbar_gtk/runtime.rs b/src/toolbar_gtk/runtime.rs index 05bea772..8b703b6b 100644 --- a/src/toolbar_gtk/runtime.rs +++ b/src/toolbar_gtk/runtime.rs @@ -125,8 +125,8 @@ pub(super) fn run( #[cfg(test)] mod tests { use std::cell::Cell; - use std::os::fd::AsRawFd; use std::rc::Rc; + use std::time::Duration; use super::*; use crate::backend::wayland::RuntimeWakeSource; @@ -140,16 +140,8 @@ mod tests { clean: Cell::new(false), }); - let mut pollfd = libc::pollfd { - fd: wake.poll_fd().as_raw_fd(), - events: libc::POLLIN, - revents: 0, - }; - // SAFETY: pollfd and the source descriptor are valid for this non-blocking poll. - let ready = unsafe { libc::poll(&raw mut pollfd, 1, 0) }; - assert_eq!(ready, 1); + assert!(wake.is_readable_within(Duration::ZERO).unwrap()); assert!(health.failed()); - assert_ne!(pollfd.revents & libc::POLLIN, 0); } #[test] @@ -162,13 +154,7 @@ mod tests { }); assert!(!health.failed()); - let mut pollfd = libc::pollfd { - fd: wake.poll_fd().as_raw_fd(), - events: libc::POLLIN, - revents: 0, - }; - // SAFETY: pollfd and the source descriptor are valid for this non-blocking poll. - assert_eq!(unsafe { libc::poll(&raw mut pollfd, 1, 0) }, 0); + assert!(!wake.is_readable_within(Duration::ZERO).unwrap()); } #[test] diff --git a/src/unix_signals/listener.rs b/src/unix_signals/listener.rs index a0328dd4..919e0004 100644 --- a/src/unix_signals/listener.rs +++ b/src/unix_signals/listener.rs @@ -7,6 +7,9 @@ use std::sync::{Arc, Mutex}; use std::thread::{self, JoinHandle}; use std::{fmt, io}; +#[cfg(test)] +use rustix::thread as rustix_thread; + use super::{ SIGNAL_WRITE_FD, begin_listener_teardown, clear_registered_signals, create_pipe, dispatch_pending_signals, finish_listener_teardown, install_handler, publish_listener_active, @@ -259,9 +262,9 @@ impl Drop for SignalListener { fn listener_thread(read_fd: RawFd, shared: &ListenerShared) { #[cfg(test)] { - // SAFETY: gettid has no preconditions and is used only by tests to - // observe that this exact thread reached its blocking read. - let tid = unsafe { libc::syscall(libc::SYS_gettid) as libc::pid_t }; + // Used only by tests to observe that this exact thread reached its + // blocking read. + let tid = rustix_thread::gettid().as_raw_pid(); shared.thread_tid.store(tid, Ordering::Release); } diff --git a/tests/cli.rs b/tests/cli.rs index 00838a2e..f47a35a5 100644 --- a/tests/cli.rs +++ b/tests/cli.rs @@ -343,6 +343,19 @@ fn clustered_help_and_version_exit_zero_without_runtime_startup() { .stdout_starts_with("wayscriber "); } +#[cfg(unix)] +#[test] +fn internal_broker_entry_rejects_descriptors_that_are_not_broker_sockets() { + run_command( + wayscriber_cmd() + .env("WAYSCRIBER_INTERNAL_PROCESS_BROKER_FD", "3") + .env("WAYSCRIBER_INTERNAL_PROCESS_BROKER_SHUTDOWN_FD", "4") + .env("WAYSCRIBER_INTERNAL_PROCESS_BROKER_TOKEN", "a".repeat(64)), + ) + .failure() + .code(126); +} + #[test] fn invalid_arguments_exit_two() { for (args, diagnostic) in [ diff --git a/tools/csharp/Commands/ChecksCommand.cs b/tools/csharp/Commands/ChecksCommand.cs index 2c09fd54..e979081f 100644 --- a/tools/csharp/Commands/ChecksCommand.cs +++ b/tools/csharp/Commands/ChecksCommand.cs @@ -334,11 +334,13 @@ internal static class ChecksCommand [RepositoryNames.Gtk4LayerShell] = [RepositoryNames.Gtk4LayerShell], ["input"] = ["libinput"], ["ksni"] = [], + ["lexopt"] = [], ["libc"] = [], ["log"] = [], ["pango"] = ["pango"], ["pangocairo"] = ["pango", "cairo"], ["png"] = [], + ["rustix"] = [], ["schemars"] = [], ["serde"] = [], ["serde_ignored"] = [],