コミット: acd1876a1e313632d7554585c314552c2d809848 - system-core (git)

system/core

コミットメタ情報

• リビジョン: acd1876a1e313632d7554585c314552c2d809848 (tree)
• 日時: 2016-09-15 18:07:50
• 作者: Linux Build Service Account <lnxbuild@loca...>
• コミッター: Linux Build Service Account

ログメッセージ

Promotion of android-framework.lnx.2.0-00009.

CRs Change ID Subject

---

1059099 I4dfe1ea30e2c211d2389321bd66e3684dd757591 debuggerd: verify that traced threads belong to the righ
1065271 I7878b89a32f0950aa43d3157943982d403632502 init: Add support for slow devices
1064124 I842ce6f62a85b19a79bef4c082e132d1c9649485 init: Fix for handling -p argument in the write command
1050321 I43d0eb850cfff24bbd61b0b1e0f43f591b71e3fe init : start time_daemon when trigger_restart_min_framew

Change-Id: I176768c33df59eb04f34b01cad67fffa0169bdc0
CRs-Fixed: 1059099, 1050321, 1065271, 1064124

変更サマリ

• modified: debuggerd/debuggerd.cpp
• modified: init/Android.mk
• modified: init/builtins.cpp
• modified: init/init.cpp
• modified: rootdir/init.rc

差分

--- a/debuggerd/debuggerd.cpp
+++ b/debuggerd/debuggerd.cpp
@@ -183,6 +183,16 @@ out:
    return allowed;
 }
 
+static bool pid_contains_tid(pid_t pid, pid_t tid) {
+  char task_path[PATH_MAX];
+  if (snprintf(task_path, PATH_MAX, "/proc/%d/task/%d", pid, tid) >= PATH_MAX) {
+    ALOGE("debuggerd: task path overflow (pid = %d, tid = %d)\n", pid, tid);
+    exit(1);
+  }
+
+  return access(task_path, F_OK) == 0;
+}
+
 static int read_request(int fd, debugger_request_t* out_request) {
   ucred cr;
   socklen_t len = sizeof(cr);
@@ -227,16 +237,13 @@ static int read_request(int fd, debugger_request_t* out_request) {
 
   if (msg.action == DEBUGGER_ACTION_CRASH) {
     // Ensure that the tid reported by the crashing process is valid.
-    char buf[64];
-    struct stat s;
-    snprintf(buf, sizeof buf, "/proc/%d/task/%d", out_request->pid, out_request->tid);
-    if (stat(buf, &s)) {
-      ALOGE("tid %d does not exist in pid %d. ignoring debug request\n",
-          out_request->tid, out_request->pid);
+    // This check needs to happen again after ptracing the requested thread to prevent a race.
+    if (!pid_contains_tid(out_request->pid, out_request->tid)) {
+      ALOGE("tid %d does not exist in pid %d. ignoring debug request\n", out_request->tid,
+            out_request->pid);
       return -1;
     }
-  } else if (cr.uid == 0
-            || (cr.uid == AID_SYSTEM && msg.action == DEBUGGER_ACTION_DUMP_BACKTRACE)) {
+  } else if (cr.uid == 0 || (cr.uid == AID_SYSTEM && msg.action == DEBUGGER_ACTION_DUMP_BACKTRACE)) {
     // Only root or system can ask us to attach to any process and dump it explicitly.
     // However, system is only allowed to collect backtraces but cannot dump tombstones.
     status = get_process_info(out_request->tid, &out_request->pid,
@@ -413,10 +420,31 @@ static void redirect_to_32(int fd, debugger_request_t* request) {
 }
 #endif
 
+// Attach to a thread, and verify that it's still a member of the given process
+static bool ptrace_attach_thread(pid_t pid, pid_t tid) {
+  if (ptrace(PTRACE_ATTACH, tid, 0, 0) != 0) {
+    return false;
+  }
+
+  // Make sure that the task we attached to is actually part of the pid we're dumping.
+  if (!pid_contains_tid(pid, tid)) {
+    if (ptrace(PTRACE_DETACH, tid, 0, 0) != 0) {
+      ALOGE("debuggerd: failed to detach from thread '%d'", tid);
+      exit(1);
+    }
+    return false;
+  }
+
+  return true;
+}
+
 static void ptrace_siblings(pid_t pid, pid_t main_tid, std::set<pid_t>& tids) {
-  char task_path[64];
+  char task_path[PATH_MAX];
 
-  snprintf(task_path, sizeof(task_path), "/proc/%d/task", pid);
+  if (snprintf(task_path, PATH_MAX, "/proc/%d/task", pid) >= PATH_MAX) {
+    ALOGE("debuggerd: task path overflow (pid = %d)\n", pid);
+    abort();
+  }
 
   std::unique_ptr<DIR, int (*)(DIR*)> d(opendir(task_path), closedir);
 
@@ -443,7 +471,7 @@ static void ptrace_siblings(pid_t pid, pid_t main_tid, std::set<pid_t>& tids) {
       continue;
     }
 
-    if (ptrace(PTRACE_ATTACH, tid, 0, 0) < 0) {
+    if (!ptrace_attach_thread(pid, tid)) {
       ALOGE("debuggerd: ptrace attach to %d failed: %s", tid, strerror(errno));
       continue;
     }
@@ -568,11 +596,33 @@ static void worker_process(int fd, debugger_request_t& request) {
   // debugger_signal_handler().
 
   // Attach to the target process.
-  if (ptrace(PTRACE_ATTACH, request.tid, 0, 0) != 0) {
+  if (!ptrace_attach_thread(request.pid, request.tid)) {
     ALOGE("debuggerd: ptrace attach failed: %s", strerror(errno));
     exit(1);
   }
 
+  // DEBUGGER_ACTION_CRASH requests can come from arbitrary processes and the tid field in the
+  // request is sent from the other side. If an attacker can cause a process to be spawned with the
+  // pid of their process, they could trick debuggerd into dumping that process by exiting after
+  // sending the request. Validate the trusted request.uid/gid to defend against this.
+  if (request.action == DEBUGGER_ACTION_CRASH) {
+    pid_t pid;
+    uid_t uid;
+    gid_t gid;
+    if (get_process_info(request.tid, &pid, &uid, &gid) != 0) {
+      ALOGE("debuggerd: failed to get process info for tid '%d'", request.tid);
+      exit(1);
+    }
+
+    if (pid != request.pid || uid != request.uid || gid != request.gid) {
+      ALOGE(
+        "debuggerd: attached task %d does not match request: "
+        "expected pid=%d,uid=%d,gid=%d, actual pid=%d,uid=%d,gid=%d",
+        request.tid, request.pid, request.uid, request.gid, pid, uid, gid);
+      exit(1);
+    }
+  }
+
   // Don't attach to the sibling threads if we want to attach gdb.
   // Supposedly, it makes the process less reliable.
   bool attach_gdb = should_attach_gdb(request);

--- a/init/Android.mk
+++ b/init/Android.mk
@@ -12,6 +12,14 @@ endif
 
 init_options += -DLOG_UEVENTS=0
 
+ifneq ($(TARGET_INIT_COLDBOOT_TIMEOUT),)
+init_options += -DCOLDBOOT_TIMEOUT_OVERRIDE=$(TARGET_INIT_COLDBOOT_TIMEOUT)
+endif
+
+ifneq ($(TARGET_INIT_CONSOLE_TIMEOUT),)
+init_options += -DCONSOLE_TIMEOUT_SEC=$(TARGET_INIT_CONSOLE_TIMEOUT)
+endif
+
 init_cflags += \
     $(init_options) \
     -Wall -Wextra \

--- a/init/builtins.cpp
+++ b/init/builtins.cpp
@@ -822,8 +822,8 @@ static int do_write(const std::vector<std::string>& args) {
             break;
         } else {
             //Value at current index is -p. The next index would
-            //be the prefix.Skip beyond that.
-            i+=2;
+            //be the prefix.Skip it.
+            i++;
         }
     }
     if (!path || !value) {

--- a/init/init.cpp
+++ b/init/init.cpp
@@ -160,15 +160,19 @@ void handle_control_message(const std::string& msg, const std::string& name) {
 
 static int wait_for_coldboot_done_action(const std::vector<std::string>& args) {
     Timer t;
-
+    int timeout = 0;
+#ifdef COLDBOOT_TIMEOUT_OVERRIDE
+    timeout = COLDBOOT_TIMEOUT_OVERRIDE;
+#else
+    timeout = 5;
+#endif
     NOTICE("Waiting for %s...\n", COLDBOOT_DONE);
     // Any longer than 1s is an unreasonable length of time to delay booting.
     // If you're hitting this timeout, check that you didn't make your
     // sepolicy regular expressions too expensive (http://b/19899875).
-    if (wait_for_file(COLDBOOT_DONE, 5)) {
+    if (wait_for_file(COLDBOOT_DONE, timeout)) {
         ERROR("Timed out waiting for %s\n", COLDBOOT_DONE);
     }
-
     NOTICE("Waiting for %s took %.2fs.\n", COLDBOOT_DONE, t.duration());
     return 0;
 }
@@ -258,15 +262,31 @@ static int keychord_init_action(const std::vector<std::string>& args)
 
 static int console_init_action(const std::vector<std::string>& args)
 {
+    int fd = -1;
     std::string console = property_get("ro.boot.console");
     if (!console.empty()) {
         console_name = "/dev/" + console;
     }
-
-    int fd = open(console_name.c_str(), O_RDWR | O_CLOEXEC);
+    fd = open(console_name.c_str(), O_RDWR | O_CLOEXEC);
+#ifdef CONSOLE_TIMEOUT_SEC
+    int num_console_retries = 0;
+    while ((fd < 0) && num_console_retries++ < CONSOLE_TIMEOUT_SEC) {
+        ERROR("Failed to open console device %s(%s)..Retrying\n",
+                       console_name.c_str(), strerror(errno));
+        sleep(1);
+        fd = open(console_name.c_str(), O_RDWR | O_CLOEXEC);
+    }
     if (fd >= 0)
+        INFO("Console device located");
+#endif
+    if (fd >= 0) {
         have_console = 1;
-    close(fd);
+        close(fd);
+    } else {
+        ERROR("console init failed. Open on Console device %s failed(%s)\n",
+                         console.c_str(), strerror(errno));
+        return 0;
+    }
 
     fd = open("/dev/tty0", O_WRONLY | O_CLOEXEC);
     if (fd >= 0) {

--- a/rootdir/init.rc
+++ b/rootdir/init.rc
@@ -580,6 +580,7 @@ on property:vold.decrypt=trigger_restart_min_framework
     # A/B update verifier that marks a successful boot.
     exec - root -- /system/bin/update_verifier trigger_restart_min_framework
     class_start main
+    start time_daemon
 
 on property:vold.decrypt=trigger_restart_framework
     # A/B update verifier that marks a successful boot.