Merge pull request systemd#6518 from joukewitteveen/process-rename

evverx · web-flow · commit a93e2f65eb75 · 2017-08-04T14:54:47.000+03:00
process-util: update the end pointer of the process name on rename
diff --git a/src/basic/process-util.c b/src/basic/process-util.c
@@ -312,19 +312,18 @@ int rename_process(const char name[]) {
         /* Third step, completely replace the argv[] array the kernel maintains for us. This requires privileges, but
          * has the advantage that the argv[] array is exactly what we want it to be, and not filled up with zeros at
          * the end. This is the best option for changing /proc/self/cmdline. */
-        if (mm_size < l+1) {
+
+        /* Let's not bother with this if we don't have euid == 0. Strictly speaking we should check for the
+         * CAP_SYS_RESOURCE capability which is independent of the euid. In our own code the capability generally is
+         * present only for euid == 0, hence let's use this as quick bypass check, to avoid calling mmap() if
+         * PR_SET_MM_ARG_{START,END} fails with EPERM later on anyway. After all geteuid() is dead cheap to call, but
+         * mmap() is not. */
+        if (geteuid() != 0)
+                log_debug("Skipping PR_SET_MM, as we don't have privileges.");
+        else if (mm_size < l+1) {
                 size_t nn_size;
                 char *nn;
 
-                /* Let's not bother with this if we don't have euid == 0. Strictly speaking if people do weird stuff
-                 * with capabilities this could work even for euid != 0, but our own code generally doesn't do that,
-                 * hence let's use this as quick bypass check, to avoid calling mmap() if PR_SET_MM_ARG_START fails
-                 * with EPERM later on anyway. After all geteuid() is dead cheap to call, but mmap() is not. */
-                if (geteuid() != 0) {
-                        log_debug("Skipping PR_SET_MM_ARG_START, as we don't have privileges.");
-                        goto use_saved_argv;
-                }
-
                 nn_size = PAGE_ALIGN(l+1);
                 nn = mmap(NULL, nn_size, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0);
                 if (nn == MAP_FAILED) {
@@ -351,9 +350,14 @@ int rename_process(const char name[]) {
 
                 mm = nn;
                 mm_size = nn_size;
-        } else
+        } else {
                 strncpy(mm, name, mm_size);
 
+                /* Update the end pointer, continuing regardless of any failure. */
+                if (prctl(PR_SET_MM, PR_SET_MM_ARG_END, (unsigned long) mm + l + 1, 0, 0) < 0)
+                        log_debug_errno(errno, "PR_SET_MM_ARG_END failed, proceeding without: %m");
+        }
+
 use_saved_argv:
         /* Fourth step: in all cases we'll also update the original argv[], so that our own code gets it right too if
          * it still looks here */
diff --git a/src/test/test-process-util.c b/src/test/test-process-util.c
@@ -355,50 +355,80 @@ static void test_get_process_cmdline_harder(void) {
         _exit(0);
 }
 
-static void test_rename_process_one(const char *p, int ret) {
+static void test_rename_process_now(const char *p, int ret) {
         _cleanup_free_ char *comm = NULL, *cmdline = NULL;
-        pid_t pid;
         int r;
 
-        pid = fork();
-        assert_se(pid >= 0);
-
-        if (pid > 0) {
-                siginfo_t si;
-
-                assert_se(wait_for_terminate(pid, &si) >= 0);
-                assert_se(si.si_code == CLD_EXITED);
-                assert_se(si.si_status == EXIT_SUCCESS);
-
-                return;
-        }
-
-        /* child */
         r = rename_process(p);
-
         assert_se(r == ret ||
                   (ret == 0 && r >= 0) ||
                   (ret > 0 && r > 0));
 
         if (r < 0)
-                goto finish;
+                return;
 
 #ifdef HAVE_VALGRIND_VALGRIND_H
         /* see above, valgrind is weird, we can't verify what we are doing here */
         if (RUNNING_ON_VALGRIND)
-                goto finish;
+                return;
 #endif
 
         assert_se(get_process_comm(0, &comm) >= 0);
         log_info("comm = <%s>", comm);
         assert_se(strneq(comm, p, 15));
 
         assert_se(get_process_cmdline(0, 0, false, &cmdline) >= 0);
-        log_info("cmdline = <%s>", cmdline);
-        assert_se(strneq(p, cmdline, strlen("test-process-util")));
-        assert_se(startswith(p, cmdline));
+        /* we cannot expect cmdline to be renamed properly without privileges */
+        if (geteuid() == 0) {
+                log_info("cmdline = <%s>", cmdline);
+                assert_se(strneq(p, cmdline, strlen("test-process-util")));
+                assert_se(startswith(p, cmdline));
+        } else
+                log_info("cmdline = <%s> (not verified)", cmdline);
+}
 
-finish:
+static void test_rename_process_one(const char *p, int ret) {
+        siginfo_t si;
+        pid_t pid;
+
+        pid = fork();
+        assert_se(pid >= 0);
+
+        if (pid == 0) {
+                /* child */
+                test_rename_process_now(p, ret);
+                _exit(EXIT_SUCCESS);
+        }
+
+        assert_se(wait_for_terminate(pid, &si) >= 0);
+        assert_se(si.si_code == CLD_EXITED);
+        assert_se(si.si_status == EXIT_SUCCESS);
+}
+
+static void test_rename_process_multi(void) {
+        pid_t pid;
+
+        pid = fork();
+        assert_se(pid >= 0);
+
+        if (pid > 0) {
+                siginfo_t si;
+
+                assert_se(wait_for_terminate(pid, &si) >= 0);
+                assert_se(si.si_code == CLD_EXITED);
+                assert_se(si.si_status == EXIT_SUCCESS);
+
+                return;
+        }
+
+        /* child */
+        test_rename_process_now("one", 1);
+        test_rename_process_now("more", 0); /* longer than "one", hence truncated */
+        setresuid(99, 99, 99);
+        test_rename_process_now("time!", 0);
+        test_rename_process_now("0", 1); /* shorter than "one", should fit */
+        test_rename_process_one("", -EINVAL);
+        test_rename_process_one(NULL, -EINVAL);
         _exit(EXIT_SUCCESS);
 }
 
@@ -408,6 +438,7 @@ static void test_rename_process(void) {
         test_rename_process_one("foo", 1); /* should always fit */
         test_rename_process_one("this is a really really long process name, followed by some more words", 0); /* unlikely to fit */
         test_rename_process_one("1234567", 1); /* should always fit */
+        test_rename_process_multi(); /* multiple invocations and dropped privileges */
 }
 
 static void test_getpid_cached(void) {
