From fda8a890460c7c627ce65897ed64929f8a8ef0cb Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 9 Mar 2018 21:35:48 +0100 Subject: [PATCH 1/7] CODING_STYLE: document why O_NONBLOCK makes sense when opening regular files, too --- doc/CODING_STYLE | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/doc/CODING_STYLE b/doc/CODING_STYLE index ae818126cb..66771ef523 100644 --- a/doc/CODING_STYLE +++ b/doc/CODING_STYLE @@ -440,3 +440,13 @@ string, always apply the C-style unescaping fist, followed by the specifier expansion. When doing the reverse, make sure to escape '%' in specifier-style first (i.e. '%' → '%%'), and then do C-style escaping where necessary. + +- It's a good idea to use O_NONBLOCK when opening 'foreign' regular files, i.e + file system objects that are supposed to be regular files whose paths where + specified by the user and hence might actually refer to other types of file + system objects. This is a good idea so that we don't end up blocking on + 'strange' file nodes, for example if the user pointed us to a FIFO or device + node which may block when opening. Moreover even for actual regular files + O_NONBLOCK has a benefit: it bypasses any mandatory lock that might be in + effect on the regular file. If in doubt consider turning off O_NONBLOCK again + after opening. From d9b50610e4e5b3f91b22030506477d8a4aa7ee63 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 20 Mar 2018 20:26:15 +0100 Subject: [PATCH 2/7] update TODO --- TODO | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/TODO b/TODO index 14955d7268..35fcaef76b 100644 --- a/TODO +++ b/TODO @@ -24,6 +24,16 @@ Janitorial Clean-ups: Features: +* nspawn's console TTY should be allocated from within the container, not + mounted in from the outside + +* show invocation ID in systemd-run output + +* bypass SIGTERM state in unit files if KillSignal is SIGKILL + +* tree-wide: ensure we always block the signals we hook into with + sd_event_add_signal() first + * add proper dbus APIs for the various sd_notify() commands, such as MAINPID=1 and so on, which would mean we could report errors and such. From cf852ff63b70537bf0e72654381694c6aee97f77 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 26 Mar 2018 13:27:26 +0200 Subject: [PATCH 3/7] journald: make use of fd_get_path() where appropriate --- src/journal/journald-native.c | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/src/journal/journald-native.c b/src/journal/journald-native.c index cee873215d..6b429bc2c7 100644 --- a/src/journal/journald-native.c +++ b/src/journal/journald-native.c @@ -343,20 +343,15 @@ void server_process_native_file( sealed = memfd_get_sealed(fd) > 0; if (!sealed && (!ucred || ucred->uid != 0)) { - _cleanup_free_ char *sl = NULL, *k = NULL; + _cleanup_free_ char *k = NULL; const char *e; /* If this is not a sealed memfd, and the peer is unknown or * unprivileged, then verify the path. */ - if (asprintf(&sl, "/proc/self/fd/%i", fd) < 0) { - log_oom(); - return; - } - - r = readlink_malloc(sl, &k); + r = fd_get_path(fd, &k); if (r < 0) { - log_error_errno(r, "readlink(%s) failed: %m", sl); + log_error_errno(r, "readlink(/proc/self/fd/%i) failed: %m", fd); return; } From 2901f4b31ffee670f9d3a97676a2b99eb04297d5 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 27 Mar 2018 14:56:04 +0200 Subject: [PATCH 4/7] util: check for overflows in xbsearch_r() --- src/basic/util.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/basic/util.c b/src/basic/util.c index 7863a14fb2..b0303a17fa 100644 --- a/src/basic/util.c +++ b/src/basic/util.c @@ -181,11 +181,13 @@ void *xbsearch_r(const void *key, const void *base, size_t nmemb, size_t size, const void *p; int comparison; + assert(!size_multiply_overflow(nmemb, size)); + l = 0; u = nmemb; while (l < u) { idx = (l + u) / 2; - p = (const char *) base + idx * size; + p = (const uint8_t*) base + idx * size; comparison = compar(key, p, arg); if (comparison < 0) u = idx; From 4c0e48f9d75a331408c92041e52f865446480b63 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 27 Mar 2018 14:58:21 +0200 Subject: [PATCH 5/7] inhibit: make use of EXIT_SUCCESS where appropriate --- src/login/inhibit.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/login/inhibit.c b/src/login/inhibit.c index c118421e6b..ba4e261f3d 100644 --- a/src/login/inhibit.c +++ b/src/login/inhibit.c @@ -284,5 +284,5 @@ int main(int argc, char *argv[]) { return r < 0 ? EXIT_FAILURE : r; } - return 0; + return EXIT_SUCCESS; } From 39e4f5bc7bdb9c47f500a5829223238c9ae7b1f7 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 28 Mar 2018 17:07:11 +0200 Subject: [PATCH 6/7] man: remove spurious 'system' --- man/bootup.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/man/bootup.xml b/man/bootup.xml index 565dda93cb..60311b78f9 100644 --- a/man/bootup.xml +++ b/man/bootup.xml @@ -301,7 +301,7 @@ systemd-reboot.service systemd-poweroff.service systemd-halt.service syste Commonly used system shutdown targets are emphasized. Note that - systemsystemd-halt.service8, + systemd-halt.service8, systemd-reboot.service, systemd-poweroff.service and systemd-kexec.service will transition the system and server manager (PID 1) into the second phase of system shutdown (implemented in the systemd-shutdown binary), which will unmount any From 771b7ead842130b27508eee04d568834741aef78 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 28 Mar 2018 17:07:42 +0200 Subject: [PATCH 7/7] machine-image,mount-setup: minor coding style fixes --- src/core/mount-setup.c | 13 ++++++------- src/shared/machine-image.c | 3 +-- 2 files changed, 7 insertions(+), 9 deletions(-) diff --git a/src/core/mount-setup.c b/src/core/mount-setup.c index a0b5bff5a7..6916c63b2d 100644 --- a/src/core/mount-setup.c +++ b/src/core/mount-setup.c @@ -397,19 +397,18 @@ static int relabel_cgroup_filesystems(void) { only when the filesystem has been already populated by a previous instance of systemd running from initrd. Otherwise don't remount anything and leave the filesystem read-write for the cgroup filesystems to be mounted inside. */ - r = statfs("/sys/fs/cgroup", &st); - if (r < 0) { + if (statfs("/sys/fs/cgroup", &st) < 0) return log_error_errno(errno, "Failed to determine mount flags for /sys/fs/cgroup: %m"); - } if (st.f_flags & ST_RDONLY) (void) mount(NULL, "/sys/fs/cgroup", NULL, MS_REMOUNT, NULL); (void) label_fix("/sys/fs/cgroup", 0); - nftw("/sys/fs/cgroup", nftw_cb, 64, FTW_MOUNT|FTW_PHYS|FTW_ACTIONRETVAL); + (void) nftw("/sys/fs/cgroup", nftw_cb, 64, FTW_MOUNT|FTW_PHYS|FTW_ACTIONRETVAL); if (st.f_flags & ST_RDONLY) (void) mount(NULL, "/sys/fs/cgroup", NULL, MS_REMOUNT|MS_RDONLY, NULL); + } else if (r < 0) return log_error_errno(r, "Failed to determine whether we are in all unified mode: %m"); @@ -435,9 +434,9 @@ int mount_setup(bool loaded_policy) { before_relabel = now(CLOCK_MONOTONIC); - nftw("/dev", nftw_cb, 64, FTW_MOUNT|FTW_PHYS|FTW_ACTIONRETVAL); - nftw("/dev/shm", nftw_cb, 64, FTW_MOUNT|FTW_PHYS|FTW_ACTIONRETVAL); - nftw("/run", nftw_cb, 64, FTW_MOUNT|FTW_PHYS|FTW_ACTIONRETVAL); + (void) nftw("/dev", nftw_cb, 64, FTW_MOUNT|FTW_PHYS|FTW_ACTIONRETVAL); + (void) nftw("/dev/shm", nftw_cb, 64, FTW_MOUNT|FTW_PHYS|FTW_ACTIONRETVAL); + (void) nftw("/run", nftw_cb, 64, FTW_MOUNT|FTW_PHYS|FTW_ACTIONRETVAL); r = relabel_cgroup_filesystems(); if (r < 0) diff --git a/src/shared/machine-image.c b/src/shared/machine-image.c index faa2eb0343..89fd8c5dba 100644 --- a/src/shared/machine-image.c +++ b/src/shared/machine-image.c @@ -879,8 +879,7 @@ int image_path_lock(const char *path, int operation, LockFile *global, LockFile r = make_lock_file_for(path, operation, &t); if (r < 0) { if ((operation & LOCK_SH) && r == -EROFS) - log_debug_errno(r, "Failed to create shared " - "lock for %s: %m", path); + log_debug_errno(r, "Failed to create shared lock for '%s', ignoring: %m", path); else return r; }