From: Zbigniew Jędrzejewski-Szmek Date: Fri, 28 Jun 2019 08:58:06 +0000 (+0200) Subject: Treat kernel version condition as a list of quoted checks X-Git-Tag: v243-rc1~214^2~1 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=910c6d09311ff41ee6f913ff4881f4d8059c2a33;p=thirdparty%2Fsystemd.git 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 045931038bb..0ac9ff48822 100644 --- a/man/systemd.unit.xml +++ b/man/systemd.unit.xml @@ -1136,10 +1136,11 @@ ConditionKernelVersion= may be used to check whether the kernel version (as reported by uname -r) 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 <, <=, =, - !=, >=, > a relative version - comparison is done, otherwise the specified string is matched with shell-style globs. + 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 <, + <=, =, !=, >=, + > a relative version comparison is done, otherwise the specified string is + matched with shell-style globs. 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 a54323a55af..8c613fcd5f9 100644 --- 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 4bbca2074f5..4fa4974adc0 100644 --- 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 3c90f548a42..ae4618c3f37 100644 --- 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