From 9f4861155447ce813d06f39ea7b739837b1a4e2d Mon Sep 17 00:00:00 2001
From: Josh Gao <jmgao@google.com>
Date: Tue, 23 Feb 2016 18:05:57 -0800
Subject: [PATCH] adb: fix leak of framework_fd.

Move the fdevent for the framework authentication connection out of
atransport into its own static variable in adb_auth_client, since its
lifetime is completely unrelated to that of the USB connection.

Bug: http://b/27297963
Change-Id: Ie6180d0b59d133120c5755e239e76ab33ed3cc1d
---
 adb/adb_auth_client.cpp | 37 +++++++++++++++++++++----------------
 adb/transport.h         |  2 --
 adb/transport_test.cpp  |  1 -
 3 files changed, 21 insertions(+), 19 deletions(-)

diff --git a/adb/adb_auth_client.cpp b/adb/adb_auth_client.cpp
index c4ffc85a2..8ef9948e9 100644
--- a/adb/adb_auth_client.cpp
+++ b/adb/adb_auth_client.cpp
@@ -44,6 +44,7 @@ static const char *key_paths[] = {
 };
 
 static fdevent listener_fde;
+static fdevent framework_fde;
 static int framework_fd = -1;
 
 static void usb_disconnected(void* unused, atransport* t);
@@ -161,29 +162,30 @@ int adb_auth_verify(uint8_t* token, uint8_t* sig, int siglen)
     return ret;
 }
 
-static void usb_disconnected(void* unused, atransport* t)
-{
+static void usb_disconnected(void* unused, atransport* t) {
     D("USB disconnect");
     usb_transport = NULL;
     needs_retry = false;
 }
 
-static void adb_auth_event(int fd, unsigned events, void *data)
-{
+static void framework_disconnected() {
+    D("Framework disconnect");
+    fdevent_remove(&framework_fde);
+    framework_fd = -1;
+}
+
+static void adb_auth_event(int fd, unsigned events, void*) {
     char response[2];
     int ret;
 
     if (events & FDE_READ) {
         ret = unix_read(fd, response, sizeof(response));
         if (ret <= 0) {
-            D("Framework disconnect");
-            if (usb_transport)
-                fdevent_remove(&usb_transport->auth_fde);
-            framework_fd = -1;
-        }
-        else if (ret == 2 && response[0] == 'O' && response[1] == 'K') {
-            if (usb_transport)
+            framework_disconnected();
+        } else if (ret == 2 && response[0] == 'O' && response[1] == 'K') {
+            if (usb_transport) {
                 adb_auth_verified(usb_transport);
+            }
         }
     }
 }
@@ -221,13 +223,9 @@ void adb_auth_confirm_key(unsigned char *key, size_t len, atransport *t)
         D("Failed to write PK, errno=%d", errno);
         return;
     }
-
-    fdevent_install(&t->auth_fde, framework_fd, adb_auth_event, t);
-    fdevent_add(&t->auth_fde, FDE_READ);
 }
 
-static void adb_auth_listener(int fd, unsigned events, void *data)
-{
+static void adb_auth_listener(int fd, unsigned events, void* data) {
     sockaddr_storage addr;
     socklen_t alen;
     int s;
@@ -240,7 +238,14 @@ static void adb_auth_listener(int fd, unsigned events, void *data)
         return;
     }
 
+    if (framework_fd >= 0) {
+        PLOG(WARNING) << "adb received framework auth socket connection again";
+        framework_disconnected();
+    }
+
     framework_fd = s;
+    fdevent_install(&framework_fde, framework_fd, adb_auth_event, nullptr);
+    fdevent_add(&framework_fde, FDE_READ);
 
     if (needs_retry) {
         needs_retry = false;
diff --git a/adb/transport.h b/adb/transport.h
index 76d6afa14..4c0c00887 100644
--- a/adb/transport.h
+++ b/adb/transport.h
@@ -50,7 +50,6 @@ public:
     // it's better to do this piece by piece.
 
     atransport() {
-        auth_fde = {};
         transport_fde = {};
         protocol_version = A_VERSION;
         max_payload = MAX_PAYLOAD;
@@ -87,7 +86,6 @@ public:
 
     void* key = nullptr;
     unsigned char token[TOKEN_SIZE] = {};
-    fdevent auth_fde;
     size_t failed_auth_attempts = 0;
 
     const std::string connection_state_name() const;
diff --git a/adb/transport_test.cpp b/adb/transport_test.cpp
index 97fc0698d..1bdea2a57 100644
--- a/adb/transport_test.cpp
+++ b/adb/transport_test.cpp
@@ -53,7 +53,6 @@ public:
 
         EXPECT_EQ(key, rhs.key);
         EXPECT_EQ(0, memcmp(token, rhs.token, TOKEN_SIZE));
-        EXPECT_EQ(0, memcmp(&auth_fde, &rhs.auth_fde, sizeof(fdevent)));
         EXPECT_EQ(failed_auth_attempts, rhs.failed_auth_attempts);
 
         EXPECT_EQ(features(), rhs.features());