Treat kernel version condition as a list of quoted checks

Before only one comparison was allowed. Let's make this more flexible:
ConditionKernelVersion = ">=4.0" "<=4.5"

Fixes #12881.

This also fixes expressions like "ConditionKernelVersion=>" which would
evaluate as true.

diff --git a/man/systemd.unit.xml b/man/systemd.unit.xml
index 0459310..0ac9ff4 100644 (file)
--- a/man/systemd.unit.xml
+++ b/man/systemd.unit.xml
@@ -1136,10 +1136,11 @@
 
         <para><varname>ConditionKernelVersion=</varname> may be used to check whether the kernel version (as
         reported by <command>uname -r</command>) matches a certain expression (or if prefixed with the
-        exclamation mark does not match it). The argument must be a single string. If the string starts with
-        one of <literal>&lt;</literal>, <literal>&lt;=</literal>, <literal>=</literal>,
-        <literal>!=</literal>, <literal>&gt;=</literal>, <literal>&gt;</literal> a relative version
-        comparison is done, otherwise the specified string is matched with shell-style globs.</para>
+        exclamation mark does not match it). The argument must be a list of (potentially quoted) expressions.
+        For each of the expressions, if it starts with one of <literal>&lt;</literal>,
+        <literal>&lt;=</literal>, <literal>=</literal>, <literal>!=</literal>, <literal>&gt;=</literal>,
+        <literal>&gt;</literal> a relative version comparison is done, otherwise the specified string is
+        matched with shell-style globs.</para>
 
         <para>Note that using the kernel version string is an unreliable way to determine which features are supported
         by a kernel, because of the widespread practice of backporting drivers, features, and fixes from newer upstream

diff --git a/src/shared/condition.c b/src/shared/condition.c
index a54323a..8c613fc 100644 (file)
--- a/src/shared/condition.c
+++ b/src/shared/condition.c
@@ -207,6 +207,7 @@ static int condition_test_kernel_version(Condition *c) {
         OrderOperator order;
         struct utsname u;
         const char *p;
+        bool first = true;
 
         assert(c);
         assert(c->parameter);
@@ -215,13 +216,49 @@ static int condition_test_kernel_version(Condition *c) {
         assert_se(uname(&u) >= 0);
 
         p = c->parameter;
-        order = parse_order(&p);
 
-        /* No prefix? Then treat as glob string */
-        if (order < 0)
-                return fnmatch(skip_leading_chars(c->parameter, NULL), u.release, 0) == 0;
+        for (;;) {
+                _cleanup_free_ char *word = NULL;
+                const char *s;
+                int r;
+
+                r = extract_first_word(&p, &word, NULL, EXTRACT_UNQUOTE);
+                if (r < 0)
+                        return log_debug_errno(r, "Failed to parse condition string \"%s\": %m", p);
+                if (r == 0)
+                        break;
+
+                s = strstrip(word);
+                order = parse_order(&s);
+                if (order >= 0) {
+                        s += strspn(s, WHITESPACE);
+                        if (isempty(s)) {
+                                if (first) {
+                                        /* For backwards compatibility, allow whitespace between the operator and
+                                         * value, without quoting, but only in the first expression. */
+                                        word = mfree(word);
+                                        r = extract_first_word(&p, &word, NULL, 0);
+                                        if (r < 0)
+                                                return log_debug_errno(r, "Failed to parse condition string \"%s\": %m", p);
+                                        if (r == 0)
+                                                return log_debug_errno(SYNTHETIC_ERRNO(EINVAL), "Unexpected end of expression: %s", p);
+                                        s = word;
+                                } else
+                                        return log_debug_errno(SYNTHETIC_ERRNO(EINVAL), "Unexpected end of expression: %s", p);
+                        }
+
+                        r = test_order(str_verscmp(u.release, s), order);
+                } else
+                        /* No prefix? Then treat as glob string */
+                        r = fnmatch(s, u.release, 0) == 0;
+
+                if (r == 0)
+                        return false;
+
+                first = false;
+        }
 
-        return test_order(str_verscmp(u.release, skip_leading_chars(p, NULL)), order);
+        return true;
 }
 
 static int condition_test_memory(Condition *c) {

diff --git a/src/test/test-condition.c b/src/test/test-condition.c
index 4bbca20..4fa4974 100644 (file)
--- a/src/test/test-condition.c
+++ b/src/test/test-condition.c
@@ -301,9 +301,11 @@ static void test_condition_test_kernel_version(void) {
         assert_se(condition_test(condition));
         condition_free(condition);
 
+        /* An artificially empty condition. It evaluates to true, but normally
+         * such condition cannot be created, because the condition list is reset instead. */
         condition = condition_new(CONDITION_KERNEL_VERSION, "", false, false);
         assert_se(condition);
-        assert_se(!condition_test(condition));
+        assert_se(condition_test(condition) > 0);
         condition_free(condition);
 
         assert_se(uname(&u) >= 0);
@@ -327,6 +329,26 @@ static void test_condition_test_kernel_version(void) {
         assert_se(condition_test(condition));
         condition_free(condition);
 
+        condition = condition_new(CONDITION_KERNEL_VERSION, ">0.1.2", false, false);
+        assert_se(condition);
+        assert_se(condition_test(condition) > 0);
+        condition_free(condition);
+
+        condition = condition_new(CONDITION_KERNEL_VERSION, "'>0.1.2' '<9.0.0'", false, false);
+        assert_se(condition);
+        assert_se(condition_test(condition) > 0);
+        condition_free(condition);
+
+        condition = condition_new(CONDITION_KERNEL_VERSION, "> 0.1.2 < 9.0.0", false, false);
+        assert_se(condition);
+        assert_se(condition_test(condition) == -EINVAL);
+        condition_free(condition);
+
+        condition = condition_new(CONDITION_KERNEL_VERSION, ">", false, false);
+        assert_se(condition);
+        assert_se(condition_test(condition) == -EINVAL);
+        condition_free(condition);
+
         condition = condition_new(CONDITION_KERNEL_VERSION, ">= 0.1.2", false, false);
         assert_se(condition);
         assert_se(condition_test(condition));

diff --git a/test/test-execute/exec-basic.service b/test/test-execute/exec-basic.service
index 3c90f54..ae4618c 100644 (file)
--- a/test/test-execute/exec-basic.service
+++ b/test/test-execute/exec-basic.service
@@ -2,6 +2,8 @@
 Description=Test for basic execution
 ConditionKernelVersion=">=3.0"
 ConditionKernelVersion=">=2.0" "<=60" "!=1.4"
+ConditionKernelVersion=" >= 2.0" " <= 60 " "!= 1.4"
+ConditionKernelVersion=" >= 2.0" " * " "*.*"
 
 [Service]
 ExecStart=touch /tmp/a ; /bin/sh -c 'touch /tmp/b' ; touch /tmp/c