Hello all, today I got a report about crashing systemd (pid 1) by installing a package with a broken unit file. Rebooting also doesn't work as systemd immediately crashes again. It turns out the unit file used a multi-line ExecStart= with ' quoting where the author (accidentally) added a trailing slash after the usual \ escaping of the newline:
---------- snip ---------
[Unit]
Description=test
[Service]
ExecStart=/bin/echo 'foo \
bar'
---------- snip ---------
Instead of actually breaking your machine, you can just copy that unit
to e. g. /tmp and then run "systemd-analyze verify test.service" to
get the same assertion.
Of course this syntax is invalid, but systemd certainly shouldn't
spill its guts that hard, but just reject the broken .service file.
Simple fix with test case attached.
Thanks,
Martin
--
Martin Pitt | http://www.piware.de
Ubuntu Developer (www.ubuntu.com) | Debian Developer (www.debian.org)
From ef581ce6e258ffc0c90f268b6054b83e15374ba6 Mon Sep 17 00:00:00 2001
From: Martin Pitt <martin.p...@ubuntu.com>
Date: Wed, 22 Apr 2015 23:09:43 +0100
Subject: [PATCH] util: Fix assertion in split() on missing '
When parsing a unit with a trailing slash after an escaped line break, like
ExecStart=/bin/echo 'foo \
bar'
the split() function (through config_parse()) asserted and crashed pid 1:
Assertion 'current[*l + 1] == quotechars[0]' failed at ../src/shared/util.c:583, function split(). Aborting.
Fix this by returning an error in this case ("trailing garbage").
Add corresponding test case. Also fix the missing "unit" argument of
config_parse_exec() in the comment.
https://launchpad.net/bugs/1447243
---
src/shared/util.c | 3 +--
src/test/test-unit-file.c | 15 +++++++++++++++
2 files changed, 16 insertions(+), 2 deletions(-)
diff --git a/src/shared/util.c b/src/shared/util.c
index 4a04484..6a883d7 100644
--- a/src/shared/util.c
+++ b/src/shared/util.c
@@ -570,13 +570,12 @@ const char* split(const char **state, size_t *l, const char *separator, bool quo
char quotechars[2] = {*current, '\0'};
*l = strcspn_escaped(current + 1, quotechars);
- if (current[*l + 1] == '\0' ||
+ if (current[*l + 1] == '\0' || current[*l + 1] != quotechars[0] ||
(current[*l + 2] && !strchr(separator, current[*l + 2]))) {
/* right quote missing or garbage at the end */
*state = current;
return NULL;
}
- assert(current[*l + 1] == quotechars[0]);
*state = current++ + *l + 2;
} else if (quoted) {
*l = strcspn_escaped(current, separator);
diff --git a/src/test/test-unit-file.c b/src/test/test-unit-file.c
index 22ac76d..5a9796d 100644
--- a/src/test/test-unit-file.c
+++ b/src/test/test-unit-file.c
@@ -91,6 +91,7 @@ static void check_execcommand(ExecCommand *c,
static void test_config_parse_exec(void) {
/* int config_parse_exec(
+ const char *unit,
const char *filename,
unsigned line,
const char *section,
@@ -302,6 +303,20 @@ static void test_config_parse_exec(void) {
assert_se(r == 0);
assert_se(c1->command_next == NULL);
+ log_info("/* missing ending ' */");
+ r = config_parse_exec(NULL, "fake", 4, "section", 1,
+ "LValue", 0, "/path 'foo",
+ &c, NULL);
+ assert_se(r == 0);
+ assert_se(c1->command_next == NULL);
+
+ log_info("/* missing ending ' with trailing backslash */");
+ r = config_parse_exec(NULL, "fake", 4, "section", 1,
+ "LValue", 0, "/path 'foo\\",
+ &c, NULL);
+ assert_se(r == 0);
+ assert_se(c1->command_next == NULL);
+
exec_command_free_list(c);
}
--
2.1.4
signature.asc
Description: Digital signature
_______________________________________________ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
