logind: fix (again) the race that might happen when logind restores VT

This patch is a new attempt to fix the race originally described in issue #9754.

The initial fix (commit ad96887a1205bad9656d280c5681f482e6d04838) consisted in
spawning a sub process that became the controlling process of the VT and hence
kicked the old controlling process off to make sure that the VT wouldn't have
entered in HUP state while logind restored the VT.

But it introduced a regression (see issue #11269) and thus was reverted. But
unlike it was described in the revert commit message, commit
adb8688b3ff445d9c48ed0d72208c7844c2acc01 alone doen't fix the initial race.

This patch fixes the race in a simpler way by trying to restore the VT a second
time after making sure to re-open it if the first attempt fails.

Indeed if the old controlling process dies before or during the first attempt,
logind will fail to restore the VT. At this point the VT is in HUP state but
we're sure that it won't enter in a HUP state a second time. Therefore we will
retry by re-opening the VT to clear the HUP state and by restoring the VT a
second time, which should be safe this time.

Fixes: #9754
Fixes: #13241

diff --git a/src/login/logind-session.c b/src/login/logind-session.c
index 9e8f04d3eaa431114929e8cdc284e3535cac1aaf..1ef73570e58b52bf35d4ae968dd62e129dd736c2 100644 (file)
--- a/src/login/logind-session.c
+++ b/src/login/logind-session.c
@@ -1239,23 +1239,27 @@ error:
 }
 
 static void session_restore_vt(Session *s) {
-        int r, vt, old_fd;
+        int r;
 
-        /* We need to get a fresh handle to the virtual terminal,
-         * since the old file-descriptor is potentially in a hung-up
-         * state after the controlling process exited; we do a
-         * little dance to avoid having the terminal be available
-         * for reuse before we've cleaned it up.
-         */
-        old_fd = TAKE_FD(s->vtfd);
+        r = vt_restore(s->vtfd);
+        if (r == -EIO) {
+                int vt, old_fd;
 
-        vt = session_open_vt(s);
-        safe_close(old_fd);
+                /* It might happen if the controlling process exited before or while we were
+                 * restoring the VT as it would leave the old file-descriptor in a hung-up
+                 * state. In this case let's retry with a fresh handle to the virtual terminal. */
 
-        if (vt < 0)
-                return;
+                /* We do a little dance to avoid having the terminal be available
+                 * for reuse before we've cleaned it up. */
+                old_fd = TAKE_FD(s->vtfd);
+
+                vt = session_open_vt(s);
+                safe_close(old_fd);
+
+                if (vt >= 0)
+                        r = vt_restore(vt);
+        }
 
-        r = vt_restore(vt);
         if (r < 0)
                 log_warning_errno(r, "Failed to restore VT, ignoring: %m");