[pbs-devel] [PATCH pve-xtermjs] Avoid allocating in `pre_exec` closure

From: Yuki Sireneva <yuki.utk8g@gmail.com>
To: pbs-devel@lists.proxmox.com
Cc: Yuki Sireneva <yuki.utk8g@gmail.com>
Subject: [pbs-devel] [PATCH pve-xtermjs] Avoid allocating in `pre_exec` closure
Date: Fri,  5 Dec 2025 12:47:50 +0000	[thread overview]
Message-ID: <20251205124802.334198-1-yuki.utk8g@gmail.com> (raw)

`Error::new` allocates memory (see [148971]). This is bad in
multi-threaded programs, which pve-xtermjs AFAIK is. If the fork occurs
while the allocator lock is held by another thread, deadlocks can occur,
since there's no one left in the new process to unlock the mutex.
I do not believe this is UB, and modern libc offer protections against
this issue, but this isn't POSIX-compliant and should preferably be
avoided.

`nix` provides a non-allocating `impl From<Errno> for std::io::Error`,
which can be used instead. This ensures that the correct error code is
forwarded to the parent process, instead of the default `-EINVAL`.

[rust#148971]: https://github.com/rust-lang/rust/pull/148971

Signed-off-by: Yuki Sireneva <yuki.utk8g@gmail.com>
---
 termproxy/src/main.rs | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/termproxy/src/main.rs b/termproxy/src/main.rs
index 135468d..31f5a41 100644
--- a/termproxy/src/main.rs
+++ b/termproxy/src/main.rs
@@ -287,7 +287,7 @@ fn run_pty<'a>(mut full_cmd: impl Iterator<Item = &'a OsString>) -> Result<Pty>
 
     unsafe {
         command.pre_exec(move || {
-            make_controlling_terminal(&secondary_name).map_err(Error::other)?;
+            make_controlling_terminal(&secondary_name)?;
             Ok(())
         });
     }
-- 
2.51.2


_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel

