From f896e672ec6101ccbb21108345946e834455a25f Mon Sep 17 00:00:00 2001 From: Franck Bui Date: Fri, 3 Apr 2020 10:00:25 +0200 Subject: [PATCH] pid1: by default make user units inherit their umask from the user manager This patch changes the way user managers set the default umask for the units it manages. Indeed one can expect that if user manager's umask is redefined through PAM (via /etc/login.defs or pam_umask), all its children including the units it spawns have their umask set to the new value. Hence make user units inherit their umask value from their parent instead of the hard coded value 0022 but allow them to override this value via their unit file. Note that reexecuting managers with 'systemctl daemon-reexec' after changing UMask= has no effect. To take effect managers need to be restarted with 'systemct restart' instead. This behavior was already present before this patch. Fixes #6077. (cherry picked from commit 5e37d1930b41b24c077ce37c6db0e36c745106c7) Related: RHEL-28048 --- man/systemd.exec.xml | 9 +++++++-- src/basic/process-util.c | 17 +++++++++++++++++ src/basic/process-util.h | 1 + src/core/unit.c | 12 ++++++++++-- 4 files changed, 35 insertions(+), 4 deletions(-) diff --git a/man/systemd.exec.xml b/man/systemd.exec.xml index b04b4ba552..844c1ce94b 100644 --- a/man/systemd.exec.xml +++ b/man/systemd.exec.xml @@ -590,8 +590,13 @@ CapabilityBoundingSet=~CAP_B CAP_C UMask= Controls the file mode creation mask. Takes an access mode in octal notation. See - umask2 for details. Defaults - to 0022. + umask2 for + details. Defaults to 0022 for system units. For units of the user service manager the default value + is inherited from the user instance (whose default is inherited from the system service manager, and + thus also is 0022). Hence changing the default value of a user instance, either via + UMask= or via a PAM module, will affect the user instance itself and all user + units started by the user instance unless a user unit has specified its own + UMask=. diff --git a/src/basic/process-util.c b/src/basic/process-util.c index 9e2237375d..af44bfab3e 100644 --- a/src/basic/process-util.c +++ b/src/basic/process-util.c @@ -657,6 +657,23 @@ int get_process_ppid(pid_t pid, pid_t *ret) { return 0; } +int get_process_umask(pid_t pid, mode_t *umask) { + _cleanup_free_ char *m = NULL; + const char *p; + int r; + + assert(umask); + assert(pid >= 0); + + p = procfs_file_alloca(pid, "status"); + + r = get_proc_field(p, "Umask", WHITESPACE, &m); + if (r == -ENOENT) + return -ESRCH; + + return parse_mode(m, umask); +} + int wait_for_terminate(pid_t pid, siginfo_t *status) { siginfo_t dummy; diff --git a/src/basic/process-util.h b/src/basic/process-util.h index a3bd2851b4..9059aad4cc 100644 --- a/src/basic/process-util.h +++ b/src/basic/process-util.h @@ -41,6 +41,7 @@ int get_process_cwd(pid_t pid, char **cwd); int get_process_root(pid_t pid, char **root); int get_process_environ(pid_t pid, char **environ); int get_process_ppid(pid_t pid, pid_t *ppid); +int get_process_umask(pid_t pid, mode_t *umask); int wait_for_terminate(pid_t pid, siginfo_t *status); diff --git a/src/core/unit.c b/src/core/unit.c index 76fb9f8075..d3459dcdd0 100644 --- a/src/core/unit.c +++ b/src/core/unit.c @@ -167,8 +167,16 @@ static void unit_init(Unit *u) { if (ec) { exec_context_init(ec); - ec->keyring_mode = MANAGER_IS_SYSTEM(u->manager) ? - EXEC_KEYRING_SHARED : EXEC_KEYRING_INHERIT; + if (MANAGER_IS_SYSTEM(u->manager)) + ec->keyring_mode = EXEC_KEYRING_SHARED; + else { + ec->keyring_mode = EXEC_KEYRING_INHERIT; + + /* User manager might have its umask redefined by PAM or UMask=. In this + * case let the units it manages inherit this value by default. They can + * still tune this value through their own unit file */ + (void) get_process_umask(getpid_cached(), &ec->umask); + } } kc = unit_get_kill_context(u);