From 955f1c852edef4c925ae021d49257c09573abb2c Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 20 Mar 2019 21:18:59 +0100 Subject: [PATCH 1/4] execute: use path_equal() to compare tty names After all they might be strings such as pts/1 which we really should consider the same as pts//1. --- src/core/execute.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/core/execute.c b/src/core/execute.c index d46476630cc..ac741c2772c 100644 --- a/src/core/execute.c +++ b/src/core/execute.c @@ -4157,7 +4157,7 @@ static bool tty_may_match_dev_console(const char *tty) { return true; /* if we could not resolve, assume it may */ /* "tty0" means the active VC, so it may be the same sometimes */ - return streq(resolved, tty) || (streq(resolved, "tty0") && tty_is_vc(tty)); + return path_equal(resolved, tty) || (streq(resolved, "tty0") && tty_is_vc(tty)); } bool exec_context_may_touch_console(const ExecContext *ec) { From 6c0ae739569038a0b65b756cae3248c889c22574 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 20 Mar 2019 21:20:00 +0100 Subject: [PATCH 2/4] execute: split check if we might touch a tty out of exec_context_may_touch_console() Some simple refactoring that'll come handy in a later commit. --- src/core/execute.c | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/src/core/execute.c b/src/core/execute.c index ac741c2772c..5511c1aac5b 100644 --- a/src/core/execute.c +++ b/src/core/execute.c @@ -4160,14 +4160,20 @@ static bool tty_may_match_dev_console(const char *tty) { return path_equal(resolved, tty) || (streq(resolved, "tty0") && tty_is_vc(tty)); } -bool exec_context_may_touch_console(const ExecContext *ec) { +static bool exec_context_may_touch_tty(const ExecContext *ec) { + assert(ec); - return (ec->tty_reset || + return ec->tty_reset || ec->tty_vhangup || ec->tty_vt_disallocate || is_terminal_input(ec->std_input) || is_terminal_output(ec->std_output) || - is_terminal_output(ec->std_error)) && + is_terminal_output(ec->std_error); +} + +bool exec_context_may_touch_console(const ExecContext *ec) { + + return exec_context_may_touch_tty(ec) && tty_may_match_dev_console(exec_context_tty_path(ec)); } From 6f765baf23d0d4ed7efa6e2db2a629fd519f3a7a Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 20 Mar 2019 21:28:02 +0100 Subject: [PATCH 3/4] 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. --- src/core/execute.c | 32 ++++++++++++++++++++++++++------ src/core/execute.h | 2 ++ src/core/service.c | 3 +++ 3 files changed, 31 insertions(+), 6 deletions(-) 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: From 13fbfc301382a56a4f14eb1bb307b90a03c0ec01 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 20 Mar 2019 21:49:33 +0100 Subject: [PATCH 4/4] update TODO --- TODO | 17 ----------------- 1 file changed, 17 deletions(-) diff --git a/TODO b/TODO index 7b678e06c46..445a83ae8aa 100644 --- a/TODO +++ b/TODO @@ -78,8 +78,6 @@ Features: * maybe implicitly attach monotonic+realtime timestamps to outgoing messages in log.c and sd-journal-send -* chown() tty a service is attached to after the service goes down - * optionally: turn on cgroup delegation for per-session scope units * introduce per-unit (i.e. per-slice, per-service) journal log size limits. @@ -258,8 +256,6 @@ Features: * support projid-based quota in machinectl for containers -* Add NetworkNamespacePath= to specify a path to a network namespace - * maybe use SOURCE_DATE_EPOCH (i.e. the env var the reproducible builds folks introduced) as the RTC epoch, instead of the mtime of NEWS. @@ -439,8 +435,6 @@ Features: * optionally, also require WATCHDOG=1 notifications during service start-up and shutdown -* resolved: when routing queries, make sure only look for the *longest* suffix... - * delay activation of logind until somebody logs in, or when /dev/tty0 pulls it in or lingering is on (so that containers don't bother with it until PAM is used). also exit-on-idle @@ -585,12 +579,6 @@ Features: service instances processing the listening socket, and open this up for ReusePort= -* socket units: support creating sockets in different namespace, - opening it up for JoinsNamespaceOf=. This would require to fork off - a tiny process that joins the namespace and creates/binds the socket - and passes this back to PID1 via SCM_RIGHTS. This also could be used - to allow Chown/chgrp on sockets without requiring NSS in PID 1. - * introduce bus call FreezeUnit(s, b), as well as "systemctl freeze $UNIT" and "systemctl thaw $UNIT" as wrappers around this. The calls should SIGSTOP all unit processes in a loop until all processes of @@ -631,9 +619,6 @@ Features: * load .d/*.conf dropins for device units -* allow implementation of InaccessibleDirectories=/ plus - ReadOnlyDirectories=... for whitelisting files for a service. - * sd-bus: - EBADSLT handling - GetAllProperties() on a non-existing object does not result in a failure currently @@ -736,8 +721,6 @@ Features: - follow PropertiesChanged state more closely, to deal with quick logouts and relogins -* exec: when deinitializating a tty device fix the perms and group, too, not only when initializing. Set access mode/gid to 0620/tty. - * journal: - consider introducing implicit _TTY= + _PPID= + _EUID= + _EGID= + _FSUID= + _FSGID= fields - import and delete pstore filesystem content at startup