On Thu, 14.05.15 09:15, Martin Pitt (martin.p...@ubuntu.com) wrote: Thanks! APplied!
> Martin Pitt [2015-05-13 17:01 +0200]: > > I got a report [1] that you can trivially crash systemd (pid1) at boot > > by creating a unit with an Exec= line with a modifier and a space: > > > > $ cat /tmp/foo.service > > [Service] > > ExecStart=- /bin/echo hello > > > > $ systemd-analyze verify /tmp/foo.service > > Assertion 'skip < l' failed at ../src/core/load-fragment.c:607, function > > config_parse_exec(). Aborting. > > Aborted (core dumped) > > > > systemd pid 1 will crash the same way at boot, but with > > systemd-analyze it's less harmful to test :-) > > This patch is a minimally invasive and straighforward fix for this > with the behaviour as discussed. It is appropriate for the stable > branch and distro updates for stable releases. > > I'd like to do the rewriting to unquote_first_word() in a separate > commit; it's easy to miss subtle corner cases. This is already > mentioned in TODO: > > * code cleanup: retire FOREACH_WORD_QUOTED, port to unquote_first_word() > loops instead > > (from the similar recent crash fixed in 470dca63c) > > Thanks, > > Martin > -- > Martin Pitt | http://www.piware.de > Ubuntu Developer (www.ubuntu.com) | Debian Developer (www.debian.org) > From 10f6ac7c8b3dad0197ce795e33383c24ea2d53b1 Mon Sep 17 00:00:00 2001 > From: Martin Pitt <martin.p...@ubuntu.com> > Date: Thu, 14 May 2015 09:06:40 +0200 > Subject: [PATCH] core: Fix assertion with empty Exec*= paths > > An Exec*= line with whitespace after modifiers, like > > ExecStart=- /bin/true > > is considered to have an empty command path. This is as specified, but causes > systemd to crash with > > Assertion 'skip < l' failed at ../src/core/load-fragment.c:607, function > config_parse_exec(). Aborting. > Aborted (core dumped) > > Fix this by logging an error instead and ignoring the invalid line. > > Add corresponding test cases. Also add a test case for a completely empty > value > which resets the command list. > > https://launchpad.net/bugs/1454173 > --- > src/core/load-fragment.c | 6 +++++- > src/test/test-unit-file.c | 21 +++++++++++++++++++++ > 2 files changed, 26 insertions(+), 1 deletion(-) > > diff --git a/src/core/load-fragment.c b/src/core/load-fragment.c > index 33d9e27..3865017 100644 > --- a/src/core/load-fragment.c > +++ b/src/core/load-fragment.c > @@ -595,7 +595,11 @@ int config_parse_exec( > skip = separate_argv0 + ignore; > > /* skip special chars in the beginning */ > - assert(skip < l); > + if (l <= skip) { > + log_syntax(unit, LOG_ERR, filename, > line, EINVAL, "Empty path in command line, ignoring: %s", rvalue); > + r = 0; > + goto fail; > + } > > } else if (strneq(word, ";", MAX(l, 1U))) > /* new commandline */ > diff --git a/src/test/test-unit-file.c b/src/test/test-unit-file.c > index f3f6c29..03ca70a 100644 > --- a/src/test/test-unit-file.c > +++ b/src/test/test-unit-file.c > @@ -317,6 +317,27 @@ static void test_config_parse_exec(void) { > assert_se(r == 0); > assert_se(c1->command_next == NULL); > > + log_info("/* invalid space between modifiers */"); > + r = config_parse_exec(NULL, "fake", 4, "section", 1, > + "LValue", 0, "- /path", > + &c, NULL); > + assert_se(r == 0); > + assert_se(c1->command_next == NULL); > + > + log_info("/* only modifiers, no path */"); > + r = config_parse_exec(NULL, "fake", 4, "section", 1, > + "LValue", 0, "-", > + &c, NULL); > + assert_se(r == 0); > + assert_se(c1->command_next == NULL); > + > + log_info("/* empty argument, reset */"); > + r = config_parse_exec(NULL, "fake", 4, "section", 1, > + "LValue", 0, "", > + &c, NULL); > + assert_se(r == 0); > + assert_se(c == NULL); > + > exec_command_free_list(c); > } > > -- > 2.1.4 > > _______________________________________________ > systemd-devel mailing list > systemd-devel@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/systemd-devel Lennart -- Lennart Poettering, Red Hat _______________________________________________ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
