From 27652ad4cff363d4035e93a3a27343024e526978 Mon Sep 17 00:00:00 2001 From: rustdesk Date: Thu, 27 Aug 2026 12:34:56 +0800 Subject: [PATCH] fix(linux): the newest-process walk must not answer with a grep or an older PID Three findings from review of the commit before this one. `/proc//environ` failing to read left the walk on to the next PID, which in `newest_first` mode is an older process -- possibly of a session that has since logged out -- where the `ps ... | tail -1` pipeline this replaces stopped at the one PID it had already picked. A read that fails is a process carrying none of the requested names, not a process to skip. The `seen` latch that was meant to hold the newest process is deleted: `accept` is reached once per matching process, so returning on the first is what it already did. The regex is matched against the whole `/proc//cmdline`, where the pipeline had a `grep -v 'grep'`. A user running `grep Xwayland` is otherwise the newest match for that pattern and answers with whatever environment their shell had -- an X forwarding endpoint over ssh, say. This is the one place the walk still differs from the `get_envs` it grew out of, which never had that filter and could take an ssh `grep` over the portal it was looking for. `get_envs` is left exactly as it was. Its completeness test was every requested name *present*; stating it through `accept` turned it into every name *non-empty* and, with the empty-value change that went with it, moved which process the existing `get_display_xauth_wayland` caller settles on. `accept` is now told the count and asks the question the loop it replaced asked. This supersedes the `get_envs` bullet of the previous commit message: an exported-but-empty value counts as found again, as it always did. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_019QgsYAUYKDei1AM5yHJsMX --- src/platform/linux.rs | 41 +++++++++++++++++------------------------ 1 file changed, 17 insertions(+), 24 deletions(-) diff --git a/src/platform/linux.rs b/src/platform/linux.rs index 430100c66..099d00a2d 100644 --- a/src/platform/linux.rs +++ b/src/platform/linux.rs @@ -1571,9 +1571,7 @@ fn get_envs<'a>( process_pat: &str, names: &[&'a str], ) -> std::collections::HashMap<&'a str, String> { - get_envs_where(uid, process_pat, names, false, |found| { - found.values().all(|value| !value.is_empty()) - }) + get_envs_where(uid, process_pat, names, false, |count| count == names.len()) } /// The newest process matching `process_pat`, whatever it happens to carry: the semantics of the @@ -1585,17 +1583,12 @@ fn get_envs_of_newest<'a>( process_pat: &str, names: &[&'a str], ) -> std::collections::HashMap<&'a str, String> { - let mut seen = false; - get_envs_where(uid, process_pat, names, true, |_| { - !std::mem::replace(&mut seen, true) - }) + get_envs_where(uid, process_pat, names, true, |_| true) } -/// `get_envs` with the caller's own notion of a complete answer: the newest process `accept` -/// takes wins outright, and the count-based ranking is only the fallback when none is accepted. -/// Ranking by how many of `names` a process carries cannot know that some of them are mandatory -/// and others interchangeable, so it can rank a process holding three optional values above the -/// one holding the mandatory pair. +/// `get_envs` with the caller's own process order and its own notion of a complete answer, told +/// how many of `names` the process carries: the first process `accept` takes wins outright, and +/// the count-based ranking is only the fallback for when no process is accepted at all. fn get_envs_where<'a, F>( uid: &str, process_pat: &str, @@ -1604,7 +1597,7 @@ fn get_envs_where<'a, F>( mut accept: F, ) -> std::collections::HashMap<&'a str, String> where - F: FnMut(&std::collections::HashMap<&'a str, String>) -> bool, + F: FnMut(usize) -> bool, { // The tie-breaking logic uses a u64 bitmask, limiting us to 64 variables. debug_assert!( @@ -1667,15 +1660,18 @@ where continue; }; let cmdline_str = String::from_utf8_lossy(&cmdline).replace('\0', " "); - if !re.is_match(&cmdline_str) { + // The `grep -v 'grep'` of the pipeline this replaces. A user grepping for one of these + // patterns is otherwise the newest match for it, and answers with whatever environment + // their shell had -- an X forwarding endpoint over ssh, say. + if cmdline_str.contains("grep") || !re.is_match(&cmdline_str) { continue; } - // Read environ and extract matching variables - let environ_path = proc_path.join("environ"); - let Ok(environ) = std::fs::read(&environ_path) else { - continue; - }; + // Read environ and extract matching variables. A read that fails -- the process exited + // between these two reads -- is a process carrying none of `names`, not a process to + // skip: skipping it would hand `newest_first` on to an older PID, where the pipeline + // this replaces stopped at the single PID its `tail -1` had already picked. + let environ = std::fs::read(proc_path.join("environ")).unwrap_or_default(); let mut found = empty.clone(); let mut found_count = 0usize; @@ -1695,10 +1691,7 @@ where continue; }; if let Some(slot) = found.get_mut(key) { - // An exported-but-empty value (`DISPLAY=`) is not an answer: counting it would - // score this process as a match and, on a single-name query, return the empty - // value before a process that has a real one is ever examined. - if slot.is_empty() && !val_bytes.is_empty() { + if slot.is_empty() { *slot = String::from_utf8_lossy(val_bytes).into_owned(); found_count += 1; @@ -1713,7 +1706,7 @@ where } } - if accept(&found) { + if accept(found_count) { return found; }