From ca82f0cbe2db096bc7ff81280b5683ea1beae534 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20Koutn=C3=BD?= Date: Wed, 15 Jan 2025 16:36:28 +0100 Subject: [PATCH 1/3] test-cgroup-util: Check return values The test is supposed to check a battery of cgroup helpers on each process found but it doesn't literally check anything besides presence of procfs. (One can visually check printed output only. Introduction in aff38e74bd ("nspawn: suffix the nspawn cgroups with ".nspawn"").) Make some assumptions about visible processes and turn the test into testing that systemd helpers can deal with whatever process they find on the SUT. Reported by Coverity and Fixes CID#1587767 --- src/test/test-cgroup-util.c | 22 ++++++++++++++-------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/src/test/test-cgroup-util.c b/src/test/test-cgroup-util.c index 3f91aa6a72..6b4c9b5aa7 100644 --- a/src/test/test-cgroup-util.c +++ b/src/test/test-cgroup-util.c @@ -218,14 +218,20 @@ TEST(proc) { if (pidref_is_kernel_thread(&pid) != 0) continue; - cg_pidref_get_path(SYSTEMD_CGROUP_CONTROLLER, &pid, &path); - cg_pid_get_path_shifted(pid.pid, NULL, &path_shifted); - cg_pidref_get_owner_uid(&pid, &uid); - cg_pidref_get_session(&pid, &session); - cg_pidref_get_unit(&pid, &unit); - cg_pid_get_user_unit(pid.pid, &user_unit); - cg_pid_get_machine_name(pid.pid, &machine); - cg_pid_get_slice(pid.pid, &slice); + ASSERT_OK_ZERO(cg_pidref_get_path(SYSTEMD_CGROUP_CONTROLLER, &pid, &path)); + ASSERT_OK_ZERO(cg_pid_get_path_shifted(pid.pid, NULL, &path_shifted)); + ASSERT_OK_ZERO(cg_pidref_get_unit(&pid, &unit)); + ASSERT_OK_ZERO(cg_pid_get_slice(pid.pid, &slice)); + + /* Not all processes belong to a specific user or a machine */ + r = cg_pidref_get_owner_uid(&pid, &uid); + ASSERT_TRUE(r == 0 || r == -ENXIO); + r = cg_pidref_get_session(&pid, &session); + ASSERT_TRUE(r == 0 || r == -ENXIO); + r = cg_pid_get_user_unit(pid.pid, &user_unit); + ASSERT_TRUE(r == 0 || r == -ENXIO); + r = cg_pid_get_machine_name(pid.pid, &machine); + ASSERT_TRUE(r == 0 || r == -ENOENT); printf(PID_FMT"\t%s\t%s\t"UID_FMT"\t%s\t%s\t%s\t%s\t%s\n", pid.pid, From b78003c793c0abb04e33100e0035df5de7725c9a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20Koutn=C3=BD?= Date: Fri, 17 Jan 2025 18:00:25 +0100 Subject: [PATCH 2/3] test-cgroup-util: Skip procs analysis without cgroupfs cg_pidref_get_path() cannot work (current implementaion) without cgroupfs (when it checks unified or not setup). Similarly, cg_pidref_get_unit() assumes all processes are part of a unit. So carry out the test only when running on a systemd setup. --- src/test/test-cgroup-util.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/test-cgroup-util.c b/src/test/test-cgroup-util.c index 6b4c9b5aa7..8743bd1cc7 100644 --- a/src/test/test-cgroup-util.c +++ b/src/test/test-cgroup-util.c @@ -198,7 +198,7 @@ TEST(get_paths, .sd_booted = true) { log_info("Root = %s", a); } -TEST(proc) { +TEST(proc, .sd_booted = true) { _cleanup_closedir_ DIR *d = NULL; int r; From b67a45841fa3889ed6790c3f4e2aafb10d921643 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20Koutn=C3=BD?= Date: Mon, 3 Feb 2025 17:02:09 +0100 Subject: [PATCH 3/3] test-cgroup-util: Ignore LXC group LXC helper processes hide themselve in .lxc cgroup, we don't have to deal with the inside tests (and the error in conversion to unit is handled). Skip those but keep iterating over remaining processes to detect what can be created around us. --- src/test/test-cgroup-util.c | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/src/test/test-cgroup-util.c b/src/test/test-cgroup-util.c index 8743bd1cc7..cdf2ea83d7 100644 --- a/src/test/test-cgroup-util.c +++ b/src/test/test-cgroup-util.c @@ -198,6 +198,13 @@ TEST(get_paths, .sd_booted = true) { log_info("Root = %s", a); } +static inline bool hidden_cgroup(const char *p) { + assert(p); + + /* Consider top-level cgroup hidden from us */ + return p[0] == '/' && p[strspn(p, "/")] == '.'; +} + TEST(proc, .sd_booted = true) { _cleanup_closedir_ DIR *d = NULL; int r; @@ -219,6 +226,10 @@ TEST(proc, .sd_booted = true) { continue; ASSERT_OK_ZERO(cg_pidref_get_path(SYSTEMD_CGROUP_CONTROLLER, &pid, &path)); + /* Test may run in a container with supervising/monitor processes that don't belong to our + * cgroup tree (slices/leaves) */ + if (hidden_cgroup(path)) + continue; ASSERT_OK_ZERO(cg_pid_get_path_shifted(pid.pid, NULL, &path_shifted)); ASSERT_OK_ZERO(cg_pidref_get_unit(&pid, &unit)); ASSERT_OK_ZERO(cg_pid_get_slice(pid.pid, &slice));