Fix setproctitle on Linux.

The former code used to assume that argv and envp fit in a memory
page, but it is not true (at least today), causing setproctitle to
fail randomly and silently.

This patch rewrites setproctitle to remove the memory page assumption
entirely. Instead it now writes the process title up to the end of
envp after saving envp storage.

BUG=chromium:1044502
TEST=set_process_title_linux_unittests  # on Linux 3.{0..19}, 4.{0..20}, 5.{0..5}
TEST=Build and run Chrome OS Chrome and see ps output

Change-Id: Ie2f060a38b3e5003155ef8e5788855761e229776
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2026772Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: Thomas Anderson <thomasanderson@chromium.org>
Reviewed-by: Ken Rockot <rockot@google.com>
Reviewed-by: Jorge Lucangeli Obes <jorgelo@chromium.org>
Commit-Queue: Shuhei Takahashi <nya@chromium.org>
Cr-Commit-Position: refs/heads/master@{#752356}

content/test/BUILD.gn

@@ -1528,6 +1528,9 @@ static_library("run_all_unittests") {
     ":test_support",
     "//base/test:test_support",
   ]
+  if (is_linux) {
+    deps += [ "//services/service_manager/embedder:set_process_title_linux" ]
+  }
 }
 
 test("content_unittests") {
@@ -2321,6 +2324,11 @@ test("content_unittests") {
   if (use_x11) {
     deps += [ "//ui/gfx/x" ]
   }
+
+  if (is_linux) {
+    sources += [ "../../services/service_manager/embedder/set_process_title_linux_unittest.cc" ]
+    deps += [ "//services/service_manager/embedder:set_process_title_linux" ]
+  }
 }
 
 if (enable_nocompile_tests) {

content/test/run_all_unittests.cc

@@ -5,10 +5,20 @@
 #include "base/bind.h"
 #include "base/test/launcher/unit_test_launcher.h"
 #include "base/values.h"
+#include "build/build_config.h"
 #include "content/public/test/unittest_test_suite.h"
 #include "content/test/content_test_suite.h"
 
+#if defined(OS_LINUX)
+#include "services/service_manager/embedder/set_process_title_linux.h"
+#endif
+
 int main(int argc, char** argv) {
+#if defined(OS_LINUX)
+  // For setproctitle unit tests.
+  setproctitle_init(const_cast<const char**>(argv));
+#endif
+
   content::UnitTestTestSuite test_suite(
       new content::ContentTestSuite(argc, argv));
   return base::LaunchUnitTests(argc, argv,

services/service_manager/embedder/BUILD.gn

@@ -2,6 +2,8 @@
 # Use of this source code is governed by a BSD-style license that can be
 # found in the LICENSE file.
 
+import("//testing/test.gni")
+
 if (!is_ios) {
   # iOS embeds the Service Manager but does not use anything in this library.
   # This avoids having an empty component library target, which can otherwise be
@@ -12,7 +14,6 @@ if (!is_ios) {
       "main_delegate.h",
       "process_type.h",
       "set_process_title.h",
-      "set_process_title_linux.h",
       "shared_file_util.h",
     ]
 
@@ -20,7 +21,6 @@ if (!is_ios) {
       "main.cc",
       "main_delegate.cc",
       "set_process_title.cc",
-      "set_process_title_linux.cc",
       "shared_file_util.cc",
     ]
 
@@ -58,6 +58,10 @@ if (!is_ios) {
       deps += [ "//ui/base" ]
     }
 
+    if (is_linux) {
+      deps += [ ":set_process_title_linux" ]
+    }
+
     defines = [ "IS_SERVICE_MANAGER_EMBEDDER_IMPL" ]
   }
 }
@@ -74,3 +78,11 @@ component("embedder_switches") {
 source_set("embedder_result_codes") {
   sources = [ "result_codes.h" ]
 }
+
+if (is_linux) {
+  source_set("set_process_title_linux") {
+    public = [ "set_process_title_linux.h" ]
+    sources = [ "set_process_title_linux.cc" ]
+    deps = [ "//base" ]
+  }
+}

services/service_manager/embedder/set_process_title_linux.cc

@@ -14,23 +14,23 @@
 //
 // These arrays contain pointers to a second location in memory, where the
 // strings themselves are stored one after another: first all the arguments,
-// then the environment variables. The kernel will allocate a single page of
-// memory for this purpose, so the end of the page containing argv[0] is the
-// end of the storage potentially available to store the process title.
+// then the environment variables.
 //
 // When the kernel reads the command line arguments for a process, it looks at
-// the range of memory within this page that it initially used for the argument
-// list. If the terminating '\0' character is still where it expects, nothing
-// further is done. If it has been overwritten, the kernel will scan up to the
-// size of a page looking for another. (Note, however, that in general not that
-// much space is actually mapped, since argv[0] is rarely page-aligned and only
-// one page is mapped.)
+// the range of memory that it initially used for the argument list. If the
+// terminating '\0' character is still where it expects, nothing further is
+// done. If it has been overwritten, the kernel will scan up to the size of
+// a page looking for another.
 //
 // Thus to change the process title, we must move any environment variables out
 // of the way to make room for a potentially longer title, and then overwrite
 // the memory pointed to by argv[0] with a single replacement string, making
 // sure its size does not exceed the available space.
 //
+// See the following kernel commit for the details of the contract between
+// kernel and setproctitle:
+// https://github.com/torvalds/linux/commit/2954152298c37804dab49d630aa959625b50cf64
+//
 // It is perhaps worth noting that patches to add a system call to Linux for
 // this, like in BSD, have never made it in: this is the "official" way to do
 // this on Linux. Presumably it is not in glibc due to some disagreement over
@@ -46,71 +46,129 @@
 #include <string.h>
 #include <unistd.h>
 
+#include <string>
+
+#include "base/files/file_util.h"
+#include "base/logging.h"
+
 extern char** environ;
 
-static char** g_main_argv = NULL;
-static char* g_orig_argv0 = NULL;
+// g_orig_argv0 is the original process name found in argv[0].
+// It is set to a copy of argv[0] in setproctitle_init. It is nullptr if
+// setproctitle_init was unsuccessful or not called.
+static char* g_orig_argv0 = nullptr;
+
+// Following pointers hold the initial argv/envp memory range.
+// They are initialized in setproctitle_init and are used to overwrite the
+// argv/envp memory range with a new process title to be read by the kernel.
+// They are nullptr if setproctitle_init was unsuccessful or not called.
+// Note that g_envp_start is not necessary because it is the same as g_argv_end.
+static char* g_argv_start = nullptr;
+static char* g_argv_end = nullptr;
+static char* g_envp_end = nullptr;
 
 void setproctitle(const char* fmt, ...) {
   va_list ap;
-  size_t i, avail_size;
-  uintptr_t page_size, page, page_end;
+
   // Sanity check before we try and set the process title.
   // The BSD version allows fmt == NULL to restore the original title.
-  if (!g_main_argv || !environ || !fmt)
+  if (!g_orig_argv0 || !fmt)
     return;
-  if (!g_orig_argv0) {
-    // Save the original argv[0].
-    g_orig_argv0 = strdup(g_main_argv[0]);
-    if (!g_orig_argv0)
-      return;
-  }
-  page_size = sysconf(_SC_PAGESIZE);
-  // Get the page on which the argument list and environment live.
-  page = (uintptr_t)g_main_argv[0];
-  page -= page % page_size;
-  page_end = page + page_size;
-  // Move the environment out of the way. Note that we are moving the values,
-  // not the environment array itself (which may not be on the page we need
-  // to overwrite anyway).
-  for (i = 0; environ[i]; ++i) {
-    uintptr_t env_i = (uintptr_t)environ[i];
-    // Only move the value if it's actually in the way. This avoids
-    // leaking copies of the values if this function is called again.
-    if (page <= env_i && env_i < page_end) {
-      char* copy = strdup(environ[i]);
-      // Be paranoid. Check for allocation failure and bail out.
-      if (!copy)
-        return;
-      environ[i] = copy;
+
+  // The title can be up to the end of envp.
+  const size_t avail_size = g_envp_end - g_argv_start - 1;
+
+  // Linux 4.18--5.2 have a bug where we can never set a process title
+  // shorter than the initial argv. Check if the bug exists in the current
+  // kernel on the first call of setproctitle.
+  static const bool buggy_kernel = [avail_size]() {
+    // Attempt to set an empty title. This will set cmdline to:
+    // ""                   (on Linux --4.17)
+    // "\0\0\0...\0\0\0.\0" (on Linux 4.18--5.2)
+    // "\0"                 (on Linux 5.3--)
+    memset(g_argv_start, 0, avail_size + 1);
+    g_argv_end[-1] = '.';
+
+    std::string cmdline;
+    if (!base::ReadFileToString(base::FilePath("/proc/self/cmdline"),
+                                &cmdline)) {
+      return false;
     }
-  }
-  // Put the title in argv[0]. We have to zero out the space first since the
-  // kernel doesn't actually look for a null terminator unless we make the
-  // argument list longer than it started.
-  avail_size = page_end - (uintptr_t)g_main_argv[0];
-  memset(g_main_argv[0], 0, avail_size);
+    return cmdline.size() >= 2;
+  }();
+
+  memset(g_argv_start, 0, avail_size + 1);
+
+  size_t size;
   va_start(ap, fmt);
   if (fmt[0] == '-') {
-    vsnprintf(g_main_argv[0], avail_size, &fmt[1], ap);
+    size = vsnprintf(g_argv_start, avail_size, &fmt[1], ap);
   } else {
-    size_t size = snprintf(g_main_argv[0], avail_size, "%s ", g_orig_argv0);
+    size = snprintf(g_argv_start, avail_size, "%s ", g_orig_argv0);
     if (size < avail_size)
-      vsnprintf(g_main_argv[0] + size, avail_size - size, fmt, ap);
+      size += vsnprintf(&g_argv_start[size], avail_size - size, fmt, ap);
   }
   va_end(ap);
-  g_main_argv[1] = NULL;
+
+  // Kernel looks for a null terminator instead of the initial argv space
+  // when the end of the space is not terminated with a null.
+  // https://github.com/torvalds/linux/commit/d26d0cd97c88eb1a5704b42e41ab443406807810
+  //
+  // If the length of the new title is shorter than the original argv space,
+  // set the last byte of the space to an arbitrary non-null character to tell
+  // the kernel that setproctitle was called.
+  //
+  // On buggy kernels we can never make the process title shorter than the
+  // initial argv. In that case, just leave the remaining bytes filled with
+  // null characters.
+  const size_t argv_size = g_argv_end - g_argv_start - 1;
+  if (!buggy_kernel && size < argv_size)
+    g_argv_end[-1] = '.';
 }
 
 // A version of this built into glibc would not need this function, since
 // it could stash the argv pointer in __libc_start_main(). But we need it.
 void setproctitle_init(const char** main_argv) {
-  if (g_main_argv)
+  static bool init_called = false;
+  if (init_called)
+    return;
+  init_called = true;
+
+  if (!main_argv)
+    return;
+
+  // Verify that the memory layout matches expectation.
+  char** argv = const_cast<char**>(main_argv);
+  char* argv_start = argv[0];
+  char* p = argv_start;
+  for (size_t i = 0; argv[i]; ++i) {
+    if (p != argv[i])
+      return;
+    p += strlen(p) + 1;
+  }
+  char* argv_end = p;
+  for (size_t i = 0; environ[i]; ++i) {
+    if (p != environ[i])
+      return;
+    p += strlen(p) + 1;
+  }
+  char* envp_end = p;
+
+  // Move the environment out of the way. Note that we are moving the values,
+  // not the environment array itself.
+  for (size_t i = 0; environ[i]; ++i) {
+    char* copy = strdup(environ[i]);
+    if (!copy)
+      return;
+    environ[i] = copy;
+  }
+
+  char* argv0 = strdup(argv[0]);
+  if (!argv[0])
     return;
 
-  uintptr_t page_size = sysconf(_SC_PAGESIZE);
-  // Check that the argv array is in fact on the same page of memory
-  // as the environment array just as an added measure of protection.
-  if (((uintptr_t)environ) / page_size == ((uintptr_t)main_argv) / page_size)
-    g_main_argv = const_cast<char**>(main_argv);
+  g_orig_argv0 = argv0;
+  g_argv_start = argv_start;
+  g_argv_end = argv_end;
+  g_envp_end = envp_end;
 }

services/service_manager/embedder/set_process_title_linux_unittest.cc

+// Copyright 2020 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include <string.h>
+#include <unistd.h>
+
+#include "base/files/file_path.h"
+#include "base/files/file_util.h"
+#include "base/strings/string_util.h"
+#include "services/service_manager/embedder/set_process_title_linux.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace {
+
+const std::string kNullChr(1, '\0');
+
+std::string ReadCmdline() {
+  std::string cmdline;
+  CHECK(base::ReadFileToString(base::FilePath("/proc/self/cmdline"), &cmdline));
+  // The process title appears in different formats depending on Linux kernel
+  // version:
+  // "title"            (on Linux --4.17)
+  // "title\0\0\0...\0" (on Linux 4.18--5.2)
+  // "title\0"          (on Linux 5.3--)
+  //
+  // For unit tests, just trim trailing null characters to support all cases.
+  return base::TrimString(cmdline, kNullChr, base::TRIM_TRAILING).as_string();
+}
+
+TEST(SetProcTitleLinuxTest, Simple) {
+  setproctitle("a %s cat", "cute");
+  EXPECT_TRUE(base::EndsWith(ReadCmdline(), " a cute cat",
+                             base::CompareCase::SENSITIVE))
+      << ReadCmdline();
+
+  setproctitle("-a %s cat", "cute");
+  EXPECT_EQ(ReadCmdline(), "a cute cat");
+}
+
+TEST(SetProcTitleLinuxTest, Empty) {
+  setproctitle("-");
+  EXPECT_EQ(ReadCmdline(), "");
+}
+
+TEST(SetProcTitleLinuxTest, Long) {
+  setproctitle("-long cat is l%0100000dng", 0);
+  EXPECT_TRUE(base::StartsWith(ReadCmdline(), "long cat is l00000000",
+                               base::CompareCase::SENSITIVE))
+      << ReadCmdline();
+}
+
+}  // namespace