From 88631ec5449a805e30cd75cec3526f6b05f91621 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Thu, 7 Apr 2022 12:15:04 +0200 Subject: [PATCH] nspawn: fix comparisons of versions with non-numerical suffixes See a2b0cd3f5ab3f450e74e2085ad20372a05451c74. When -Dshared-lib-tag is used, libsystemd-shared.so and libsystemd-core.so get a suffix which breaks the parsing done by systemd_installation_has_version(). We can assume that the tag will be something like "251-rc1-1.fc37" that is currently used in Fedora. (Anything that does *not* start with the version would be completely crazy.) By switching to strverscmp_improved() we simplify the code and fix comparisons with such versions. $ build/test-nspawn-util /var/lib/machines/rawhide ... Found libsystemd shared at "/var/lib/machines/rawhide/lib/systemd/libsystemd-shared-251-rc1-1.fc37.so.so", version 251-rc1-1.fc37 (OK). /var/lib/machines/rawhide has systemd >= 251: yes ... I noticed this when I started a systemd-nspawn container with Redora rawhide and got the message "Not running with unified cgroup hierarchy, LSM BPF is not supported". I thought the message is in error, but it was actually correct: nspawn was misdetecting that the container does not sport new-enough systemd to support cgroups-v2. (cherry picked from commit 7e6821ed4e09d68c45858ba463a013eb7593c2c6) --- src/nspawn/nspawn-util.c | 21 +++++++-------------- src/nspawn/nspawn-util.h | 2 +- src/nspawn/nspawn.c | 4 ++-- src/nspawn/test-nspawn-util.c | 11 ++++++----- 4 files changed, 16 insertions(+), 22 deletions(-) diff --git a/src/nspawn/nspawn-util.c b/src/nspawn/nspawn-util.c index 39d7b733e6..6db6ba3b09 100644 --- a/src/nspawn/nspawn-util.c +++ b/src/nspawn/nspawn-util.c @@ -9,7 +9,7 @@ #include "path-util.h" #include "string-util.h" -int systemd_installation_has_version(const char *root, unsigned minimal_version) { +int systemd_installation_has_version(const char *root, const char *minimal_version) { const char *pattern; int r; @@ -49,7 +49,6 @@ int systemd_installation_has_version(const char *root, unsigned minimal_version) STRV_FOREACH(name, names) { /* This is most likely to run only once, hence let's not optimize anything. */ char *t, *t2; - unsigned version; t = startswith(*name, path); if (!t) @@ -58,19 +57,13 @@ int systemd_installation_has_version(const char *root, unsigned minimal_version) t2 = endswith(t, ".so"); if (!t2) continue; + *t2 = '\0'; - t2[0] = '\0'; /* truncate the suffix */ - - r = safe_atou(t, &version); - if (r < 0) { - log_debug_errno(r, "Found libsystemd shared at \"%s.so\", but failed to parse version: %m", *name); - continue; - } - - log_debug("Found libsystemd shared at \"%s.so\", version %u (%s).", - *name, version, - version >= minimal_version ? "OK" : "too old"); - if (version >= minimal_version) + r = strverscmp_improved(t, minimal_version); + log_debug("Found libsystemd shared at \"%s.so\", version %s (%s).", + *name, t, + r >= 0 ? "OK" : "too old"); + if (r >= 0) return true; } } diff --git a/src/nspawn/nspawn-util.h b/src/nspawn/nspawn-util.h index 1e90862c9d..e83cd564da 100644 --- a/src/nspawn/nspawn-util.h +++ b/src/nspawn/nspawn-util.h @@ -1,4 +1,4 @@ /* SPDX-License-Identifier: LGPL-2.1-or-later */ #pragma once -int systemd_installation_has_version(const char *root, unsigned minimal_version); +int systemd_installation_has_version(const char *root, const char *minimal_version); diff --git a/src/nspawn/nspawn.c b/src/nspawn/nspawn.c index 23bc4d6325..8ee41bf870 100644 --- a/src/nspawn/nspawn.c +++ b/src/nspawn/nspawn.c @@ -511,7 +511,7 @@ static int detect_unified_cgroup_hierarchy_from_image(const char *directory) { if (r > 0) { /* Unified cgroup hierarchy support was added in 230. Unfortunately the detection * routine only detects 231, so we'll have a false negative here for 230. */ - r = systemd_installation_has_version(directory, 230); + r = systemd_installation_has_version(directory, "230"); if (r < 0) return log_error_errno(r, "Failed to determine systemd version in container: %m"); if (r > 0) @@ -520,7 +520,7 @@ static int detect_unified_cgroup_hierarchy_from_image(const char *directory) { arg_unified_cgroup_hierarchy = CGROUP_UNIFIED_NONE; } else if (cg_unified_controller(SYSTEMD_CGROUP_CONTROLLER) > 0) { /* Mixed cgroup hierarchy support was added in 233 */ - r = systemd_installation_has_version(directory, 233); + r = systemd_installation_has_version(directory, "233"); if (r < 0) return log_error_errno(r, "Failed to determine systemd version in container: %m"); if (r > 0) diff --git a/src/nspawn/test-nspawn-util.c b/src/nspawn/test-nspawn-util.c index 7d55db8934..b13c2bba2b 100644 --- a/src/nspawn/test-nspawn-util.c +++ b/src/nspawn/test-nspawn-util.c @@ -2,17 +2,18 @@ #include "nspawn-util.h" #include "string-util.h" +#include "strv.h" #include "tests.h" TEST(systemd_installation_has_version) { - static const unsigned versions[] = {0, 231, PROJECT_VERSION, 999}; + const char *version; int r; - for (size_t i = 0; i < ELEMENTSOF(versions); i++) { - r = systemd_installation_has_version(saved_argv[1], versions[i]); + FOREACH_STRING(version, "0", "231", STRINGIFY(PROJECT_VERSION), "999") { + r = systemd_installation_has_version(saved_argv[1], version); assert_se(r >= 0); - log_info("%s has systemd >= %u: %s", - saved_argv[1] ?: "Current installation", versions[i], yes_no(r)); + log_info("%s has systemd >= %s: %s", + saved_argv[1] ?: "Current installation", version, yes_no(r)); } }