ptyfwd: avoid touching forwarder after exit drain

author Luca Boccassi <luca.boccassi@gmail.com>

Mon, 29 Jun 2026 17:07:01 +0000 (18:07 +0100)

committer Luca Boccassi <luca.boccassi@gmail.com>

Tue, 30 Jun 2026 12:27:57 +0000 (13:27 +0100)
author Luca Boccassi <luca.boccassi@gmail.com>
Mon, 29 Jun 2026 17:07:01 +0000 (18:07 +0100)
committer Luca Boccassi <luca.boccassi@gmail.com>
Tue, 30 Jun 2026 12:27:57 +0000 (13:27 +0100)
diff --git a/src/shared/ptyfwd.c b/src/shared/ptyfwd.c

index fcc1500dc5c8bd91a8c918cf5b4f8a1d2d591aa0..5445da2a1b6990c2fa5e339106a7e64bfc220f1a 100644 (file)
--- a/src/shared/ptyfwd.c
+++ b/src/shared/ptyfwd.c
@@ -889,19 +889,16 @@ static int on_sigwinch_event(sd_event_source *e, const struct signalfd_siginfo *
  
  static int on_exit_event(sd_event_source *e, void *userdata) {
          PTYForward *f = ASSERT_PTR(userdata);
-        int r;
  
          assert(e);
          assert(e == f->exit_event_source);
  
-        if (!pty_forward_drain(f)) {
-                /* If not drained, try to drain the buffer. */
-                r = shovel_force(f);
-                if (r < 0)
-                        return r;
-        }
-
-        return pty_forward_done(f, 0);
+        /* The event loop is exiting while the forwarder is still active. Force a final synchronous drain of
+         * whatever is still buffered. shovel() may complete the drain and call pty_forward_done(), which
+         * can free f via the hangup handler, so we must not touch f afterwards, only propagate the return
+         * value, exactly like on_master_event() does. */
+        f->drain = true;
+        return shovel_force(f);
  }
  
  static int on_defer_event(sd_event_source *s, void *userdata) {
diff --git a/test/units/TEST-74-AUX-UTILS.run.sh b/test/units/TEST-74-AUX-UTILS.run.sh

index 2f120638b5037da7e56dfc10379cc8ce4ccedd0c..bdb0fd1a7eb061b39c9e2ffe1700fff01e1cc05b 100755 (executable)
--- a/test/units/TEST-74-AUX-UTILS.run.sh
+++ b/test/units/TEST-74-AUX-UTILS.run.sh
@@ -314,3 +314,19 @@ fi
  
  # Tests whether intermediate disconnects corrupt us (modified testcase from https://github.com/systemd/systemd/issues/27204)
  assert_rc "37" timeout 300 systemd-run --unit=disconnecttest --wait --pipe --user -M testuser@.host bash -ec 'systemctl --user daemon-reexec; sleep 3; exit 37'
+
+# Trigger on_exit_event(): SIGINT makes systemd-run leave the event loop
+# while PTY forwarding is active. coproc gives us a pollable stdout and exec
+# makes $PTY_FWD_PID point at systemd-run
+coproc PTY_FWD { exec systemd-run --quiet --pty -- bash -c 'echo PTY_FORWARD_READY; exec sleep 60'; }
+PTY_FWD_PID_SAVED="$PTY_FWD_PID"
+read -r -t 30 -u "${PTY_FWD[0]}" PTY_FWD_LINE || true
+[[ "$PTY_FWD_LINE" == *PTY_FORWARD_READY* ]]
+kill -INT "$PTY_FWD_PID_SAVED"
+PTY_FWD_RC=0
+wait "$PTY_FWD_PID_SAVED" || PTY_FWD_RC=$?
+# ASan reports the UAF as systemd-run exiting non-zero.
+if [[ "$PTY_FWD_RC" != 0 ]]; then
+    echo "systemd-run --pty failed"
+    exit 1
+fi
author	Luca Boccassi <luca.boccassi@gmail.com>
	Mon, 29 Jun 2026 17:07:01 +0000 (18:07 +0100)
committer	Luca Boccassi <luca.boccassi@gmail.com>
	Tue, 30 Jun 2026 12:27:57 +0000 (13:27 +0100)
src/shared/ptyfwd.c		patch \| blob \| blame \| history
test/units/TEST-74-AUX-UTILS.run.sh		patch \| blob \| blame \| history