From 6c34ed511173ae830784e4e49dc97a4d925451d2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Wed, 3 Apr 2019 08:19:08 +0200 Subject: [PATCH 1/4] test: use newer verb to set log levels --- test/TEST-20-MAINPIDGAMES/testsuite.sh | 6 +++--- test/TEST-23-TYPE-EXEC/testsuite.sh | 6 +++--- test/TEST-27-STDOUTFILE/testsuite.sh | 6 +++--- test/TEST-30-ONCLOCKCHANGE/testsuite.sh | 6 +++--- test/networkd-test.py | 4 ++-- 5 files changed, 14 insertions(+), 14 deletions(-) diff --git a/test/TEST-20-MAINPIDGAMES/testsuite.sh b/test/TEST-20-MAINPIDGAMES/testsuite.sh index 0e1a116b07f..b183ec9a6b0 100755 --- a/test/TEST-20-MAINPIDGAMES/testsuite.sh +++ b/test/TEST-20-MAINPIDGAMES/testsuite.sh @@ -4,8 +4,8 @@ set -ex set -o pipefail -systemd-analyze set-log-level debug -systemd-analyze set-log-target console +systemd-analyze log-level debug +systemd-analyze log-target console test `systemctl show -p MainPID --value testsuite.service` -eq $$ @@ -134,7 +134,7 @@ chmod 755 /dev/shm/mainpid3.sh # Test that this failed due to timeout, and not some other error test `systemctl show -p Result --value mainpidsh3.service` = timeout -systemd-analyze set-log-level info +systemd-analyze log-level info echo OK > /testok diff --git a/test/TEST-23-TYPE-EXEC/testsuite.sh b/test/TEST-23-TYPE-EXEC/testsuite.sh index 80734bbbdc3..4f7cc44310f 100755 --- a/test/TEST-23-TYPE-EXEC/testsuite.sh +++ b/test/TEST-23-TYPE-EXEC/testsuite.sh @@ -4,8 +4,8 @@ set -ex set -o pipefail -systemd-analyze set-log-level debug -systemd-analyze set-log-target console +systemd-analyze log-level debug +systemd-analyze log-target console # Create a binary for which execve() will fail touch /tmp/brokenbinary @@ -21,7 +21,7 @@ systemd-run --unit=four -p Type=exec /bin/sleep infinity ! systemd-run --unit=five -p Type=exec -p User=idontexist /bin/sleep infinity ! systemd-run --unit=six -p Type=exec /tmp/brokenbinary -systemd-analyze set-log-level info +systemd-analyze log-level info echo OK > /testok diff --git a/test/TEST-27-STDOUTFILE/testsuite.sh b/test/TEST-27-STDOUTFILE/testsuite.sh index b8fcc46e336..6975b7966f2 100755 --- a/test/TEST-27-STDOUTFILE/testsuite.sh +++ b/test/TEST-27-STDOUTFILE/testsuite.sh @@ -4,8 +4,8 @@ set -ex set -o pipefail -systemd-analyze set-log-level debug -systemd-analyze set-log-target console +systemd-analyze log-level debug +systemd-analyze log-target console systemd-run --wait --unit=one -p StandardOutput=file:/tmp/stdout -p StandardError=file:/tmp/stderr -p Type=exec sh -c 'echo x ; echo y >&2' cmp /tmp/stdout < /testok diff --git a/test/TEST-30-ONCLOCKCHANGE/testsuite.sh b/test/TEST-30-ONCLOCKCHANGE/testsuite.sh index 8ded85633af..3f7c2640005 100755 --- a/test/TEST-30-ONCLOCKCHANGE/testsuite.sh +++ b/test/TEST-30-ONCLOCKCHANGE/testsuite.sh @@ -4,8 +4,8 @@ set -ex set -o pipefail -systemd-analyze set-log-level debug -systemd-analyze set-log-target console +systemd-analyze log-level debug +systemd-analyze log-target console systemctl disable --now systemd-timesyncd.service @@ -26,7 +26,7 @@ timedatectl set-time 2018-1-1 while ! test -f /tmp/clock-changed ; do sleep .5 ; done -systemd-analyze set-log-level info +systemd-analyze log-level info echo OK > /testok diff --git a/test/networkd-test.py b/test/networkd-test.py index 9af141fa654..8b1aeeda35b 100755 --- a/test/networkd-test.py +++ b/test/networkd-test.py @@ -283,11 +283,11 @@ class ClientTestBase(NetworkdTestingUtilities): klass.orig_log_level = subprocess.check_output( ['systemctl', 'show', '--value', '--property', 'LogLevel'], universal_newlines=True).strip() - subprocess.check_call(['systemd-analyze', 'set-log-level', 'debug']) + subprocess.check_call(['systemd-analyze', 'log-level', 'debug']) @classmethod def tearDownClass(klass): - subprocess.check_call(['systemd-analyze', 'set-log-level', klass.orig_log_level]) + subprocess.check_call(['systemd-analyze', 'log-level', klass.orig_log_level]) def setUp(self): self.iface = 'test_eth42' From e7ccdfa8099711b76e63bba7ccb3d3ddafb30361 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Wed, 3 Apr 2019 08:56:06 +0200 Subject: [PATCH 2/4] core: use a temporary variable for calculation of seccomp flags I think it is easier to read this way. --- src/core/dbus-execute.c | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/src/core/dbus-execute.c b/src/core/dbus-execute.c index 653847acad4..1f704127322 100644 --- a/src/core/dbus-execute.c +++ b/src/core/dbus-execute.c @@ -1395,7 +1395,7 @@ int bus_exec_context_set_transient_property( if (!UNIT_WRITE_FLAGS_NOOP(flags)) { _cleanup_free_ char *joined = NULL; - bool invert = !whitelist; + SeccompParseFlags invert_flag = whitelist ? 0 : SECCOMP_PARSE_INVERT; char **s; if (strv_isempty(l)) { @@ -1414,7 +1414,10 @@ int bus_exec_context_set_transient_property( c->syscall_whitelist = whitelist; if (c->syscall_whitelist) { - r = seccomp_parse_syscall_filter("@default", -1, c->syscall_filter, SECCOMP_PARSE_WHITELIST | (invert ? SECCOMP_PARSE_INVERT : 0)); + r = seccomp_parse_syscall_filter("@default", + -1, + c->syscall_filter, + SECCOMP_PARSE_WHITELIST | invert_flag); if (r < 0) return r; } @@ -1428,7 +1431,10 @@ int bus_exec_context_set_transient_property( if (r < 0) return r; - r = seccomp_parse_syscall_filter(n, e, c->syscall_filter, (invert ? SECCOMP_PARSE_INVERT : 0) | (c->syscall_whitelist ? SECCOMP_PARSE_WHITELIST : 0)); + r = seccomp_parse_syscall_filter(n, + e, + c->syscall_filter, + (c->syscall_whitelist ? SECCOMP_PARSE_WHITELIST : 0) | invert_flag); if (r < 0) return r; } @@ -1506,7 +1512,6 @@ int bus_exec_context_set_transient_property( if (!UNIT_WRITE_FLAGS_NOOP(flags)) { _cleanup_free_ char *joined = NULL; - bool invert = !whitelist; char **s; if (strv_isempty(l)) { @@ -1532,7 +1537,7 @@ int bus_exec_context_set_transient_property( if (af < 0) return af; - if (!invert == c->address_families_whitelist) { + if (whitelist == c->address_families_whitelist) { r = set_put(c->address_families, INT_TO_PTR(af)); if (r < 0) return r; From 6bfb1daff14344b74c034238980390190d4faa69 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Wed, 3 Apr 2019 09:13:37 +0200 Subject: [PATCH 3/4] basic/log: log any available location information in log_syntax() We would log "(null):0: Failed to parse system call, ignoring: rseq" from log_syntax_internal() from log_syntax() from seccomp_parse_syscall_filter_full() from seccomp_parse_syscall_filter() from config_parse_syscall_filter(), when generating the built-in @default whitelist. Since it was not based on the unit file, we would not pass a file name. So let's make sure that log_syntax() does not print "(null)" pointer (which is iffy and ugly), and use the unit name as fallback or nothing if both are missing. In principle, one of the two should be always available, since why use log_syntax() otherwise, but let's make things more resilient by guarding against this case too. log_syntax() is called from a thousand places, and often in error path, so it's hard to verify all callers. --- src/basic/log.c | 38 ++++++++++++++++++++++++++++---------- 1 file changed, 28 insertions(+), 10 deletions(-) diff --git a/src/basic/log.c b/src/basic/log.c index 37a5ffa74f9..ea252c41302 100644 --- a/src/basic/log.c +++ b/src/basic/log.c @@ -1231,16 +1231,34 @@ int log_syntax_internal( if (unit) unit_fmt = getpid_cached() == 1 ? "UNIT=%s" : "USER_UNIT=%s"; - return log_struct_internal( - LOG_REALM_PLUS_LEVEL(LOG_REALM_SYSTEMD, level), - error, - file, line, func, - "MESSAGE_ID=" SD_MESSAGE_INVALID_CONFIGURATION_STR, - "CONFIG_FILE=%s", config_file, - "CONFIG_LINE=%u", config_line, - LOG_MESSAGE("%s:%u: %s", config_file, config_line, buffer), - unit_fmt, unit, - NULL); + if (config_file) + return log_struct_internal( + LOG_REALM_PLUS_LEVEL(LOG_REALM_SYSTEMD, level), + error, + file, line, func, + "MESSAGE_ID=" SD_MESSAGE_INVALID_CONFIGURATION_STR, + "CONFIG_FILE=%s", config_file, + "CONFIG_LINE=%u", config_line, + LOG_MESSAGE("%s:%u: %s", config_file, config_line, buffer), + unit_fmt, unit, + NULL); + else if (unit) + return log_struct_internal( + LOG_REALM_PLUS_LEVEL(LOG_REALM_SYSTEMD, level), + error, + file, line, func, + "MESSAGE_ID=" SD_MESSAGE_INVALID_CONFIGURATION_STR, + LOG_MESSAGE("%s: %s", unit, buffer), + unit_fmt, unit, + NULL); + else + return log_struct_internal( + LOG_REALM_PLUS_LEVEL(LOG_REALM_SYSTEMD, level), + error, + file, line, func, + "MESSAGE_ID=" SD_MESSAGE_INVALID_CONFIGURATION_STR, + LOG_MESSAGE("%s", buffer), + NULL); } int log_syntax_invalid_utf8_internal( From 58f6ab4454fbd2ac440f97ce4a230a5b57dd5392 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Wed, 3 Apr 2019 09:17:42 +0200 Subject: [PATCH 4/4] pid1: pass unit name to seccomp parser when we have no file location Building on previous commit, let's pass the unit name when parsing dbus message or builtin whitelist, which is better than nothing. seccomp_parse_syscall_filter() is not needed anymore, so it is removed, and seccomp_parse_syscall_filter_full() is renamed to take its place. --- src/core/dbus-execute.c | 8 ++++++-- src/core/load-fragment.c | 6 ++++-- src/shared/seccomp-util.c | 4 ++-- src/shared/seccomp-util.h | 14 +++++++------- 4 files changed, 19 insertions(+), 13 deletions(-) diff --git a/src/core/dbus-execute.c b/src/core/dbus-execute.c index 1f704127322..5532d1ada93 100644 --- a/src/core/dbus-execute.c +++ b/src/core/dbus-execute.c @@ -1417,7 +1417,9 @@ int bus_exec_context_set_transient_property( r = seccomp_parse_syscall_filter("@default", -1, c->syscall_filter, - SECCOMP_PARSE_WHITELIST | invert_flag); + SECCOMP_PARSE_WHITELIST | invert_flag, + u->id, + NULL, 0); if (r < 0) return r; } @@ -1434,7 +1436,9 @@ int bus_exec_context_set_transient_property( r = seccomp_parse_syscall_filter(n, e, c->syscall_filter, - (c->syscall_whitelist ? SECCOMP_PARSE_WHITELIST : 0) | invert_flag); + (c->syscall_whitelist ? SECCOMP_PARSE_WHITELIST : 0) | invert_flag, + u->id, + NULL, 0); if (r < 0) return r; } diff --git a/src/core/load-fragment.c b/src/core/load-fragment.c index 6acfd9b8ea4..9b7c8cba672 100644 --- a/src/core/load-fragment.c +++ b/src/core/load-fragment.c @@ -2735,7 +2735,9 @@ int config_parse_syscall_filter( /* Accept default syscalls if we are on a whitelist */ r = seccomp_parse_syscall_filter( "@default", -1, c->syscall_filter, - SECCOMP_PARSE_PERMISSIVE|SECCOMP_PARSE_WHITELIST); + SECCOMP_PARSE_PERMISSIVE|SECCOMP_PARSE_WHITELIST, + unit, + NULL, 0); if (r < 0) return r; } @@ -2762,7 +2764,7 @@ int config_parse_syscall_filter( continue; } - r = seccomp_parse_syscall_filter_full( + r = seccomp_parse_syscall_filter( name, num, c->syscall_filter, SECCOMP_PARSE_LOG|SECCOMP_PARSE_PERMISSIVE| (invert ? SECCOMP_PARSE_INVERT : 0)| diff --git a/src/shared/seccomp-util.c b/src/shared/seccomp-util.c index 7a179998bdb..aab1feadb3c 100644 --- a/src/shared/seccomp-util.c +++ b/src/shared/seccomp-util.c @@ -1016,7 +1016,7 @@ int seccomp_load_syscall_filter_set_raw(uint32_t default_action, Hashmap* set, u return 0; } -int seccomp_parse_syscall_filter_full( +int seccomp_parse_syscall_filter( const char *name, int errno_num, Hashmap *filter, @@ -1049,7 +1049,7 @@ int seccomp_parse_syscall_filter_full( * away the SECCOMP_PARSE_LOG flag) since any issues in the group table are our own problem, * not a problem in user configuration data and we shouldn't pretend otherwise by complaining * about them. */ - r = seccomp_parse_syscall_filter_full(i, errno_num, filter, flags &~ SECCOMP_PARSE_LOG, unit, filename, line); + r = seccomp_parse_syscall_filter(i, errno_num, filter, flags &~ SECCOMP_PARSE_LOG, unit, filename, line); if (r < 0) return r; } diff --git a/src/shared/seccomp-util.h b/src/shared/seccomp-util.h index 31c6c211fd2..14dbc42691e 100644 --- a/src/shared/seccomp-util.h +++ b/src/shared/seccomp-util.h @@ -70,13 +70,13 @@ typedef enum SeccompParseFlags { SECCOMP_PARSE_PERMISSIVE = 1 << 3, } SeccompParseFlags; -int seccomp_parse_syscall_filter_full( - const char *name, int errno_num, Hashmap *filter, SeccompParseFlags flags, - const char *unit, const char *filename, unsigned line); - -static inline int seccomp_parse_syscall_filter(const char *name, int errno_num, Hashmap *filter, SeccompParseFlags flags) { - return seccomp_parse_syscall_filter_full(name, errno_num, filter, flags, NULL, NULL, 0); -} +int seccomp_parse_syscall_filter( + const char *name, + int errno_num, + Hashmap *filter, + SeccompParseFlags flags, + const char *unit, + const char *filename, unsigned line); int seccomp_restrict_archs(Set *archs); int seccomp_restrict_namespaces(unsigned long retain);