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 5511c1aac5b5ef123d41cd227accf207dcbac40b..dabb6d824fbc4ce2e6bf8986510f51b8650bb57c 100644 (file)
--- 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 df6dd9f38868e4dce62b928caa678f2ef73d5260..214dd64bbd2fc50a11977db2b65584d852962870 100644 (file)
--- 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 25a129756952afd993ea32e41ab7b599bd8a3ed3..89029b6d115eaa72c0c43ae04fbf19264b49800c 100644 (file)
--- 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: