From 94c4c6227957ef34b7f46daaab75bb5911062462 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 21 Feb 2017 16:18:03 +0100 Subject: dissect: generate friendly error messages for more error conditions Fixes: #5408 --- src/dissect/dissect.c | 8 ++++++++ 1 file changed, 8 insertions(+) (limited to 'src') diff --git a/src/dissect/dissect.c b/src/dissect/dissect.c index 59bd7d9e84..06564e94b1 100644 --- a/src/dissect/dissect.c +++ b/src/dissect/dissect.c @@ -208,6 +208,14 @@ int main(int argc, char *argv[]) { log_error_errno(r, "No root partition for specified root hash found in %s.", arg_image); goto finish; } + if (r == -ENOTUNIQ) { + log_error_errno(r, "Multiple suitable root partitions found in image %s.", arg_image); + goto finish; + } + if (r == -ENXIO) { + log_error_errno(r, "No suitable root partition found in image %s.", arg_image); + goto finish; + } if (r < 0) { log_error_errno(r, "Failed to dissect image: %m"); goto finish; -- cgit v1.2.3-54-g00ecf From 71994cff31f2507429dc91550f73fd8e35646147 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 21 Feb 2017 16:25:02 +0100 Subject: sd-netlink: don't give up on netlink on ENOBUFS If our netlink input buffer overruns the kernel will send us ENOBUFS on the next recvmsg(). Don't consider this a complete failure resulting in closing of the netlink socket. Instead, simply continue (after debug logging). Of course, ideally we'd have a better strategy for this, and would have a way to resync if this happens (as well as a scheme for cancelling all ongoing asynchronous transactions), but for now let's at least not choke fatally, and simply accept that we lost some messages and continue. Note that if we lose messages when synchronously waiting for an operation to complete, we'll still propagate the ENOBUFS up, to make the individual transaction fail. See: #5398 (This bug does not properly fix the issue, hence we should leave the bug open.) --- src/libsystemd/sd-netlink/netlink-socket.c | 4 ++-- src/libsystemd/sd-netlink/sd-netlink.c | 4 ++++ 2 files changed, 6 insertions(+), 2 deletions(-) (limited to 'src') diff --git a/src/libsystemd/sd-netlink/netlink-socket.c b/src/libsystemd/sd-netlink/netlink-socket.c index a0fd8a3ac9..129bfd2d80 100644 --- a/src/libsystemd/sd-netlink/netlink-socket.c +++ b/src/libsystemd/sd-netlink/netlink-socket.c @@ -281,7 +281,7 @@ static int socket_recv_message(int fd, struct iovec *iov, uint32_t *_group, bool else if (errno == EAGAIN) log_debug("rtnl: no data in socket"); - return (errno == EAGAIN || errno == EINTR) ? 0 : -errno; + return IN_SET(errno, EAGAIN, EINTR) ? 0 : -errno; } if (sender.nl.nl_pid != 0) { @@ -292,7 +292,7 @@ static int socket_recv_message(int fd, struct iovec *iov, uint32_t *_group, bool /* drop the message */ r = recvmsg(fd, &msg, 0); if (r < 0) - return (errno == EAGAIN || errno == EINTR) ? 0 : -errno; + return IN_SET(errno, EAGAIN, EINTR) ? 0 : -errno; } return 0; diff --git a/src/libsystemd/sd-netlink/sd-netlink.c b/src/libsystemd/sd-netlink/sd-netlink.c index 43114eb825..68435564de 100644 --- a/src/libsystemd/sd-netlink/sd-netlink.c +++ b/src/libsystemd/sd-netlink/sd-netlink.c @@ -276,6 +276,10 @@ static int dispatch_rqueue(sd_netlink *rtnl, sd_netlink_message **message) { if (rtnl->rqueue_size <= 0) { /* Try to read a new message */ r = socket_read_message(rtnl); + if (r == -ENOBUFS) { /* FIXME: ignore buffer overruns for now */ + log_debug_errno(r, "Got ENOBUFS from netlink socket, ignoring."); + return 1; + } if (r <= 0) return r; } -- cgit v1.2.3-54-g00ecf From ce21ed5c6120e33dd3502b25237c721dbbea891a Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 21 Feb 2017 17:13:28 +0100 Subject: copy: a plain unlink() works here too --- src/basic/copy.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src') diff --git a/src/basic/copy.c b/src/basic/copy.c index 6273ac9b47..e120b9eb4e 100644 --- a/src/basic/copy.c +++ b/src/basic/copy.c @@ -558,7 +558,7 @@ int copy_file_atomic(const char *from, const char *to, mode_t mode, unsigned cha } else r = rename_noreplace(AT_FDCWD, t, AT_FDCWD, to); if (r < 0) { - (void) unlink_noerrno(t); + (void) unlink(t); return r; } -- cgit v1.2.3-54-g00ecf From 175d308cad2075658b925e5abdbdfe7fa5fda466 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 21 Feb 2017 17:13:36 +0100 Subject: bootctl: rework file copy routines to reuse copy_bytes() from copy.c Also, make sure to reuse temporary file handling used elsewhere. --- src/boot/bootctl.c | 121 +++++++++++++++++++---------------------------------- 1 file changed, 42 insertions(+), 79 deletions(-) (limited to 'src') diff --git a/src/boot/bootctl.c b/src/boot/bootctl.c index b747a95133..37f52c430a 100644 --- a/src/boot/bootctl.c +++ b/src/boot/bootctl.c @@ -38,20 +38,22 @@ #include "alloc-util.h" #include "blkid-util.h" +#include "copy.h" #include "dirent-util.h" #include "efivars.h" #include "fd-util.h" #include "fileio.h" +#include "fs-util.h" #include "locale-util.h" #include "parse-util.h" #include "rm-rf.h" +#include "stat-util.h" #include "string-util.h" #include "strv.h" #include "umask-util.h" #include "util.h" #include "verbs.h" #include "virt.h" -#include "stat-util.h" static char *arg_path = NULL; static bool arg_touch_variables = true; @@ -476,16 +478,16 @@ static int compare_version(const char *a, const char *b) { return strverscmp(a, b); } -static int version_check(int fd, const char *from, const char *to) { +static int version_check(int fd_from, const char *from, int fd_to, const char *to) { _cleanup_free_ char *a = NULL, *b = NULL; - _cleanup_close_ int fd2 = -1; int r; - assert(fd >= 0); + assert(fd_from >= 0); assert(from); + assert(fd_to >= 0); assert(to); - r = get_file_version(fd, &a); + r = get_file_version(fd_from, &a); if (r < 0) return r; if (r == 0) { @@ -493,15 +495,7 @@ static int version_check(int fd, const char *from, const char *to) { return -EINVAL; } - fd2 = open(to, O_RDONLY|O_CLOEXEC); - if (fd2 < 0) { - if (errno == ENOENT) - return 0; - - return log_error_errno(errno, "Failed to open \"%s\" for reading: %m", to); - } - - r = get_file_version(fd2, &b); + r = get_file_version(fd_to, &b); if (r < 0) return r; if (r == 0 || compare_product(a, b) != 0) { @@ -517,90 +511,59 @@ static int version_check(int fd, const char *from, const char *to) { return 0; } -static int copy_file(const char *from, const char *to, bool force) { - _cleanup_fclose_ FILE *f = NULL, *g = NULL; - char *p; +static int copy_file_with_version_check(const char *from, const char *to, bool force) { + _cleanup_close_ int fd_from = -1, fd_to = -1; + _cleanup_free_ char *t = NULL; int r; - struct timespec t[2]; - struct stat st; - - assert(from); - assert(to); - f = fopen(from, "re"); - if (!f) + fd_from = open(from, O_RDONLY|O_CLOEXEC|O_NOCTTY); + if (fd_from < 0) return log_error_errno(errno, "Failed to open \"%s\" for reading: %m", from); if (!force) { - /* If this is an update, then let's compare versions first */ - r = version_check(fileno(f), from, to); - if (r < 0) - return r; - } - - p = strjoina(to, "~"); - g = fopen(p, "wxe"); - if (!g) { - /* Directory doesn't exist yet? Then let's skip this... */ - if (!force && errno == ENOENT) - return 0; - - return log_error_errno(errno, "Failed to open \"%s\" for writing: %m", to); - } + fd_to = open(to, O_RDONLY|O_CLOEXEC|O_NOCTTY); + if (fd_to < 0) { + if (errno != -ENOENT) + return log_error_errno(errno, "Failed to open \"%s\" for reading: %m", to); + } else { + r = version_check(fd_from, from, fd_to, to); + if (r < 0) + return r; - rewind(f); - do { - size_t k; - uint8_t buf[32*1024]; + if (lseek(fd_from, 0, SEEK_SET) == (off_t) -1) + return log_error_errno(errno, "Failed to seek in \%s\": %m", from); - k = fread(buf, 1, sizeof(buf), f); - if (ferror(f)) { - r = log_error_errno(EIO, "Failed to read \"%s\": %m", from); - goto error; + fd_to = safe_close(fd_to); } + } - if (k == 0) - break; - - fwrite(buf, 1, k, g); - if (ferror(g)) { - r = log_error_errno(EIO, "Failed to write \"%s\": %m", to); - goto error; - } - } while (!feof(f)); + r = tempfn_random(to, NULL, &t); + if (r < 0) + return log_oom(); - r = fflush_and_check(g); - if (r < 0) { - log_error_errno(r, "Failed to write \"%s\": %m", to); - goto error; + RUN_WITH_UMASK(0000) { + fd_to = open(t, O_WRONLY|O_CREAT|O_CLOEXEC|O_EXCL|O_NOFOLLOW, 0644); + if (fd_to < 0) + return log_error_errno(errno, "Failed to open \"%s\" for writing: %m", t); } - r = fstat(fileno(f), &st); + r = copy_bytes(fd_from, fd_to, (uint64_t) -1, COPY_REFLINK); if (r < 0) { - r = log_error_errno(errno, "Failed to get file timestamps of \"%s\": %m", from); - goto error; + unlink(t); + return log_error_errno(errno, "Failed to copy data from \"%s\" to \"%s\": %m", from, t); } - t[0] = st.st_atim; - t[1] = st.st_mtim; + (void) copy_times(fd_from, fd_to); - r = futimens(fileno(g), t); + r = renameat(AT_FDCWD, t, AT_FDCWD, to); if (r < 0) { - r = log_error_errno(errno, "Failed to set file timestamps on \"%s\": %m", p); - goto error; - } - - if (rename(p, to) < 0) { - r = log_error_errno(errno, "Failed to rename \"%s\" to \"%s\": %m", p, to); - goto error; + (void) unlink_noerrno(t); + return log_error_errno(errno, "Failed to rename \"%s\" to \"%s\": %m", t, to); } log_info("Copied \"%s\" to \"%s\".", from, to); - return 0; -error: - (void) unlink(p); - return r; + return 0; } static int mkdir_one(const char *prefix, const char *suffix) { @@ -644,7 +607,7 @@ static int copy_one_file(const char *esp_path, const char *name, bool force) { p = strjoina(BOOTLIBDIR "/", name); q = strjoina(esp_path, "/EFI/systemd/", name); - r = copy_file(p, q, force); + r = copy_file_with_version_check(p, q, force); if (startswith(name, "systemd-boot")) { int k; @@ -654,7 +617,7 @@ static int copy_one_file(const char *esp_path, const char *name, bool force) { v = strjoina(esp_path, "/EFI/BOOT/BOOT", name + strlen("systemd-boot")); ascii_strupper(strrchr(v, '/') + 1); - k = copy_file(p, v, force); + k = copy_file_with_version_check(p, v, force); if (k < 0 && r == 0) r = k; } -- cgit v1.2.3-54-g00ecf From f5b84de2abbb48b33c710ec76c8b2f59e90386ae Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 21 Feb 2017 17:44:35 +0100 Subject: bootctl: create loader.conf only if it doesn't exist yet If the snippet aleady exists, don't do anything, as the file was already installed then. (This also reworks the code to create the file atomically) Fixes: #5396 --- src/basic/fs-util.h | 6 ++++++ src/boot/bootctl.c | 31 +++++++++++++++++++++++++------ 2 files changed, 31 insertions(+), 6 deletions(-) (limited to 'src') diff --git a/src/basic/fs-util.h b/src/basic/fs-util.h index 5fe5c71ff0..094acf1799 100644 --- a/src/basic/fs-util.h +++ b/src/basic/fs-util.h @@ -91,3 +91,9 @@ static inline void rmdir_and_free(char *p) { free(p); } DEFINE_TRIVIAL_CLEANUP_FUNC(char*, rmdir_and_free); + +static inline void unlink_and_free(char *p) { + (void) unlink(p); + free(p); +} +DEFINE_TRIVIAL_CLEANUP_FUNC(char*, unlink_and_free); diff --git a/src/boot/bootctl.c b/src/boot/bootctl.c index 37f52c430a..116608bbd3 100644 --- a/src/boot/bootctl.c +++ b/src/boot/bootctl.c @@ -913,20 +913,31 @@ static int remove_variables(sd_id128_t uuid, const char *path, bool in_order) { static int install_loader_config(const char *esp_path) { - _cleanup_fclose_ FILE *f = NULL; char machine_string[SD_ID128_STRING_MAX]; + _cleanup_(unlink_and_freep) char *t = NULL; + _cleanup_fclose_ FILE *f = NULL; sd_id128_t machine_id; const char *p; - int r; + int r, fd; r = sd_id128_get_machine(&machine_id); if (r < 0) return log_error_errno(r, "Failed to get machine did: %m"); p = strjoina(esp_path, "/loader/loader.conf"); - f = fopen(p, "wxe"); - if (!f) - return log_error_errno(errno, "Failed to open loader.conf for writing: %m"); + + if (access(p, F_OK) >= 0) /* Silently skip creation if the file already exists (early check) */ + return 0; + + fd = open_tmpfile_linkable(p, O_WRONLY|O_CLOEXEC, &t); + if (fd < 0) + return log_error_errno(fd, "Failed to open \"%s\" for writing: %m", p); + + f = fdopen(fd, "we"); + if (!f) { + safe_close(fd); + return log_oom(); + } fprintf(f, "#timeout 3\n"); fprintf(f, "default %s-*\n", sd_id128_to_string(machine_id, machine_string)); @@ -935,7 +946,15 @@ static int install_loader_config(const char *esp_path) { if (r < 0) return log_error_errno(r, "Failed to write \"%s\": %m", p); - return 0; + r = link_tmpfile(fd, t, p); + if (r == -EEXIST) + return 0; /* Silently skip creation if the file exists now (recheck) */ + if (r < 0) + return log_error_errno(r, "Failed to move \"%s\" into place: %m", p); + + t = mfree(t); + + return 1; } static int help(int argc, char *argv[], void *userdata) { -- cgit v1.2.3-54-g00ecf From 48a601fe5de8aa0d89ba6dadde168769fa7ce992 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 21 Feb 2017 17:57:55 +0100 Subject: log: never log into foreign fd #2 in PID 1 or its pre-execve() children Fixes: #5401 --- src/basic/log.c | 7 ++++++- src/basic/log.h | 1 + src/core/main.c | 11 +++++++++-- 3 files changed, 16 insertions(+), 3 deletions(-) (limited to 'src') diff --git a/src/basic/log.c b/src/basic/log.c index e6d2d61d72..36efc9ac7d 100644 --- a/src/basic/log.c +++ b/src/basic/log.c @@ -72,6 +72,7 @@ static bool show_color = false; static bool show_location = false; static bool upgrade_syslog_to_journal = false; +static bool always_reopen_console = false; /* Akin to glibc's __abort_msg; which is private and we hence cannot * use here. */ @@ -95,7 +96,7 @@ static int log_open_console(void) { if (console_fd >= 0) return 0; - if (getpid() == 1) { + if (always_reopen_console) { console_fd = open_terminal("/dev/console", O_WRONLY|O_NOCTTY|O_CLOEXEC); if (console_fd < 0) return console_fd; @@ -1171,3 +1172,7 @@ int log_syntax_internal( unit_fmt, unit, NULL); } + +void log_set_always_reopen_console(bool b) { + always_reopen_console = b; +} diff --git a/src/basic/log.h b/src/basic/log.h index 9cacbb6b70..72714e02e5 100644 --- a/src/basic/log.h +++ b/src/basic/log.h @@ -220,6 +220,7 @@ LogTarget log_target_from_string(const char *s) _pure_; void log_received_signal(int level, const struct signalfd_siginfo *si); void log_set_upgrade_syslog_to_journal(bool b); +void log_set_always_reopen_console(bool b); int log_syntax_internal( const char *unit, diff --git a/src/core/main.c b/src/core/main.c index 3c6b18229c..bcf9ea5f25 100644 --- a/src/core/main.c +++ b/src/core/main.c @@ -1414,10 +1414,17 @@ int main(int argc, char *argv[]) { log_set_upgrade_syslog_to_journal(true); - /* Disable the umask logic */ - if (getpid() == 1) + if (getpid() == 1) { + /* Disable the umask logic */ umask(0); + /* Always reopen /dev/console when running as PID 1 or one of its pre-execve() children. This is + * important so that we never end up logging to any foreign stderr, for example if we have to log in a + * child process right before execve()'ing the actual binary, at a point in time where socket + * activation stderr/stdout area already set up. */ + log_set_always_reopen_console(true); + } + if (getpid() == 1 && detect_container() <= 0) { /* Running outside of a container as PID 1 */ -- cgit v1.2.3-54-g00ecf From bcab914f7fb0570eb728907163ada55c6ae3d602 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 21 Feb 2017 18:11:12 +0100 Subject: Revert "basic/strv: allow NULLs to be inserted into strv" This reverts commit 18f71a3c8174774c5386c4aba94d54f3b5c36a84. According to @keszybz we don't need this anymore, hence drop it: https://github.com/systemd/systemd/pull/5131/commits/18f71a3c8174774c5386c4aba94d54f3b5c36a84#r102232368 --- src/basic/strv.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) (limited to 'src') diff --git a/src/basic/strv.c b/src/basic/strv.c index 60f92e6373..0eec868eed 100644 --- a/src/basic/strv.c +++ b/src/basic/strv.c @@ -564,6 +564,9 @@ int strv_extend_front(char ***l, const char *value) { /* Like strv_extend(), but prepends rather than appends the new entry */ + if (!value) + return 0; + n = strv_length(*l); /* Increase and overflow check. */ @@ -571,12 +574,9 @@ int strv_extend_front(char ***l, const char *value) { if (m < n) return -ENOMEM; - if (value) { - v = strdup(value); - if (!v) - return -ENOMEM; - } else - v = NULL; + v = strdup(value); + if (!v) + return -ENOMEM; c = realloc_multiply(*l, sizeof(char*), m); if (!c) { -- cgit v1.2.3-54-g00ecf