From 8405dcf752487a38d92896486ced2889aea8e746 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Thu, 15 Feb 2018 10:18:25 +0100 Subject: nspawn: make sure we don't leak the fd in chase_symlinks_and_update No callers use CHASE_OPEN right now, but let's be defensive. --- src/nspawn/nspawn.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) (limited to 'src/nspawn') diff --git a/src/nspawn/nspawn.c b/src/nspawn/nspawn.c index 0f05ecff03..7405359cc7 100644 --- a/src/nspawn/nspawn.c +++ b/src/nspawn/nspawn.c @@ -2233,10 +2233,8 @@ static int chase_symlinks_and_update(char **p, unsigned flags) { if (r < 0) return log_error_errno(r, "Failed to resolve path %s: %m", *p); - free(*p); - *p = chased; - - return 0; + free_and_replace(*p, chased); + return r; /* r might be an fd here in case we ever use CHASE_OPEN in flags */ } static int determine_uid_shift(const char *directory) { -- cgit v1.2.3 From 30ffb010ff98fd47ae6e1fe1139661396f697e89 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Mon, 12 Feb 2018 17:23:35 +0900 Subject: nspawn: fix indentation --- src/nspawn/nspawn-mount.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) (limited to 'src/nspawn') diff --git a/src/nspawn/nspawn-mount.c b/src/nspawn/nspawn-mount.c index c9236ea3d1..c507b6a7ca 100644 --- a/src/nspawn/nspawn-mount.c +++ b/src/nspawn/nspawn-mount.c @@ -545,21 +545,21 @@ int mount_all(const char *dest, { "/proc/sys", "/proc/sys", NULL, NULL, MS_BIND, MOUNT_FATAL|MOUNT_IN_USERNS|MOUNT_APPLY_APIVFS_RO }, /* Bind mount first ... */ { "/proc/sys/net", "/proc/sys/net", NULL, NULL, MS_BIND, MOUNT_FATAL|MOUNT_IN_USERNS|MOUNT_APPLY_APIVFS_RO|MOUNT_APPLY_APIVFS_NETNS }, /* (except for this) */ { NULL, "/proc/sys", NULL, NULL, MS_BIND|MS_RDONLY|MS_NOSUID|MS_NOEXEC|MS_NODEV|MS_REMOUNT, MOUNT_FATAL|MOUNT_IN_USERNS|MOUNT_APPLY_APIVFS_RO }, /* ... then, make it r/o */ - { "/proc/sysrq-trigger", "/proc/sysrq-trigger", NULL, NULL, MS_BIND, MOUNT_IN_USERNS|MOUNT_APPLY_APIVFS_RO }, /* Bind mount first ... */ - { NULL, "/proc/sysrq-trigger", NULL, NULL, MS_BIND|MS_RDONLY|MS_NOSUID|MS_NOEXEC|MS_NODEV|MS_REMOUNT, MOUNT_IN_USERNS|MOUNT_APPLY_APIVFS_RO }, /* ... then, make it r/o */ + { "/proc/sysrq-trigger", "/proc/sysrq-trigger", NULL, NULL, MS_BIND, MOUNT_IN_USERNS|MOUNT_APPLY_APIVFS_RO }, /* Bind mount first ... */ + { NULL, "/proc/sysrq-trigger", NULL, NULL, MS_BIND|MS_RDONLY|MS_NOSUID|MS_NOEXEC|MS_NODEV|MS_REMOUNT, MOUNT_IN_USERNS|MOUNT_APPLY_APIVFS_RO }, /* ... then, make it r/o */ /* outer child mounts */ - { "tmpfs", "/tmp", "tmpfs", "mode=1777", MS_NOSUID|MS_NODEV|MS_STRICTATIME, MOUNT_FATAL }, + { "tmpfs", "/tmp", "tmpfs", "mode=1777", MS_NOSUID|MS_NODEV|MS_STRICTATIME, MOUNT_FATAL }, { "tmpfs", "/sys", "tmpfs", "mode=755", MS_NOSUID|MS_NOEXEC|MS_NODEV, MOUNT_FATAL|MOUNT_APPLY_APIVFS_NETNS }, { "sysfs", "/sys", "sysfs", NULL, MS_RDONLY|MS_NOSUID|MS_NOEXEC|MS_NODEV, MOUNT_FATAL|MOUNT_APPLY_APIVFS_RO }, /* skipped if above was mounted */ - { "sysfs", "/sys", "sysfs", NULL, MS_NOSUID|MS_NOEXEC|MS_NODEV, MOUNT_FATAL }, /* skipped if above was mounted */ + { "sysfs", "/sys", "sysfs", NULL, MS_NOSUID|MS_NOEXEC|MS_NODEV, MOUNT_FATAL }, /* skipped if above was mounted */ { "tmpfs", "/dev", "tmpfs", "mode=755", MS_NOSUID|MS_STRICTATIME, MOUNT_FATAL }, { "tmpfs", "/dev/shm", "tmpfs", "mode=1777", MS_NOSUID|MS_NODEV|MS_STRICTATIME, MOUNT_FATAL }, { "tmpfs", "/run", "tmpfs", "mode=755", MS_NOSUID|MS_NODEV|MS_STRICTATIME, MOUNT_FATAL }, #if HAVE_SELINUX - { "/sys/fs/selinux", "/sys/fs/selinux", NULL, NULL, MS_BIND, 0 }, /* Bind mount first */ - { NULL, "/sys/fs/selinux", NULL, NULL, MS_BIND|MS_RDONLY|MS_NOSUID|MS_NOEXEC|MS_NODEV|MS_REMOUNT, 0 }, /* Then, make it r/o */ + { "/sys/fs/selinux", "/sys/fs/selinux", NULL, NULL, MS_BIND, 0 }, /* Bind mount first */ + { NULL, "/sys/fs/selinux", NULL, NULL, MS_BIND|MS_RDONLY|MS_NOSUID|MS_NOEXEC|MS_NODEV|MS_REMOUNT, 0 }, /* Then, make it r/o */ #endif }; -- cgit v1.2.3 From 72d967df3e27186dd014bed2c6e7400cc32d84c5 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Wed, 14 Feb 2018 14:25:22 +0900 Subject: nspawn: remove unnecessary mount option parsing logic --- src/nspawn/nspawn-mount.c | 45 ++------------------------------------------- 1 file changed, 2 insertions(+), 43 deletions(-) (limited to 'src/nspawn') diff --git a/src/nspawn/nspawn-mount.c b/src/nspawn/nspawn-mount.c index c507b6a7ca..5193f1f69d 100644 --- a/src/nspawn/nspawn-mount.c +++ b/src/nspawn/nspawn-mount.c @@ -634,56 +634,15 @@ int mount_all(const char *dest, return 0; } -static int parse_mount_bind_options(const char *options, unsigned long *mount_flags, char **mount_opts) { - const char *p = options; - unsigned long flags = *mount_flags; - char *opts = NULL; - int r; - - assert(options); - - for (;;) { - _cleanup_free_ char *word = NULL; - - r = extract_first_word(&p, &word, ",", 0); - if (r < 0) - return log_error_errno(r, "Failed to extract mount option: %m"); - if (r == 0) - break; - - if (streq(word, "rbind")) - flags |= MS_REC; - else if (streq(word, "norbind")) - flags &= ~MS_REC; - else { - log_error("Invalid bind mount option: %s", word); - return -EINVAL; - } - } - - *mount_flags = flags; - /* in the future mount_opts will hold string options for mount(2) */ - *mount_opts = opts; - - return 0; -} - static int mount_bind(const char *dest, CustomMount *m) { - _cleanup_free_ char *mount_opts = NULL, *where = NULL; - unsigned long mount_flags = MS_BIND | MS_REC; + _cleanup_free_ char *where = NULL; struct stat source_st, dest_st; int r; assert(dest); assert(m); - if (m->options) { - r = parse_mount_bind_options(m->options, &mount_flags, &mount_opts); - if (r < 0) - return r; - } - if (stat(m->source, &source_st) < 0) return log_error_errno(errno, "Failed to stat %s: %m", m->source); @@ -723,7 +682,7 @@ static int mount_bind(const char *dest, CustomMount *m) { } - r = mount_verbose(LOG_ERR, m->source, where, NULL, mount_flags, mount_opts); + r = mount_verbose(LOG_ERR, m->source, where, NULL, MS_BIND | MS_REC, m->options); if (r < 0) return r; -- cgit v1.2.3 From aa484f356110d9118c44389fe8f03ee7b25a7746 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Mon, 26 Feb 2018 21:20:00 +0100 Subject: tree-wide: use reallocarray instead of our home-grown realloc_multiply (#8279) There isn't much difference, but in general we prefer to use the standard functions. glibc provides reallocarray since version 2.26. I moved explicit_bzero is configure test to the bottom, so that the two stdlib functions are at the bottom. --- src/nspawn/nspawn-mount.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src/nspawn') diff --git a/src/nspawn/nspawn-mount.c b/src/nspawn/nspawn-mount.c index 5193f1f69d..0ee69114b2 100644 --- a/src/nspawn/nspawn-mount.c +++ b/src/nspawn/nspawn-mount.c @@ -48,7 +48,7 @@ CustomMount* custom_mount_add(CustomMount **l, unsigned *n, CustomMountType t) { assert(t >= 0); assert(t < _CUSTOM_MOUNT_TYPE_MAX); - c = realloc_multiply(*l, (*n + 1), sizeof(CustomMount)); + c = reallocarray(*l, *n + 1, sizeof(CustomMount)); if (!c) return NULL; -- cgit v1.2.3 From c5b82d86b5fe1587a6798266baf1a2f8457e6f23 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 26 Feb 2018 15:29:30 +0100 Subject: nspawn: port some code to use read_line() This shortens our code a bit. Which is always nice. --- src/nspawn/nspawn-setuid.c | 44 +++++++++++++++++++++----------------------- 1 file changed, 21 insertions(+), 23 deletions(-) (limited to 'src/nspawn') diff --git a/src/nspawn/nspawn-setuid.c b/src/nspawn/nspawn-setuid.c index b08bcd988a..029c4e7954 100644 --- a/src/nspawn/nspawn-setuid.c +++ b/src/nspawn/nspawn-setuid.c @@ -23,8 +23,10 @@ #include #include "alloc-util.h" +#include "def.h" #include "errno.h" #include "fd-util.h" +#include "fileio.h" #include "mkdir.h" #include "nspawn-setuid.h" #include "process-util.h" @@ -87,10 +89,10 @@ static int spawn_getent(const char *database, const char *key, pid_t *rpid) { } int change_uid_gid(const char *user, char **_home) { - char line[LINE_MAX], *x, *u, *g, *h; + char *x, *u, *g, *h; const char *word, *state; _cleanup_free_ uid_t *uids = NULL; - _cleanup_free_ char *home = NULL; + _cleanup_free_ char *home = NULL, *line = NULL; _cleanup_fclose_ FILE *f = NULL; _cleanup_close_ int fd = -1; unsigned n_uids = 0; @@ -118,21 +120,18 @@ int change_uid_gid(const char *user, char **_home) { if (fd < 0) return fd; - f = fdopen(fd, "r"); + f = fdopen(fd, "re"); if (!f) return log_oom(); fd = -1; - if (!fgets(line, sizeof(line), f)) { - if (!ferror(f)) { - log_error("Failed to resolve user %s.", user); - return -ESRCH; - } - - return log_error_errno(errno, "Failed to read from getent: %m"); + r = read_line(f, LONG_LINE_MAX, &line); + if (r == 0) { + log_error("Failed to resolve user %s.", user); + return -ESRCH; } - - truncate_nl(line); + if (r < 0) + return log_error_errno(r, "Failed to read from getent: %m"); (void) wait_for_terminate_and_check("getent passwd", pid, WAIT_LOG); @@ -195,27 +194,26 @@ int change_uid_gid(const char *user, char **_home) { if (!home) return log_oom(); + f = safe_fclose(f); + line = mfree(line); + /* Second, get group memberships */ fd = spawn_getent("initgroups", user, &pid); if (fd < 0) return fd; - fclose(f); - f = fdopen(fd, "r"); + f = fdopen(fd, "re"); if (!f) return log_oom(); fd = -1; - if (!fgets(line, sizeof(line), f)) { - if (!ferror(f)) { - log_error("Failed to resolve user %s.", user); - return -ESRCH; - } - - return log_error_errno(errno, "Failed to read from getent: %m"); + r = read_line(f, LONG_LINE_MAX, &line); + if (r == 0) { + log_error("Failed to resolve user %s.", user); + return -ESRCH; } - - truncate_nl(line); + if (r < 0) + return log_error_errno(r, "Failed to read from getent: %m"); (void) wait_for_terminate_and_check("getent initgroups", pid, WAIT_LOG); -- cgit v1.2.3 From 5018c0c9e8062c38e41da54759b3088a241d734b Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 26 Feb 2018 15:30:05 +0100 Subject: nspawn: use STR_IN_SET() where we can --- src/nspawn/nspawn-setuid.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'src/nspawn') diff --git a/src/nspawn/nspawn-setuid.c b/src/nspawn/nspawn-setuid.c index 029c4e7954..a98fcc6844 100644 --- a/src/nspawn/nspawn-setuid.c +++ b/src/nspawn/nspawn-setuid.c @@ -32,6 +32,7 @@ #include "process-util.h" #include "signal-util.h" #include "string-util.h" +#include "strv.h" #include "user-util.h" #include "util.h" @@ -104,7 +105,7 @@ int change_uid_gid(const char *user, char **_home) { assert(_home); - if (!user || streq(user, "root") || streq(user, "0")) { + if (!user || STR_IN_SET(user, "root", "0")) { /* Reset everything fully to 0, just in case */ r = reset_uid_gid(); -- cgit v1.2.3 From c7f9a8d2705ad4d18d1ca43f3a8625575f1186e5 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 26 Feb 2018 15:30:19 +0100 Subject: nspawn: propagate original error. No need to make up -EIO --- src/nspawn/nspawn-setuid.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) (limited to 'src/nspawn') diff --git a/src/nspawn/nspawn-setuid.c b/src/nspawn/nspawn-setuid.c index a98fcc6844..8f2359ad91 100644 --- a/src/nspawn/nspawn-setuid.c +++ b/src/nspawn/nspawn-setuid.c @@ -233,10 +233,8 @@ int change_uid_gid(const char *user, char **_home) { return log_oom(); r = parse_uid(c, &uids[n_uids++]); - if (r < 0) { - log_error("Failed to parse group data from getent."); - return -EIO; - } + if (r < 0) + return log_error_errno(r, "Failed to parse group data from getent: %m"); } r = mkdir_parents(home, 0775); -- cgit v1.2.3 From e7685a77b41bbd1b8289aeaf75fccaf4bb68a361 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 26 Feb 2018 15:41:38 +0100 Subject: util: add new safe_close_above_stdio() wrapper At various places we only want to close fds if they are not stdin/stdout/stderr, i.e. fds 0, 1, 2. Let's add a unified helper call for that, and port everything over. --- src/nspawn/nspawn-setuid.c | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) (limited to 'src/nspawn') diff --git a/src/nspawn/nspawn-setuid.c b/src/nspawn/nspawn-setuid.c index 8f2359ad91..c4ad172512 100644 --- a/src/nspawn/nspawn-setuid.c +++ b/src/nspawn/nspawn-setuid.c @@ -57,10 +57,8 @@ static int spawn_getent(const char *database, const char *key, pid_t *rpid) { if (dup3(pipe_fds[1], STDOUT_FILENO, 0) < 0) _exit(EXIT_FAILURE); - if (pipe_fds[0] > 2) - safe_close(pipe_fds[0]); - if (pipe_fds[1] > 2) - safe_close(pipe_fds[1]); + safe_close_above_stdio(pipe_fds[0]); + safe_close_above_stdio(pipe_fds[1]); nullfd = open("/dev/null", O_RDWR); if (nullfd < 0) @@ -72,8 +70,7 @@ static int spawn_getent(const char *database, const char *key, pid_t *rpid) { if (dup3(nullfd, STDERR_FILENO, 0) < 0) _exit(EXIT_FAILURE); - if (nullfd > 2) - safe_close(nullfd); + safe_close_above_stdio(nullfd); close_all_fds(NULL, 0); -- cgit v1.2.3 From 05a8b3305f88fff91b14fd6417f74e1b5b7388b3 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 26 Feb 2018 20:51:04 +0100 Subject: nspawn: close pipe on error --- src/nspawn/nspawn-setuid.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) (limited to 'src/nspawn') diff --git a/src/nspawn/nspawn-setuid.c b/src/nspawn/nspawn-setuid.c index c4ad172512..46cdcd2e84 100644 --- a/src/nspawn/nspawn-setuid.c +++ b/src/nspawn/nspawn-setuid.c @@ -48,11 +48,13 @@ static int spawn_getent(const char *database, const char *key, pid_t *rpid) { return log_error_errno(errno, "Failed to allocate pipe: %m"); r = safe_fork("(getent)", FORK_RESET_SIGNALS|FORK_DEATHSIG|FORK_LOG, &pid); - if (r < 0) + if (r < 0) { + safe_close_pair(pipe_fds); return r; + } if (r == 0) { - int nullfd; char *empty_env = NULL; + int nullfd; if (dup3(pipe_fds[1], STDOUT_FILENO, 0) < 0) _exit(EXIT_FAILURE); -- cgit v1.2.3 From 2b33ab0957f453a06b58e4bee482f2c2d4e100c1 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 28 Feb 2018 23:32:49 +0100 Subject: tree-wide: port various places over to use new rearrange_stdio() --- src/nspawn/nspawn-setuid.c | 18 ++---------------- src/nspawn/nspawn.c | 22 +++++++--------------- 2 files changed, 9 insertions(+), 31 deletions(-) (limited to 'src/nspawn') diff --git a/src/nspawn/nspawn-setuid.c b/src/nspawn/nspawn-setuid.c index 46cdcd2e84..2dee5f8ec8 100644 --- a/src/nspawn/nspawn-setuid.c +++ b/src/nspawn/nspawn-setuid.c @@ -54,26 +54,12 @@ static int spawn_getent(const char *database, const char *key, pid_t *rpid) { } if (r == 0) { char *empty_env = NULL; - int nullfd; - if (dup3(pipe_fds[1], STDOUT_FILENO, 0) < 0) - _exit(EXIT_FAILURE); - - safe_close_above_stdio(pipe_fds[0]); - safe_close_above_stdio(pipe_fds[1]); - - nullfd = open("/dev/null", O_RDWR); - if (nullfd < 0) - _exit(EXIT_FAILURE); + safe_close(pipe_fds[0]); - if (dup3(nullfd, STDIN_FILENO, 0) < 0) + if (rearrange_stdio(-1, pipe_fds[1], -1) < 0) _exit(EXIT_FAILURE); - if (dup3(nullfd, STDERR_FILENO, 0) < 0) - _exit(EXIT_FAILURE); - - safe_close_above_stdio(nullfd); - close_all_fds(NULL, 0); execle("/usr/bin/getent", "getent", database, key, NULL, &empty_env); diff --git a/src/nspawn/nspawn.c b/src/nspawn/nspawn.c index 7405359cc7..90f1c4184f 100644 --- a/src/nspawn/nspawn.c +++ b/src/nspawn/nspawn.c @@ -2582,23 +2582,15 @@ static int outer_child( return log_error_errno(errno, "PR_SET_PDEATHSIG failed: %m"); if (interactive) { - close_nointr(STDIN_FILENO); - close_nointr(STDOUT_FILENO); - close_nointr(STDERR_FILENO); - - r = open_terminal(console, O_RDWR); - if (r != STDIN_FILENO) { - if (r >= 0) { - safe_close(r); - r = -EINVAL; - } + int terminal; - return log_error_errno(r, "Failed to open console: %m"); - } + terminal = open_terminal(console, O_RDWR); + if (terminal < 0) + return log_error_errno(terminal, "Failed to open console: %m"); - if (dup2(STDIN_FILENO, STDOUT_FILENO) != STDOUT_FILENO || - dup2(STDIN_FILENO, STDERR_FILENO) != STDERR_FILENO) - return log_error_errno(errno, "Failed to duplicate console: %m"); + r = rearrange_stdio(terminal, terminal, terminal); /* invalidates 'terminal' on success and failure */ + if (r < 0) + return log_error_errno(r, "Failed to move console to stdin/stdout/stderr: %m"); } r = reset_audit_loginuid(); -- cgit v1.2.3