[systemd-commits] 3 commits - src/core src/shared src/test

Zbigniew Jędrzejewski-Szmek zbyszek at kemper.freedesktop.org
Wed Dec 17 21:56:15 PST 2014


 src/core/load-fragment.c  |    4 +++-
 src/core/main.c           |   25 +++++++++++++++----------
 src/shared/missing.h      |   10 +++++++---
 src/test/test-unit-file.c |   38 +++++++++++++++++++++-----------------
 src/test/test-util.c      |   20 ++++++++++++++++++++
 5 files changed, 66 insertions(+), 31 deletions(-)

New commits:
commit ee05e7795bb9ad7d1212dd49ad362f3e9603c4fd
Author: Zbigniew Jędrzejewski-Szmek <zbyszek at in.waw.pl>
Date:   Tue Dec 16 23:53:23 2014 -0500

    core: use raw_clone instead of fork in signal handler
    
    fork() is not async-signal-safe and calling it from the signal handler
    could result in a deadlock when at_fork() handlers are called. Using
    the raw clone() syscall sidesteps that problem.
    
    The tricky part is that raise() does not work, since getpid() does not
    work. Add raw_getpid() to get the real pid, and use kill() instead of
    raise().
    
    https://bugs.freedesktop.org/show_bug.cgi?id=86604

diff --git a/src/core/main.c b/src/core/main.c
index 77980e3..300567a 100644
--- a/src/core/main.c
+++ b/src/core/main.c
@@ -142,7 +142,7 @@ noreturn static void crash(int sig) {
                 /* We want to wait for the core process, hence let's enable SIGCHLD */
                 sigaction(SIGCHLD, &sa, NULL);
 
-                pid = fork();
+                pid = raw_clone(SIGCHLD, NULL);
                 if (pid < 0)
                         log_emergency_errno(errno, "Caught <%s>, cannot fork for core dump: %m", signal_to_string(sig));
 
@@ -163,11 +163,11 @@ noreturn static void crash(int sig) {
                         chdir("/");
 
                         /* Raise the signal again */
-                        raise(sig);
+                        pid = raw_getpid();
+                        kill(pid, sig); /* raise() would kill the parent */
 
                         assert_not_reached("We shouldn't be here...");
                         _exit(1);
-
                 } else {
                         siginfo_t status;
                         int r;
@@ -177,7 +177,13 @@ noreturn static void crash(int sig) {
                         if (r < 0)
                                 log_emergency_errno(r, "Caught <%s>, waitpid() failed: %m", signal_to_string(sig));
                         else if (status.si_code != CLD_DUMPED)
-                                log_emergency("Caught <%s>, core dump failed.", signal_to_string(sig));
+                                log_emergency("Caught <%s>, core dump failed (child "PID_FMT", code=%s, status=%i/%s).",
+                                              signal_to_string(sig),
+                                              pid, sigchld_code_to_string(status.si_code),
+                                              status.si_status,
+                                              strna(status.si_code == CLD_EXITED
+                                                    ? exit_status_to_string(status.si_status, EXIT_STATUS_FULL)
+                                                    : signal_to_string(status.si_status)));
                         else
                                 log_emergency("Caught <%s>, dumped core as pid "PID_FMT".", signal_to_string(sig), pid);
                 }
@@ -199,18 +205,17 @@ noreturn static void crash(int sig) {
                 /* Let the kernel reap children for us */
                 assert_se(sigaction(SIGCHLD, &sa, NULL) == 0);
 
-                pid = fork();
+                pid = raw_clone(SIGCHLD, NULL);
                 if (pid < 0)
                         log_emergency_errno(errno, "Failed to fork off crash shell: %m");
                 else if (pid == 0) {
                         make_console_stdio();
-                        execl("/bin/sh", "/bin/sh", NULL);
+                        execle("/bin/sh", "/bin/sh", NULL, environ);
 
-                        log_emergency_errno(errno, "execl() failed: %m");
+                        log_emergency_errno(errno, "execle() failed: %m");
                         _exit(1);
-                }
-
-                log_info("Successfully spawned crash shell as pid "PID_FMT".", pid);
+                } else
+                        log_info("Successfully spawned crash shell as PID "PID_FMT".", pid);
         }
 
         log_emergency("Freezing execution.");
diff --git a/src/shared/missing.h b/src/shared/missing.h
index bea1254..91a6215 100644
--- a/src/shared/missing.h
+++ b/src/shared/missing.h
@@ -636,12 +636,16 @@ static inline int setns(int fd, int nstype) {
 #define CAP_AUDIT_READ 37
 #endif
 
-static inline long raw_clone(unsigned long flags, void *child_stack) {
+static inline int raw_clone(unsigned long flags, void *child_stack) {
 #if defined(__s390__) || defined(__CRIS__)
         /* On s390 and cris the order of the first and second arguments
          * of the raw clone() system call is reversed. */
-        return syscall(__NR_clone, child_stack, flags);
+        return (int) syscall(__NR_clone, child_stack, flags);
 #else
-        return syscall(__NR_clone, flags, child_stack);
+        return (int) syscall(__NR_clone, flags, child_stack);
 #endif
 }
+
+static inline pid_t raw_getpid(void) {
+        return (pid_t) syscall(__NR_getpid);
+}
diff --git a/src/test/test-util.c b/src/test/test-util.c
index 6c7d77b..bbf7512 100644
--- a/src/test/test-util.c
+++ b/src/test/test-util.c
@@ -1312,6 +1312,25 @@ static void test_parse_proc_cmdline(void) {
         assert_se(parse_proc_cmdline(parse_item) >= 0);
 }
 
+static void test_raw_clone(void) {
+        pid_t parent, pid, pid2;
+
+        parent = getpid();
+        log_info("before clone: getpid()→"PID_FMT, parent);
+        assert_se(raw_getpid() == parent);
+
+        pid = raw_clone(0, NULL);
+        assert(pid >= 0);
+
+        pid2 = raw_getpid();
+        log_info("raw_clone: "PID_FMT" getpid()→"PID_FMT" raw_getpid()→"PID_FMT,
+                 pid, getpid(), pid2);
+        if (pid == 0)
+                assert(pid2 != parent);
+        else
+                assert(pid2 == parent);
+}
+
 int main(int argc, char *argv[]) {
         log_parse_environment();
         log_open();
@@ -1384,6 +1403,7 @@ int main(int argc, char *argv[]) {
         test_unquote_first_word();
         test_unquote_many_words();
         test_parse_proc_cmdline();
+        test_raw_clone();
 
         return 0;
 }

commit 503dbda6d94c16161762b7b489677a377f235590
Author: Zbigniew Jędrzejewski-Szmek <zbyszek at in.waw.pl>
Date:   Thu Dec 18 00:52:28 2014 -0500

    test-unit-file: add test for semicolon escaping
    
    https://bugs.freedesktop.org/show_bug.cgi?id=87393

diff --git a/src/test/test-unit-file.c b/src/test/test-unit-file.c
index f31a1bb..08da2ba 100644
--- a/src/test/test-unit-file.c
+++ b/src/test/test-unit-file.c
@@ -71,6 +71,7 @@ static void check_execcommand(ExecCommand *c,
                               const char* path,
                               const char* argv0,
                               const char* argv1,
+                              const char* argv2,
                               bool ignore) {
         assert_se(c);
         log_info("%s %s %s %s",
@@ -78,7 +79,7 @@ static void check_execcommand(ExecCommand *c,
         assert_se(streq(c->path, path));
         assert_se(streq(c->argv[0], argv0));
         assert_se(streq(c->argv[1], argv1));
-        assert_se(c->argv[2] == NULL);
+        assert_se(streq_ptr(c->argv[2], argv2));
         assert_se(c->ignore == ignore);
 }
 
@@ -102,7 +103,7 @@ static void test_config_parse_exec(void) {
                               "LValue", 0, "/RValue r1",
                               &c, NULL);
         assert_se(r >= 0);
-        check_execcommand(c, "/RValue", "/RValue", "r1", false);
+        check_execcommand(c, "/RValue", "/RValue", "r1", NULL, false);
 
         r = config_parse_exec(NULL, "fake", 2, "section", 1,
                               "LValue", 0, "/RValue///slashes/// r1",
@@ -110,8 +111,7 @@ static void test_config_parse_exec(void) {
        /* test slashes */
         assert_se(r >= 0);
         c1 = c->command_next;
-        check_execcommand(c1, "/RValue/slashes", "/RValue///slashes///",
-                          "r1", false);
+        check_execcommand(c1, "/RValue/slashes", "/RValue///slashes///", "r1", NULL, false);
 
         /* honour_argv0 */
         r = config_parse_exec(NULL, "fake", 3, "section", 1,
@@ -119,7 +119,7 @@ static void test_config_parse_exec(void) {
                               &c, NULL);
         assert_se(r >= 0);
         c1 = c1->command_next;
-        check_execcommand(c1, "/RValue/slashes2", "argv0", "r1", false);
+        check_execcommand(c1, "/RValue/slashes2", "argv0", "r1", NULL, false);
 
         /* ignore && honour_argv0 */
         r = config_parse_exec(NULL, "fake", 4, "section", 1,
@@ -127,8 +127,7 @@ static void test_config_parse_exec(void) {
                               &c, NULL);
         assert_se(r >= 0);
         c1 = c1->command_next;
-        check_execcommand(c1,
-                          "/RValue/slashes3", "argv0a", "r1", true);
+        check_execcommand(c1, "/RValue/slashes3", "argv0a", "r1", NULL, true);
 
         /* ignore && honour_argv0 */
         r = config_parse_exec(NULL, "fake", 4, "section", 1,
@@ -136,8 +135,7 @@ static void test_config_parse_exec(void) {
                               &c, NULL);
         assert_se(r >= 0);
         c1 = c1->command_next;
-        check_execcommand(c1,
-                          "/RValue/slashes4", "argv0b", "r1", true);
+        check_execcommand(c1, "/RValue/slashes4", "argv0b", "r1", NULL, true);
 
         /* ignore && ignore */
         r = config_parse_exec(NULL, "fake", 4, "section", 1,
@@ -161,12 +159,10 @@ static void test_config_parse_exec(void) {
                               &c, NULL);
         assert_se(r >= 0);
         c1 = c1->command_next;
-        check_execcommand(c1,
-                          "/RValue", "argv0", "r1", true);
+        check_execcommand(c1, "/RValue", "argv0", "r1", NULL, true);
 
         c1 = c1->command_next;
-        check_execcommand(c1,
-                          "/goo/goo", "/goo/goo", "boo", false);
+        check_execcommand(c1, "/goo/goo", "/goo/goo", "boo", NULL, false);
 
         /* trailing semicolon */
         r = config_parse_exec(NULL, "fake", 5, "section", 1,
@@ -175,20 +171,28 @@ static void test_config_parse_exec(void) {
                               &c, NULL);
         assert_se(r >= 0);
         c1 = c1->command_next;
-        check_execcommand(c1,
-                          "/RValue", "argv0", "r1", true);
+        check_execcommand(c1, "/RValue", "argv0", "r1", NULL, true);
 
         assert_se(c1->command_next == NULL);
 
         /* escaped semicolon */
         r = config_parse_exec(NULL, "fake", 5, "section", 1,
                               "LValue", 0,
-                              "/usr/bin/find \\;",
+                              "/bin/find \\;",
+                              &c, NULL);
+        assert_se(r >= 0);
+        c1 = c1->command_next;
+        check_execcommand(c1, "/bin/find", "/bin/find", ";", NULL, false);
+
+        /* escaped semicolon with following arg */
+        r = config_parse_exec(NULL, "fake", 5, "section", 1,
+                              "LValue", 0,
+                              "/sbin/find \\; x",
                               &c, NULL);
         assert_se(r >= 0);
         c1 = c1->command_next;
         check_execcommand(c1,
-                          "/usr/bin/find", "/usr/bin/find", ";", false);
+                          "/sbin/find", "/sbin/find", ";", "x", false);
 
         exec_command_free_list(c);
 }

commit 3851c51ad1dae6a1266896e7e364e424dc82467a
Author: tomsod-m ya ru <tomsod-m at ya.ru>
Date:   Wed Dec 17 23:01:06 2014 -0500

    load-fragment: properly unescape \;
    
    https://bugs.freedesktop.org/show_bug.cgi?id=87393

diff --git a/src/core/load-fragment.c b/src/core/load-fragment.c
index 8e5be87..358d36b 100644
--- a/src/core/load-fragment.c
+++ b/src/core/load-fragment.c
@@ -601,8 +601,10 @@ int config_parse_exec(const char *unit,
                 FOREACH_WORD_QUOTED(word, l, rvalue, state) {
                         if (strneq(word, ";", MAX(l, 1U)))
                                 break;
-                        else if (strneq(word, "\\;", MAX(l, 1U)))
+                        else if (strneq(word, "\\;", MAX(l, 1U))) {
                                 word ++;
+                                l --;
+                        }
 
                         if (honour_argv0 && word == rvalue) {
                                 assert(!path);



More information about the systemd-commits mailing list