sassycrown/punktfunk
1
0
Fork 0
forked from unom/punktfunk
Code Pull Requests Activity

fix(pf-capture): the gamescope cursor fallback rewrote environ under a live multithreaded host

Browse Source 
`connect_via_env_swap` did set_var("XAUTHORITY", …) / connect / restore, guarded
by a mutex that serialised this source against itself and against nothing else.
`getenv` takes no lock. setenv/unsetenv rewrite the process-global `environ`, and
glibc REALLOCATES that array when a variable is added — while, at that exact
moment, the PipeWire thread is inside pw_init()'s dlopen making bare getenv()
calls and EGL/CUDA init is running alongside. The file's own doc already called
the pattern "unsound from a live multithreaded host"; it stayed as a fallback.

Three things made it worse than the comment suggested:

- The damaging branch is the one where XAUTHORITY is ABSENT and therefore gets
  ADDED (the realloc case). scripts/punktfunk-host.service deliberately does not
  import the login shell's environment, so absent is the DOCUMENTED NORMAL
  configuration for the shipped unit, not an edge case.
- `rediscover` re-runs this every 2 s for the whole session. A display whose
  connect fails is never pushed into `displays`, so the dead-display skip never
  covers it — the race is not once at startup, it repeats forever.
- It is unfixable in place. Sharing pf_vdisplay's ENV_LOCK is the wrong layer: it
  cannot make C `getenv` take a lock.

The fix is to stop writing `environ` at all. Connecting with an explicitly empty
auth token is what the swap actually achieved: we only reach the fallback when
our own lookup found no usable MIT-MAGIC-COOKIE-1 entry, and x11rb's internal
lookup reads the same file with a STRICTER matcher (it matches family/address
too, which we deliberately do not), so where we find nothing it finds nothing
either and connects unauthenticated. That is exactly why the swap "worked"
against a nested Xwayland started without -auth.

Gives up one case: an .Xauthority using an auth family we decline to guess at but
x11rb would have handled. A gamescope Xwayland writes a single-entry
MIT-MAGIC-COOKIE-1 file, so it is not reachable here, and declining to attach a
cursor overlay beats tearing `environ` out from under a live session.

Also removes XAUTH_LOCK, whose only user this was.

Verified on 192.168.1.25 (Ubuntu, cargo 1.96.0 — the pinned toolchain, pipewire
dev headers present): `cargo check -p pf-capture --locked` and
`cargo clippy -p pf-capture --all-targets --locked -- -D warnings` both clean.
Not verified on glass: the fallback is only reached when the cookie parse fails,
so a normal gamescope session does not enter it. Forcing it needs a nested
Xwayland started without -auth, or a mangled cookie file, on .181/.136.
This commit is contained in:
enricobuehler
2026-08-11 13:54:56 +02:00
parent df6f270e7b
commit 972af2992f
1 changed files with 38 additions and 35 deletions
Download Patch File Download Diff File Expand all files Collapse all files
crates/pf-capture/src/linux/xfixes_cursor.rs
+38 -35
View File
@@ -55,16 +55,6 @@ use x11rb::rust_connection::{DefaultStream, RustConnection};
use crate::GamescopeCursorTargets;
/// Serializes the `XAUTHORITY` env swap of the LEGACY connect fallback (the var is process-global).
///
/// The fallback is a last resort now — see [`connect_conn`]. It serialises this source against
/// itself and nothing else: `getenv` needs no lock to be racy, so every OTHER thread's read (libspa
/// plugin load, EGL/CUDA init — concurrent by construction, since `attach_gamescope_cursor` runs
/// while the PipeWire thread is starting) could still observe the swapped value or a torn
/// environ. That is why the primary path parses the cookie itself and never touches the
/// environment.
static XAUTH_LOCK: Mutex<()> = Mutex::new(());
/// The `MIT-MAGIC-COOKIE-1` auth-protocol name, as it appears in an `.Xauthority` entry.
const MIT_MAGIC_COOKIE_1: &[u8] = b"MIT-MAGIC-COOKIE-1";
@@ -267,17 +257,18 @@ fn connect(dpy: &str, xauthority: Option<&str>) -> Result<Connected, String> {
/// environment.
///
/// `RustConnection::connect` reads `XAUTHORITY` from the env, so the original implementation
/// `set_var`'d it around each connect under [`XAUTH_LOCK`]. That is unsound from a live
/// multithreaded host: the lock serialises this source against itself, but `getenv` takes no lock,
/// so any concurrent reader (libspa's plugin load, EGL/CUDA init — running at exactly this moment,
/// since the PipeWire thread is starting up) could read the swapped value or race the environ
/// rewrite outright. The project already has a process-wide env-lock discipline elsewhere, but
/// sharing it would be the wrong layer AND would still not fix `getenv`.
/// `set_var`'d it around each connect under a mutex. That is unsound from a live multithreaded
/// host: the lock serialised this source against itself, but `getenv` takes no lock, so any
/// concurrent reader (libspa's plugin load, EGL/CUDA init — running at exactly this moment, since
/// the PipeWire thread is starting up) could read the swapped value or race the environ rewrite
/// outright. The project already has a process-wide env-lock discipline elsewhere, but sharing it
/// would be the wrong layer AND would still not fix `getenv`.
///
/// So: parse the MIT-MAGIC-COOKIE-1 entry out of the file ourselves and hand it to
/// `connect_to_stream_with_auth_info`, which is what `RustConnection::connect` does internally with
/// the cookie IT found. The env swap survives only as a fallback for a file we cannot parse (an
/// unexpected layout, or an auth family whose entry we decline to guess at).
/// the cookie IT found. Where that finds nothing usable we connect with an explicitly empty token
/// ([`connect_unauthenticated`]) rather than swapping the environment — this process no longer
/// writes `environ` at all.
fn connect_conn(dpy: &str, xauthority: Option<&str>) -> Result<(RustConnection, usize), String> {
let Some(path) = xauthority else {
// No per-display cookie file to inject: the ambient environment is already what this
@@ -289,16 +280,16 @@ fn connect_conn(dpy: &str, xauthority: Option<&str>) -> Result<(RustConnection,
Ok(v) => return Ok(v),
Err(e) => tracing::debug!(
dpy = %dpy, xauthority = %path, error = %e,
"gamescope cursor: cookie connect failed — falling back to the XAUTHORITY env swap"
"gamescope cursor: cookie connect failed — retrying unauthenticated"
),
},
None => tracing::debug!(
dpy = %dpy, xauthority = %path,
"gamescope cursor: no MIT-MAGIC-COOKIE-1 entry for this display — falling back to the \
XAUTHORITY env swap"
"gamescope cursor: no MIT-MAGIC-COOKIE-1 entry for this display — connecting \
unauthenticated"
),
}
connect_via_env_swap(dpy, path)
connect_unauthenticated(dpy)
}
/// Connect to `dpy` and complete the setup handshake with an explicit cookie — the same two steps
@@ -331,19 +322,31 @@ fn connect_with_cookie(
.map_err(|e| format!("setup: {e}"))
}
/// LEGACY fallback (see [`connect_conn`]): swap `XAUTHORITY`, connect, restore. Serialised against
/// this source's own concurrent connects, but NOT against other threads' `getenv` — which is why it
/// is a fallback and not the path taken.
fn connect_via_env_swap(dpy: &str, xauthority: &str) -> Result<(RustConnection, usize), String> {
let _g = XAUTH_LOCK.lock().unwrap_or_else(|e| e.into_inner());
let prev = std::env::var_os("XAUTHORITY");
std::env::set_var("XAUTHORITY", xauthority);
let out = RustConnection::connect(Some(dpy));
match prev {
Some(p) => std::env::set_var("XAUTHORITY", p),
None => std::env::remove_var("XAUTHORITY"),
}
out.map_err(|e| format!("connect: {e}"))
/// Last-resort fallback (see [`connect_conn`]): connect with an EXPLICITLY EMPTY auth token.
///
/// This replaces a `set_var("XAUTHORITY", …)` / connect / restore dance, which was unsound and is
/// not fixable in place. `setenv`/`unsetenv` rewrite the process-global `environ`; glibc
/// *reallocates* that array when a variable is added, and the host is emphatically multithreaded
/// at this moment — `attach_gamescope_cursor` runs while the PipeWire thread is inside `pw_init`'s
/// `dlopen` and a dozen bare `getenv()` calls, with EGL/CUDA init alongside. A mutex here
/// serialised this source against itself and against nothing else, because `getenv` takes no lock.
/// The damaging branch is the one where `XAUTHORITY` is ABSENT and therefore gets *added* — which
/// `scripts/punktfunk-host.service` makes the normal configuration, since the unit deliberately
/// does not import the login shell's environment. And `rediscover` re-runs this every 2 s for the
/// whole session, because a display whose connect fails is never recorded and so is never skipped.
///
/// Connecting with an empty token is what the swap actually achieved. We only reach here when our
/// own lookup found no usable `MIT-MAGIC-COOKIE-1` entry, and x11rb's internal lookup reads the
/// same file with a STRICTER matcher (it also matches family/address, which we deliberately do
/// not) — so where we find nothing, it finds nothing too, and connects unauthenticated. That is
/// precisely why the swap "worked" against a nested Xwayland started without `-auth`.
///
/// The one case this gives up is an `.Xauthority` whose entry uses an auth family we decline to
/// guess at but x11rb would have handled. A gamescope Xwayland writes a single-entry
/// MIT-MAGIC-COOKIE-1 file, so that case is not reachable here — and a cursor overlay that
/// declines to attach is the correct outcome anyway, against a torn `environ` in a live session.
fn connect_unauthenticated(dpy: &str) -> Result<(RustConnection, usize), String> {
connect_with_cookie(dpy, Vec::new(), Vec::new())
}
/// The `MIT-MAGIC-COOKIE-1` `(name, data)` for `dpy` from the `.Xauthority`-format file at `path`.