From 5cacac04fa7a7f2203a13a69d41201c3771a719f Mon Sep 17 00:00:00 2001 From: Mike Yuan Date: Thu, 23 Nov 2023 20:42:11 +0800 Subject: [PATCH 1/5] core/manager: rename result parameter to ret --- src/core/manager.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/core/manager.c b/src/core/manager.c index e61ebee253..8c8a25826e 100644 --- a/src/core/manager.c +++ b/src/core/manager.c @@ -847,12 +847,12 @@ static int compare_job_priority(const void *a, const void *b) { return unit_compare_priority(x->unit, y->unit); } -int manager_new(RuntimeScope runtime_scope, ManagerTestRunFlags test_run_flags, Manager **_m) { +int manager_new(RuntimeScope runtime_scope, ManagerTestRunFlags test_run_flags, Manager **ret) { _cleanup_(manager_freep) Manager *m = NULL; int r; - assert(_m); assert(IN_SET(runtime_scope, RUNTIME_SCOPE_SYSTEM, RUNTIME_SCOPE_USER)); + assert(ret); m = new(Manager, 1); if (!m) @@ -1054,7 +1054,7 @@ int manager_new(RuntimeScope runtime_scope, ManagerTestRunFlags test_run_flags, /* Note that we do not set up the notify fd here. We do that after deserialization, * since they might have gotten serialized across the reexec. */ - *_m = TAKE_PTR(m); + *ret = TAKE_PTR(m); return 0; } From 9e743e7ea93efea2e20eab38d5bf51948336887d Mon Sep 17 00:00:00 2001 From: Mike Yuan Date: Thu, 23 Nov 2023 20:38:29 +0800 Subject: [PATCH 2/5] core/manager: correct and simplify errno handling open_parent() is our own function that returns negative errno. --- src/core/manager.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/core/manager.c b/src/core/manager.c index 8c8a25826e..dadfc51369 100644 --- a/src/core/manager.c +++ b/src/core/manager.c @@ -1032,23 +1032,23 @@ int manager_new(RuntimeScope runtime_scope, ManagerTestRunFlags test_run_flags, self_dir_fd = open_parent(self_exe, O_CLOEXEC|O_DIRECTORY, 0); if (self_dir_fd < 0) - return -errno; + return self_dir_fd; - m->executor_fd = openat(self_dir_fd, "systemd-executor", O_CLOEXEC|O_PATH); - if (m->executor_fd < 0 && errno == ENOENT) - m->executor_fd = openat(AT_FDCWD, "systemd-executor", O_CLOEXEC|O_PATH); - if (m->executor_fd < 0 && errno == ENOENT) { - m->executor_fd = open(SYSTEMD_EXECUTOR_BINARY_PATH, O_CLOEXEC|O_PATH); + m->executor_fd = RET_NERRNO(openat(self_dir_fd, "systemd-executor", O_CLOEXEC|O_PATH)); + if (m->executor_fd == -ENOENT) + m->executor_fd = RET_NERRNO(openat(AT_FDCWD, "systemd-executor", O_CLOEXEC|O_PATH)); + if (m->executor_fd == -ENOENT) { + m->executor_fd = RET_NERRNO(open(SYSTEMD_EXECUTOR_BINARY_PATH, O_CLOEXEC|O_PATH)); level = LOG_WARNING; /* Tests should normally use local builds */ } if (m->executor_fd < 0) - return -errno; + return m->executor_fd; r = fd_get_path(m->executor_fd, &executor_path); if (r < 0) return r; - log_full(level, "Using systemd-executor binary from '%s'", executor_path); + log_full(level, "Using systemd-executor binary from '%s'.", executor_path); } /* Note that we do not set up the notify fd here. We do that after deserialization, From 57d3a6aaf829bbf244ffa39a16656f71ea997c7e Mon Sep 17 00:00:00 2001 From: Mike Yuan Date: Thu, 23 Nov 2023 20:44:32 +0800 Subject: [PATCH 3/5] core/manager: open our parent dir with O_PATH --- src/core/manager.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/core/manager.c b/src/core/manager.c index dadfc51369..37e4f70950 100644 --- a/src/core/manager.c +++ b/src/core/manager.c @@ -1030,7 +1030,7 @@ int manager_new(RuntimeScope runtime_scope, ManagerTestRunFlags test_run_flags, if (r < 0) return r; - self_dir_fd = open_parent(self_exe, O_CLOEXEC|O_DIRECTORY, 0); + self_dir_fd = open_parent(self_exe, O_CLOEXEC|O_PATH|O_DIRECTORY, 0); if (self_dir_fd < 0) return self_dir_fd; From 9136de69342b5ff03547e0c10d64bd28d0bd6b21 Mon Sep 17 00:00:00 2001 From: Mike Yuan Date: Thu, 23 Nov 2023 20:52:16 +0800 Subject: [PATCH 4/5] analyze: use FOREACH_ARRAY more --- src/analyze/analyze-verify-util.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/analyze/analyze-verify-util.c b/src/analyze/analyze-verify-util.c index ae4164d053..5bba388dfc 100644 --- a/src/analyze/analyze-verify-util.c +++ b/src/analyze/analyze-verify-util.c @@ -168,12 +168,12 @@ static int verify_executables(Unit *u, const char *root) { RET_GATHER(r, verify_executable(u, exec, root)); if (u->type == UNIT_SERVICE) - for (unsigned i = 0; i < ELEMENTSOF(SERVICE(u)->exec_command); i++) - RET_GATHER(r, verify_executable(u, SERVICE(u)->exec_command[i], root)); + FOREACH_ARRAY(i, SERVICE(u)->exec_command, ELEMENTSOF(SERVICE(u)->exec_command)) + RET_GATHER(r, verify_executable(u, *i, root)); if (u->type == UNIT_SOCKET) - for (unsigned i = 0; i < ELEMENTSOF(SOCKET(u)->exec_command); i++) - RET_GATHER(r, verify_executable(u, SOCKET(u)->exec_command[i], root)); + FOREACH_ARRAY(i, SOCKET(u)->exec_command, ELEMENTSOF(SOCKET(u)->exec_command)) + RET_GATHER(r, verify_executable(u, *i, root)); return r; } @@ -225,7 +225,7 @@ static int verify_unit(Unit *u, bool check_man, const char *root) { return r; } -static void set_destroy_ignore_pointer_max(Set** s) { +static void set_destroy_ignore_pointer_max(Set **s) { if (*s == POINTER_MAX) return; set_free_free(*s); @@ -302,8 +302,8 @@ int verify_units( count++; } - for (int i = 0; i < count; i++) - RET_GATHER(r, verify_unit(units[i], check_man, root)); + FOREACH_ARRAY(i, units, count) + RET_GATHER(r, verify_unit(*i, check_man, root)); if (s == POINTER_MAX) return log_oom(); From bed6bffc9a9f1bdc389c832698e9d4c2778949c5 Mon Sep 17 00:00:00 2001 From: Mike Yuan Date: Thu, 23 Nov 2023 20:53:50 +0800 Subject: [PATCH 5/5] analyze: don't open systemd-executor needlessly Fixes #30162 --- src/analyze/analyze-condition.c | 2 +- src/analyze/analyze-security.c | 1 + src/analyze/analyze-verify-util.c | 1 + 3 files changed, 3 insertions(+), 1 deletion(-) diff --git a/src/analyze/analyze-condition.c b/src/analyze/analyze-condition.c index f5d874b9c7..3b290e0323 100644 --- a/src/analyze/analyze-condition.c +++ b/src/analyze/analyze-condition.c @@ -92,7 +92,7 @@ static int verify_conditions(char **lines, RuntimeScope scope, const char *unit, assert_se(set_unit_path(var) >= 0); } - r = manager_new(scope, MANAGER_TEST_RUN_MINIMAL, &m); + r = manager_new(scope, MANAGER_TEST_RUN_MINIMAL|MANAGER_TEST_DONT_OPEN_EXECUTOR, &m); if (r < 0) return log_error_errno(r, "Failed to initialize manager: %m"); diff --git a/src/analyze/analyze-security.c b/src/analyze/analyze-security.c index 2745100f5d..b119ad7351 100644 --- a/src/analyze/analyze-security.c +++ b/src/analyze/analyze-security.c @@ -2690,6 +2690,7 @@ static int offline_security_checks( MANAGER_TEST_RUN_MINIMAL | MANAGER_TEST_RUN_ENV_GENERATORS | MANAGER_TEST_RUN_IGNORE_DEPENDENCIES | + MANAGER_TEST_DONT_OPEN_EXECUTOR | run_generators * MANAGER_TEST_RUN_GENERATORS; _cleanup_(manager_freep) Manager *m = NULL; diff --git a/src/analyze/analyze-verify-util.c b/src/analyze/analyze-verify-util.c index 5bba388dfc..622a10f6b0 100644 --- a/src/analyze/analyze-verify-util.c +++ b/src/analyze/analyze-verify-util.c @@ -242,6 +242,7 @@ int verify_units( const ManagerTestRunFlags flags = MANAGER_TEST_RUN_MINIMAL | MANAGER_TEST_RUN_ENV_GENERATORS | + MANAGER_TEST_DONT_OPEN_EXECUTOR | (recursive_errors == RECURSIVE_ERRORS_NO) * MANAGER_TEST_RUN_IGNORE_DEPENDENCIES | run_generators * MANAGER_TEST_RUN_GENERATORS;