From c2cfb12641eb6d47c66ae14d916f0df86de5d8f8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Fri, 31 Jul 2020 15:36:07 +0200 Subject: [PATCH 1/7] NEWS: clarify two points I was reading a summary of changes on Phoronix, and (while not incorrect) those two points were rather misleading. --- NEWS | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/NEWS b/NEWS index 95685ed7f7..ac29a5e0d4 100644 --- a/NEWS +++ b/NEWS @@ -114,7 +114,9 @@ CHANGES WITH 246: * tmpfs mounts automatically created by systemd (/tmp, /run, /dev/shm, and others) now have a size and inode limits applied (50% of RAM for - /tmp and /dev/shm, 10% of RAM for other mounts, etc.) + /tmp and /dev/shm, 10% of RAM for other mounts, etc.). Please note + that the implicit kernel default is 50% too, so there is no change + in the size limit for /tmp and /dev/shm. * nss-mymachines lost support for resolution of users and groups, and now only does resolution of hostnames. This functionality is now @@ -156,12 +158,17 @@ CHANGES WITH 246: now automatically set to "Y" at boot, in order to enable pstore generation for collection with systemd-pstore. - * A new 'hwdb' file has been added that collects information about PCI - and USB devices that correctly support auto-suspend, on top of the - databases for this we import from the ChromiumOS project. If you have - a device that supports auto-suspend correctly and where it should be - enabled by default, please submit a patch that adds it to the - database (see /usr/lib/udev/hwdb.d/60-autosuspend.hwdb). + * We provide a set of udev rules to enable auto-suspend on PCI and USB + devices that were tested to currectly support it. Previously, this + was distributed as a set of udev rules, but has now been replaced by + by a set of hwdb entries (and a much shorter udev rule to take action + if the device modalias matches one of the new hwdb entries). + + As before, entries are periodically imported from the database + maintained by the ChromiumOS project. If you have a device that + supports auto-suspend correctly and where it should be enabled by + default, please submit a patch that adds it to the database (see + /usr/lib/udev/hwdb.d/60-autosuspend.hwdb). * systemd-udevd gained the new configuration option timeout_signal= as well as a corresponding kernel command line option udev.timeout_signal=. From 01ecb3674ad3650bcbb14155b2dcbd4b9f4ed57e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Sat, 1 Aug 2020 11:28:09 +0200 Subject: [PATCH 2/7] analyze-security: do not assign badness to filtered-out syscalls Fixes #16451, https://bugzilla.redhat.com/show_bug.cgi?id=1856273. --- src/analyze/analyze-security.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/analyze/analyze-security.c b/src/analyze/analyze-security.c index d4996c3c65..5356dafbb8 100644 --- a/src/analyze/analyze-security.c +++ b/src/analyze/analyze-security.c @@ -566,7 +566,7 @@ static int assess_system_call_filter( b = 10; } else { (void) asprintf(&d, "System call deny list defined for service, and %s is included", f->name); - b = 5; + b = 0; } } } From a9134af2e3ab22eba96228ea6ba440e89213cef2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Sat, 1 Aug 2020 11:41:57 +0200 Subject: [PATCH 3/7] analyze-security: include an actual syscall name in the message This information was already available in the debug output, but I think it is good to include it in the message in the table. This makes it easier to wrap one's head around the allowlist/denylist filtering. --- src/analyze/analyze-security.c | 33 +++++++++++++++++++++------------ 1 file changed, 21 insertions(+), 12 deletions(-) diff --git a/src/analyze/analyze-security.c b/src/analyze/analyze-security.c index 5356dafbb8..9a822de879 100644 --- a/src/analyze/analyze-security.c +++ b/src/analyze/analyze-security.c @@ -493,7 +493,7 @@ static int assess_system_call_architectures( #if HAVE_SECCOMP -static bool syscall_names_in_filter(Set *s, bool allow_list, const SyscallFilterSet *f) { +static bool syscall_names_in_filter(Set *s, bool allow_list, const SyscallFilterSet *f, const char **ret_offending_syscall) { const char *syscall; NULSTR_FOREACH(syscall, f->value) { @@ -503,7 +503,7 @@ static bool syscall_names_in_filter(Set *s, bool allow_list, const SyscallFilter const SyscallFilterSet *g; assert_se(g = syscall_filter_set_find(syscall)); - if (syscall_names_in_filter(s, allow_list, g)) + if (syscall_names_in_filter(s, allow_list, g, ret_offending_syscall)) return true; /* bad! */ continue; @@ -516,10 +516,13 @@ static bool syscall_names_in_filter(Set *s, bool allow_list, const SyscallFilter if (set_contains(s, syscall) == allow_list) { log_debug("Offending syscall filter item: %s", syscall); + if (ret_offending_syscall) + *ret_offending_syscall = syscall; return true; /* bad! */ } } + *ret_offending_syscall = NULL; return false; } @@ -530,42 +533,48 @@ static int assess_system_call_filter( uint64_t *ret_badness, char **ret_description) { - const SyscallFilterSet *f; - char *d = NULL; - uint64_t b; - assert(a); assert(info); assert(ret_badness); assert(ret_description); assert(a->parameter < _SYSCALL_FILTER_SET_MAX); - f = syscall_filter_sets + a->parameter; + const SyscallFilterSet *f = syscall_filter_sets + a->parameter; + + char *d = NULL; + uint64_t b; if (!info->system_call_filter_allow_list && set_isempty(info->system_call_filter)) { d = strdup("Service does not filter system calls"); b = 10; } else { bool bad; + const char *offender = NULL; log_debug("Analyzing system call filter, checking against: %s", f->name); - bad = syscall_names_in_filter(info->system_call_filter, info->system_call_filter_allow_list, f); + bad = syscall_names_in_filter(info->system_call_filter, info->system_call_filter_allow_list, f, &offender); log_debug("Result: %s", bad ? "bad" : "good"); if (info->system_call_filter_allow_list) { if (bad) { - (void) asprintf(&d, "System call allow list defined for service, and %s is included", f->name); + (void) asprintf(&d, "System call allow list defined for service, and %s is included " + "(e.g. %s is allowed)", + f->name, offender); b = 9; } else { - (void) asprintf(&d, "System call allow list defined for service, and %s is not included", f->name); + (void) asprintf(&d, "System call allow list defined for service, and %s is not included", + f->name); b = 0; } } else { if (bad) { - (void) asprintf(&d, "System call deny list defined for service, and %s is not included", f->name); + (void) asprintf(&d, "System call deny list defined for service, and %s is not included " + "(e.g. %s is allowed)", + f->name, offender); b = 10; } else { - (void) asprintf(&d, "System call deny list defined for service, and %s is included", f->name); + (void) asprintf(&d, "System call deny list defined for service, and %s is included", + f->name); b = 0; } } From b91ae210e62d7630346a9b5545dedc55c7831d95 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Fri, 24 Jul 2020 12:19:08 +0200 Subject: [PATCH 4/7] journal: adjust line about when the journal begins and ends This comes up occasionally with new users. The phrase "Logs begin ..." is ambiguous because it can be taken to mean the logs being displayed or all logs (the intended meaning). Let's rephrase this as "Journal begins ..." to make this clearer. --- man/journalctl.xml | 2 +- man/systemd-run.xml | 4 ++-- src/journal/journalctl.c | 4 ++-- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/man/journalctl.xml b/man/journalctl.xml index 07310d90a1..e562935eea 100644 --- a/man/journalctl.xml +++ b/man/journalctl.xml @@ -453,7 +453,7 @@ Suppresses all informational messages - (i.e. "-- Logs begin at …", "-- Reboot --"), + (i.e. "-- Journal begins at …", "-- Reboot --"), any warning messages regarding inaccessible system journals when run as a normal user. diff --git a/man/systemd-run.xml b/man/systemd-run.xml index a88f60fbb6..c1471e108f 100644 --- a/man/systemd-run.xml +++ b/man/systemd-run.xml @@ -463,11 +463,11 @@ Mon Dec 8 20:44:24 KST 2014 Running as unit: run-71.timer Will run service as unit: run-71.service # journalctl -b -u run-71.timer --- Logs begin at Fri 2014-12-05 19:09:21 KST, end at Mon 2014-12-08 20:44:54 KST. -- +-- Journal begins at Fri 2014-12-05 19:09:21 KST, ends at Mon 2014-12-08 20:44:54 KST. -- Dec 08 20:44:38 container systemd[1]: Starting /bin/touch /tmp/foo. Dec 08 20:44:38 container systemd[1]: Started /bin/touch /tmp/foo. # journalctl -b -u run-71.service --- Logs begin at Fri 2014-12-05 19:09:21 KST, end at Mon 2014-12-08 20:44:54 KST. -- +-- Journal begins at Fri 2014-12-05 19:09:21 KST, ends at Mon 2014-12-08 20:44:54 KST. -- Dec 08 20:44:48 container systemd[1]: Starting /bin/touch /tmp/foo... Dec 08 20:44:48 container systemd[1]: Started /bin/touch /tmp/foo. diff --git a/src/journal/journalctl.c b/src/journal/journalctl.c index 8d4897b942..7613cb0b53 100644 --- a/src/journal/journalctl.c +++ b/src/journal/journalctl.c @@ -2562,10 +2562,10 @@ int main(int argc, char *argv[]) { if (r > 0) { if (arg_follow) - printf("-- Logs begin at %s. --\n", + printf("-- Journal begins at %s. --\n", format_timestamp_maybe_utc(start_buf, sizeof(start_buf), start)); else - printf("-- Logs begin at %s, end at %s. --\n", + printf("-- Journal begins at %s, ends at %s. --\n", format_timestamp_maybe_utc(start_buf, sizeof(start_buf), start), format_timestamp_maybe_utc(end_buf, sizeof(end_buf), end)); } From 9a485918db2522c02173df2980a1146d3693d5c1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Thu, 30 Jul 2020 17:00:25 +0200 Subject: [PATCH 5/7] basic/string-util: reduce scope of variables --- src/basic/string-util.h | 16 +++++----------- 1 file changed, 5 insertions(+), 11 deletions(-) diff --git a/src/basic/string-util.h b/src/basic/string-util.h index 09131455bf..cf8c74b822 100644 --- a/src/basic/string-util.h +++ b/src/basic/string-util.h @@ -135,8 +135,8 @@ char *strjoin_real(const char *x, ...) _sentinel_; ({ \ const char *_appendees_[] = { a, __VA_ARGS__ }; \ char *_d_, *_p_; \ - size_t _len_ = 0; \ - size_t _i_; \ + size_t _len_ = 0; \ + size_t _i_; \ for (_i_ = 0; _i_ < ELEMENTSOF(_appendees_) && _appendees_[_i_]; _i_++) \ _len_ += strlen(_appendees_[_i_]); \ _p_ = _d_ = newa(char, _len_ + 1); \ @@ -152,7 +152,6 @@ char *delete_trailing_chars(char *s, const char *bad); char *truncate_nl(char *s); static inline char *skip_leading_chars(const char *s, const char *bad) { - if (!s) return NULL; @@ -231,11 +230,9 @@ REENABLE_WARNING; /* Like startswith(), but operates on arbitrary memory blocks */ static inline void *memory_startswith(const void *p, size_t sz, const char *token) { - size_t n; - assert(token); - n = strlen(token); + size_t n = strlen(token); if (sz < n) return NULL; @@ -251,20 +248,17 @@ static inline void *memory_startswith(const void *p, size_t sz, const char *toke * It works only for ASCII strings. */ static inline void *memory_startswith_no_case(const void *p, size_t sz, const char *token) { - size_t n, i; - assert(token); - n = strlen(token); + size_t n = strlen(token); if (sz < n) return NULL; assert(p); - for (i = 0; i < n; i++) { + for (size_t i = 0; i < n; i++) if (ascii_tolower(((char *)p)[i]) != ascii_tolower(token[i])) return NULL; - } return (uint8_t*) p + n; } From 618727dae5ee7eb18e42be3d520496da3431e70a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Thu, 30 Jul 2020 17:08:39 +0200 Subject: [PATCH 6/7] basic/utf8: rename parameter Every time I was using this function I had to check whether "newline" means that newlines are good or bad. --- src/basic/utf8.c | 4 ++-- src/basic/utf8.h | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/basic/utf8.c b/src/basic/utf8.c index c857185ec8..174075be54 100644 --- a/src/basic/utf8.c +++ b/src/basic/utf8.c @@ -123,7 +123,7 @@ int utf8_encoded_to_unichar(const char *str, char32_t *ret_unichar) { return 0; } -bool utf8_is_printable_newline(const char* str, size_t length, bool newline) { +bool utf8_is_printable_newline(const char* str, size_t length, bool allow_newline) { const char *p; assert(str); @@ -140,7 +140,7 @@ bool utf8_is_printable_newline(const char* str, size_t length, bool newline) { r = utf8_encoded_to_unichar(p, &val); if (r < 0 || unichar_is_control(val) || - (!newline && val == '\n')) + (!allow_newline && val == '\n')) return false; length -= encoded_len; diff --git a/src/basic/utf8.h b/src/basic/utf8.h index 62e99b7280..52b487955b 100644 --- a/src/basic/utf8.h +++ b/src/basic/utf8.h @@ -18,7 +18,7 @@ char *utf8_is_valid(const char *s) _pure_; char *ascii_is_valid(const char *s) _pure_; char *ascii_is_valid_n(const char *str, size_t len); -bool utf8_is_printable_newline(const char* str, size_t length, bool newline) _pure_; +bool utf8_is_printable_newline(const char* str, size_t length, bool allow_newline) _pure_; #define utf8_is_printable(str, length) utf8_is_printable_newline(str, length, true) char *utf8_escape_invalid(const char *s); From 3a193ac62ec294b56c0a8f71525bfb1430c28a26 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Fri, 31 Jul 2020 10:49:57 +0200 Subject: [PATCH 7/7] shared/cgroup-setup: reduce scope of variables --- src/shared/cgroup-setup.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/shared/cgroup-setup.c b/src/shared/cgroup-setup.c index e8398cbde5..ce8e54df2e 100644 --- a/src/shared/cgroup-setup.c +++ b/src/shared/cgroup-setup.c @@ -618,8 +618,6 @@ int cg_create_everywhere(CGroupMask supported, CGroupMask mask, const char *path } int cg_attach_everywhere(CGroupMask supported, const char *path, pid_t pid, cg_migrate_callback_t path_callback, void *userdata) { - CGroupController c; - CGroupMask done; int r; r = cg_attach(SYSTEMD_CGROUP_CONTROLLER, path, pid); @@ -633,9 +631,9 @@ int cg_attach_everywhere(CGroupMask supported, const char *path, pid_t pid, cg_m return 0; supported &= CGROUP_MASK_V1; - done = 0; + CGroupMask done = 0; - for (c = 0; c < _CGROUP_CONTROLLER_MAX; c++) { + for (CGroupController c = 0; c < _CGROUP_CONTROLLER_MAX; c++) { CGroupMask bit = CGROUP_CONTROLLER_TO_MASK(c); const char *p = NULL;