From: Lennart Poettering <lennart@poettering.net>
Date: Wed, 20 Mar 2019 20:28:02 +0000 (+0100)
Subject: core: rework how we reset the TTY after use by a service
X-Git-Tag: v242-rc1~96^2~1
X-Git-Url: http://git.ipfire.org/?p=thirdparty%2Fsystemd.git;a=commitdiff_plain;h=6f765baf23d0d4ed7efa6e2db2a629fd519f3a7a

core: rework how we reset the TTY after use by a service

This makes two changes:

1. Instead of resetting the configured service TTY each time after a
   process exited, let's do so only when the service goes back to "dead"
   state. This should be preferable in case the started processes leave
   background child processes around that still reference the TTY.

2. chmod() and chown() the TTY at the same time. This should make it
   safe to run "systemd-run -p DynamicUser=1 -p StandardInput=tty -p
   TTYPath=/dev/tty8 /bin/bash" without leaving a TTY owned by a dynamic
   user around.
---

diff --git a/src/core/execute.c b/src/core/execute.c
index 5511c1aac5b..dabb6d824fb 100644
--- a/src/core/execute.c
+++ b/src/core/execute.c
@@ -4640,6 +4640,30 @@ void exec_context_free_log_extra_fields(ExecContext *c) {
         c->n_log_extra_fields = 0;
 }
 
+void exec_context_revert_tty(ExecContext *c) {
+        int r;
+
+        assert(c);
+
+        /* First, reset the TTY (possibly kicking everybody else from the TTY) */
+        exec_context_tty_reset(c, NULL);
+
+        /* And then undo what chown_terminal() did earlier. Note that we only do this if we have a path
+         * configured. If the TTY was passed to us as file descriptor we assume the TTY is opened and managed
+         * by whoever passed it to us and thus knows better when and how to chmod()/chown() it back. */
+
+        if (exec_context_may_touch_tty(c)) {
+                const char *path;
+
+                path = exec_context_tty_path(c);
+                if (path) {
+                        r = chmod_and_chown(path, TTY_MODE, 0, TTY_GID);
+                        if (r < 0 && r != -ENOENT)
+                                log_warning_errno(r, "Failed to reset TTY ownership/access mode of %s, ignoring: %m", path);
+                }
+        }
+}
+
 void exec_status_start(ExecStatus *s, pid_t pid) {
         assert(s);
 
@@ -4664,12 +4688,8 @@ void exec_status_exit(ExecStatus *s, const ExecContext *context, pid_t pid, int
         s->code = code;
         s->status = status;
 
-        if (context) {
-                if (context->utmp_id)
-                        (void) utmp_put_dead_process(context->utmp_id, pid, code, status);
-
-                exec_context_tty_reset(context, NULL);
-        }
+        if (context && context->utmp_id)
+                (void) utmp_put_dead_process(context->utmp_id, pid, code, status);
 }
 
 void exec_status_reset(ExecStatus *s) {
diff --git a/src/core/execute.h b/src/core/execute.h
index df6dd9f3886..214dd64bbd2 100644
--- a/src/core/execute.h
+++ b/src/core/execute.h
@@ -374,6 +374,8 @@ int exec_context_get_effective_ioprio(const ExecContext *c);
 
 void exec_context_free_log_extra_fields(ExecContext *c);
 
+void exec_context_revert_tty(ExecContext *c);
+
 void exec_status_start(ExecStatus *s, pid_t pid);
 void exec_status_exit(ExecStatus *s, const ExecContext *context, pid_t pid, int code, int status);
 void exec_status_dump(const ExecStatus *s, FILE *f, const char *prefix);
diff --git a/src/core/service.c b/src/core/service.c
index 25a12975695..89029b6d115 100644
--- a/src/core/service.c
+++ b/src/core/service.c
@@ -1754,6 +1754,9 @@ static void service_enter_dead(Service *s, ServiceResult f, bool allow_restart)
         if (s->pid_file)
                 (void) unlink(s->pid_file);
 
+        /* Reset TTY ownership if necessary */
+        exec_context_revert_tty(&s->exec_context);
+
         return;
 
 fail: