From fbfa84045d30a8288c3feab9d7ce7c0c20d7728b Mon Sep 17 00:00:00 2001
From: Yabin Cui <yabinc@google.com>
Date: Fri, 30 Oct 2015 18:37:26 -0700
Subject: [PATCH] adb: run reverse_service() in main thread.

reverse_service() calls handle_forward_request(), which calls
functions in fdevent.cpp. fdevent functions is only supposed
to be called in the main thread.
Add check in fdevent.cpp to make sure all operations come from
main thread.

Bug: 25355808
Change-Id: Iceb9273f3056acc0713eaafe086ac950ca80ff4f
---
 adb/fdevent.cpp  | 22 ++++++++++++++++++++++
 adb/services.cpp | 29 ++++++++++++-----------------
 2 files changed, 34 insertions(+), 17 deletions(-)

diff --git a/adb/fdevent.cpp b/adb/fdevent.cpp
index ddd15a2b5..4458c85ec 100644
--- a/adb/fdevent.cpp
+++ b/adb/fdevent.cpp
@@ -72,6 +72,19 @@ struct PollNode {
 // That's why we don't need a lock for fdevent.
 static std::unordered_map<int, PollNode> g_poll_node_map;
 static std::list<fdevent*> g_pending_list;
+static bool main_thread_valid;
+static pthread_t main_thread;
+
+static void check_main_thread() {
+    if (main_thread_valid) {
+        CHECK_NE(0, pthread_equal(main_thread, pthread_self()));
+    }
+}
+
+static void set_main_thread() {
+    main_thread_valid = true;
+    main_thread = pthread_self();
+}
 
 static std::string dump_fde(const fdevent* fde) {
     std::string state;
@@ -101,6 +114,7 @@ static std::string dump_fde(const fdevent* fde) {
 
 fdevent *fdevent_create(int fd, fd_func func, void *arg)
 {
+    check_main_thread();
     fdevent *fde = (fdevent*) malloc(sizeof(fdevent));
     if(fde == 0) return 0;
     fdevent_install(fde, fd, func, arg);
@@ -110,6 +124,7 @@ fdevent *fdevent_create(int fd, fd_func func, void *arg)
 
 void fdevent_destroy(fdevent *fde)
 {
+    check_main_thread();
     if(fde == 0) return;
     if(!(fde->state & FDE_CREATED)) {
         LOG(FATAL) << "destroying fde not created by fdevent_create(): " << dump_fde(fde);
@@ -119,6 +134,7 @@ void fdevent_destroy(fdevent *fde)
 }
 
 void fdevent_install(fdevent* fde, int fd, fd_func func, void* arg) {
+    check_main_thread();
     CHECK_GE(fd, 0);
     memset(fde, 0, sizeof(fdevent));
     fde->state = FDE_ACTIVE;
@@ -137,6 +153,7 @@ void fdevent_install(fdevent* fde, int fd, fd_func func, void* arg) {
 }
 
 void fdevent_remove(fdevent* fde) {
+    check_main_thread();
     D("fdevent_remove %s", dump_fde(fde).c_str());
     if (fde->state & FDE_ACTIVE) {
         g_poll_node_map.erase(fde->fd);
@@ -171,6 +188,7 @@ static void fdevent_update(fdevent* fde, unsigned events) {
 }
 
 void fdevent_set(fdevent* fde, unsigned events) {
+    check_main_thread();
     events &= FDE_EVENTMASK;
     if ((fde->state & FDE_EVENTMASK) == events) {
         return;
@@ -190,10 +208,12 @@ void fdevent_set(fdevent* fde, unsigned events) {
 }
 
 void fdevent_add(fdevent* fde, unsigned events) {
+    check_main_thread();
     fdevent_set(fde, (fde->state & FDE_EVENTMASK) | events);
 }
 
 void fdevent_del(fdevent* fde, unsigned events) {
+    check_main_thread();
     fdevent_set(fde, (fde->state & FDE_EVENTMASK) & ~events);
 }
 
@@ -335,6 +355,7 @@ void fdevent_subproc_setup()
 
 void fdevent_loop()
 {
+    set_main_thread();
 #if !ADB_HOST
     fdevent_subproc_setup();
 #endif // !ADB_HOST
@@ -359,4 +380,5 @@ size_t fdevent_installed_count() {
 void fdevent_reset() {
     g_poll_node_map.clear();
     g_pending_list.clear();
+    main_thread_valid = false;
 }
diff --git a/adb/services.cpp b/adb/services.cpp
index e24b4705e..19a6726f8 100644
--- a/adb/services.cpp
+++ b/adb/services.cpp
@@ -184,15 +184,18 @@ void reboot_service(int fd, void* arg)
     adb_close(fd);
 }
 
-void reverse_service(int fd, void* arg)
-{
-    const char* command = reinterpret_cast<const char*>(arg);
-
-    if (handle_forward_request(command, kTransportAny, NULL, fd) < 0) {
-        SendFail(fd, "not a reverse forwarding command");
+int reverse_service(const char* command) {
+    int s[2];
+    if (adb_socketpair(s)) {
+        PLOG(ERROR) << "cannot create service socket pair.";
+        return -1;
     }
-    free(arg);
-    adb_close(fd);
+    VLOG(SERVICES) << "service socketpair: " << s[0] << ", " << s[1];
+    if (handle_forward_request(command, kTransportAny, nullptr, s[1]) < 0) {
+        SendFail(s[1], "not a reverse forwarding command");
+    }
+    adb_close(s[1]);
+    return s[0];
 }
 
 // Shell service string can look like:
@@ -335,15 +338,7 @@ int service_to_fd(const char* name, const atransport* transport) {
     } else if(!strncmp(name, "usb:", 4)) {
         ret = create_service_thread(restart_usb_service, NULL);
     } else if (!strncmp(name, "reverse:", 8)) {
-        char* cookie = strdup(name + 8);
-        if (cookie == NULL) {
-            ret = -1;
-        } else {
-            ret = create_service_thread(reverse_service, cookie);
-            if (ret < 0) {
-                free(cookie);
-            }
-        }
+        ret = reverse_service(name + 8);
     } else if(!strncmp(name, "disable-verity:", 15)) {
         ret = create_service_thread(set_verity_enabled_state_service, (void*)0);
     } else if(!strncmp(name, "enable-verity:", 15)) {