From 8246905af07f741a739c4add8da2c4ade0f20fcf Mon Sep 17 00:00:00 2001 From: Franck Bui Date: Fri, 18 Oct 2019 12:44:51 +0200 Subject: [PATCH] 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 --- src/login/logind-session.c | 30 +++++++++++++++++------------- 1 file changed, 17 insertions(+), 13 deletions(-) diff --git a/src/login/logind-session.c b/src/login/logind-session.c index 9e8f04d3ea..1ef73570e5 100644 --- 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");