Revert "Reland "adbd: don't close ep0 until we receive FUNCTIONFS_UNBIND.""

This reverts commit c33aee8ac1c0958df32dcd2db27ca3d0d1badbd1.

Bug: http://b/135753060
Test: treehugger
Change-Id: Ic83a44e7c8f860a6c044ca20ec81f365a8a6ba4b

3 files changed, 112 insertions, 111 deletions

diff --git a/adb/daemon/include/adbd/usb.h b/adb/daemon/include/adbd/usb.h
index 15ba97c59..fca3c58e8 100644
--- a/adb/daemon/include/adbd/usb.h
+++ b/adb/daemon/include/adbd/usb.h
@@ -16,8 +16,6 @@
  * limitations under the License.
  */
 
-#include <linux/usb/functionfs.h>
-
 #include <atomic>
 #include <condition_variable>
 #include <mutex>
@@ -64,9 +62,5 @@ struct usb_handle {
 };
 
 usb_handle *create_usb_handle(unsigned num_bufs, unsigned io_size);
-
-struct usb_functionfs_event;
-const char* ffs_event_to_string(enum usb_functionfs_event_type type);
-bool read_functionfs_setup(int fd, usb_functionfs_event* event);
 bool open_functionfs(android::base::unique_fd* control, android::base::unique_fd* bulk_out,
                      android::base::unique_fd* bulk_in);
diff --git a/adb/daemon/usb.cpp b/adb/daemon/usb.cpp
index 48fa771d2..f4aa9fbf2 100644
--- a/adb/daemon/usb.cpp
+++ b/adb/daemon/usb.cpp
@@ -66,6 +66,25 @@ static constexpr size_t kUsbReadSize = 4 * PAGE_SIZE;
 static constexpr size_t kUsbWriteQueueDepth = 8;
 static constexpr size_t kUsbWriteSize = 4 * PAGE_SIZE;
 
+static const char* to_string(enum usb_functionfs_event_type type) {
+    switch (type) {
+        case FUNCTIONFS_BIND:
+            return "FUNCTIONFS_BIND";
+        case FUNCTIONFS_UNBIND:
+            return "FUNCTIONFS_UNBIND";
+        case FUNCTIONFS_ENABLE:
+            return "FUNCTIONFS_ENABLE";
+        case FUNCTIONFS_DISABLE:
+            return "FUNCTIONFS_DISABLE";
+        case FUNCTIONFS_SETUP:
+            return "FUNCTIONFS_SETUP";
+        case FUNCTIONFS_SUSPEND:
+            return "FUNCTIONFS_SUSPEND";
+        case FUNCTIONFS_RESUME:
+            return "FUNCTIONFS_RESUME";
+    }
+}
+
 enum class TransferDirection : uint64_t {
     READ = 0,
     WRITE = 1,
@@ -150,12 +169,12 @@ struct ScopedAioContext {
 };
 
 struct UsbFfsConnection : public Connection {
-    UsbFfsConnection(unique_fd* control, unique_fd read, unique_fd write,
+    UsbFfsConnection(unique_fd control, unique_fd read, unique_fd write,
                      std::promise<void> destruction_notifier)
         : worker_started_(false),
           stopped_(false),
           destruction_notifier_(std::move(destruction_notifier)),
-          control_fd_(control),
+          control_fd_(std::move(control)),
           read_fd_(std::move(read)),
           write_fd_(std::move(write)) {
         LOG(INFO) << "UsbFfsConnection constructed";
@@ -164,6 +183,11 @@ struct UsbFfsConnection : public Connection {
             PLOG(FATAL) << "failed to create eventfd";
         }
 
+        monitor_event_fd_.reset(eventfd(0, EFD_CLOEXEC));
+        if (monitor_event_fd_ == -1) {
+            PLOG(FATAL) << "failed to create eventfd";
+        }
+
         aio_context_ = ScopedAioContext::Create(kUsbReadQueueDepth + kUsbWriteQueueDepth);
     }
 
@@ -175,6 +199,7 @@ struct UsbFfsConnection : public Connection {
         // We need to explicitly close our file descriptors before we notify our destruction,
         // because the thread listening on the future will immediately try to reopen the endpoint.
         aio_context_.reset();
+        control_fd_.reset();
         read_fd_.reset();
         write_fd_.reset();
 
@@ -221,6 +246,13 @@ struct UsbFfsConnection : public Connection {
             PLOG(FATAL) << "failed to notify worker eventfd to stop UsbFfsConnection";
         }
         CHECK_EQ(static_cast<size_t>(rc), sizeof(notify));
+
+        rc = adb_write(monitor_event_fd_.get(), &notify, sizeof(notify));
+        if (rc < 0) {
+            PLOG(FATAL) << "failed to notify monitor eventfd to stop UsbFfsConnection";
+        }
+
+        CHECK_EQ(static_cast<size_t>(rc), sizeof(notify));
     }
 
   private:
@@ -239,24 +271,33 @@ struct UsbFfsConnection : public Connection {
         monitor_thread_ = std::thread([this]() {
             adb_thread_setname("UsbFfs-monitor");
 
+            bool bound = false;
             bool enabled = false;
             bool running = true;
             while (running) {
                 adb_pollfd pfd[2] = {
-                        {.fd = control_fd_->get(), .events = POLLIN, .revents = 0},
+                  { .fd = control_fd_.get(), .events = POLLIN, .revents = 0 },
+                  { .fd = monitor_event_fd_.get(), .events = POLLIN, .revents = 0 },
                 };
 
-                int rc = TEMP_FAILURE_RETRY(adb_poll(pfd, 2, -1));
+                // If we don't see our first bind within a second, try again.
+                int timeout_ms = bound ? -1 : 1000;
+
+                int rc = TEMP_FAILURE_RETRY(adb_poll(pfd, 2, timeout_ms));
                 if (rc == -1) {
                     PLOG(FATAL) << "poll on USB control fd failed";
+                } else if (rc == 0) {
+                    LOG(WARNING) << "timed out while waiting for FUNCTIONFS_BIND, trying again";
+                    break;
                 }
 
                 if (pfd[1].revents) {
-                    // We were told to die, continue reading until FUNCTIONFS_UNBIND.
+                    // We were told to die.
+                    break;
                 }
 
                 struct usb_functionfs_event event;
-                rc = TEMP_FAILURE_RETRY(adb_read(control_fd_->get(), &event, sizeof(event)));
+                rc = TEMP_FAILURE_RETRY(adb_read(control_fd_.get(), &event, sizeof(event)));
                 if (rc == -1) {
                     PLOG(FATAL) << "failed to read functionfs event";
                 } else if (rc == 0) {
@@ -268,15 +309,32 @@ struct UsbFfsConnection : public Connection {
                 }
 
                 LOG(INFO) << "USB event: "
-                          << ffs_event_to_string(
-                                     static_cast<usb_functionfs_event_type>(event.type));
+                          << to_string(static_cast<usb_functionfs_event_type>(event.type));
 
                 switch (event.type) {
                     case FUNCTIONFS_BIND:
-                        LOG(FATAL) << "received FUNCTIONFS_BIND after already opened?";
+                        if (bound) {
+                            LOG(WARNING) << "received FUNCTIONFS_BIND while already bound?";
+                            running = false;
+                            break;
+                        }
+
+                        if (enabled) {
+                            LOG(WARNING) << "received FUNCTIONFS_BIND while already enabled?";
+                            running = false;
+                            break;
+                        }
+
+                        bound = true;
                         break;
 
                     case FUNCTIONFS_ENABLE:
+                        if (!bound) {
+                            LOG(WARNING) << "received FUNCTIONFS_ENABLE while not bound?";
+                            running = false;
+                            break;
+                        }
+
                         if (enabled) {
                             LOG(WARNING) << "received FUNCTIONFS_ENABLE while already enabled?";
                             running = false;
@@ -288,6 +346,10 @@ struct UsbFfsConnection : public Connection {
                         break;
 
                     case FUNCTIONFS_DISABLE:
+                        if (!bound) {
+                            LOG(WARNING) << "received FUNCTIONFS_DISABLE while not bound?";
+                        }
+
                         if (!enabled) {
                             LOG(WARNING) << "received FUNCTIONFS_DISABLE while not enabled?";
                         }
@@ -301,12 +363,44 @@ struct UsbFfsConnection : public Connection {
                             LOG(WARNING) << "received FUNCTIONFS_UNBIND while still enabled?";
                         }
 
+                        if (!bound) {
+                            LOG(WARNING) << "received FUNCTIONFS_UNBIND when not bound?";
+                        }
+
+                        bound = false;
                         running = false;
                         break;
 
                     case FUNCTIONFS_SETUP: {
-                        read_functionfs_setup(*control_fd_, &event);
-                        break;
+                        LOG(INFO) << "received FUNCTIONFS_SETUP control transfer: bRequestType = "
+                                  << static_cast<int>(event.u.setup.bRequestType)
+                                  << ", bRequest = " << static_cast<int>(event.u.setup.bRequest)
+                                  << ", wValue = " << static_cast<int>(event.u.setup.wValue)
+                                  << ", wIndex = " << static_cast<int>(event.u.setup.wIndex)
+                                  << ", wLength = " << static_cast<int>(event.u.setup.wLength);
+
+                        if ((event.u.setup.bRequestType & USB_DIR_IN)) {
+                            LOG(INFO) << "acking device-to-host control transfer";
+                            ssize_t rc = adb_write(control_fd_.get(), "", 0);
+                            if (rc != 0) {
+                                PLOG(ERROR) << "failed to write empty packet to host";
+                                break;
+                            }
+                        } else {
+                            std::string buf;
+                            buf.resize(event.u.setup.wLength + 1);
+
+                            ssize_t rc = adb_read(control_fd_.get(), buf.data(), buf.size());
+                            if (rc != event.u.setup.wLength) {
+                                LOG(ERROR)
+                                        << "read " << rc
+                                        << " bytes when trying to read control request, expected "
+                                        << event.u.setup.wLength;
+                            }
+
+                            LOG(INFO) << "control request contents: " << buf;
+                            break;
+                        }
                     }
                 }
             }
@@ -332,12 +426,6 @@ struct UsbFfsConnection : public Connection {
                 uint64_t dummy;
                 ssize_t rc = adb_read(worker_event_fd_.get(), &dummy, sizeof(dummy));
                 if (rc == -1) {
-                    if (errno == EINTR) {
-                        // We were interrupted either to stop, or because of a backtrace.
-                        // Check stopped_ again to see if we need to exit.
-                        continue;
-                    }
-
                     PLOG(FATAL) << "failed to read from eventfd";
                 } else if (rc == 0) {
                     LOG(FATAL) << "hit EOF on eventfd";
@@ -374,7 +462,6 @@ struct UsbFfsConnection : public Connection {
         }
 
         worker_thread_.join();
-        worker_started_ = false;
     }
 
     void PrepareReadBlock(IoBlock* block, uint64_t id) {
@@ -605,13 +692,10 @@ struct UsbFfsConnection : public Connection {
     std::once_flag error_flag_;
 
     unique_fd worker_event_fd_;
+    unique_fd monitor_event_fd_;
 
     ScopedAioContext aio_context_;
-
-    // We keep a pointer to the control fd, so that we can reuse it to avoid USB reconfiguration,
-    // and still be able to reset it to force a reopen after FUNCTIONFS_UNBIND or running into an
-    // unexpected situation.
-    unique_fd* control_fd_;
+    unique_fd control_fd_;
     unique_fd read_fd_;
     unique_fd write_fd_;
 
@@ -640,16 +724,15 @@ void usb_init_legacy();
 static void usb_ffs_open_thread() {
     adb_thread_setname("usb ffs open");
 
-    unique_fd control;
-    unique_fd bulk_out;
-    unique_fd bulk_in;
-
     while (true) {
         if (gFfsAioSupported.has_value() && !gFfsAioSupported.value()) {
             LOG(INFO) << "failed to use nonblocking ffs, falling back to legacy";
             return usb_init_legacy();
         }
 
+        unique_fd control;
+        unique_fd bulk_out;
+        unique_fd bulk_in;
         if (!open_functionfs(&control, &bulk_out, &bulk_in)) {
             std::this_thread::sleep_for(1s);
             continue;
@@ -660,7 +743,7 @@ static void usb_ffs_open_thread() {
         std::promise<void> destruction_notifier;
         std::future<void> future = destruction_notifier.get_future();
         transport->SetConnection(std::make_unique<UsbFfsConnection>(
-                &control, std::move(bulk_out), std::move(bulk_in),
+                std::move(control), std::move(bulk_out), std::move(bulk_in),
                 std::move(destruction_notifier)));
         register_transport(transport);
         future.wait();
diff --git a/adb/daemon/usb_ffs.cpp b/adb/daemon/usb_ffs.cpp
index ff6eb6a49..954e53015 100644
--- a/adb/daemon/usb_ffs.cpp
+++ b/adb/daemon/usb_ffs.cpp
@@ -208,56 +208,6 @@ static const struct {
 };
 // clang-format on
 
-const char* ffs_event_to_string(enum usb_functionfs_event_type type) {
-    switch (type) {
-        case FUNCTIONFS_BIND:
-            return "FUNCTIONFS_BIND";
-        case FUNCTIONFS_UNBIND:
-            return "FUNCTIONFS_UNBIND";
-        case FUNCTIONFS_ENABLE:
-            return "FUNCTIONFS_ENABLE";
-        case FUNCTIONFS_DISABLE:
-            return "FUNCTIONFS_DISABLE";
-        case FUNCTIONFS_SETUP:
-            return "FUNCTIONFS_SETUP";
-        case FUNCTIONFS_SUSPEND:
-            return "FUNCTIONFS_SUSPEND";
-        case FUNCTIONFS_RESUME:
-            return "FUNCTIONFS_RESUME";
-    }
-}
-
-bool read_functionfs_setup(int fd, usb_functionfs_event* event) {
-    LOG(INFO) << "received FUNCTIONFS_SETUP control transfer: bRequestType = "
-              << static_cast<int>(event->u.setup.bRequestType)
-              << ", bRequest = " << static_cast<int>(event->u.setup.bRequest)
-              << ", wValue = " << static_cast<int>(event->u.setup.wValue)
-              << ", wIndex = " << static_cast<int>(event->u.setup.wIndex)
-              << ", wLength = " << static_cast<int>(event->u.setup.wLength);
-
-    if ((event->u.setup.bRequestType & USB_DIR_IN)) {
-        LOG(INFO) << "acking device-to-host control transfer";
-        ssize_t rc = adb_write(fd, "", 0);
-        if (rc != 0) {
-            PLOG(ERROR) << "failed to write empty packet to host";
-            return false;
-        }
-    } else {
-        std::string buf;
-        buf.resize(event->u.setup.wLength + 1);
-
-        ssize_t rc = adb_read(fd, buf.data(), buf.size());
-        if (rc != event->u.setup.wLength) {
-            LOG(ERROR) << "read " << rc << " bytes when trying to read control request, expected "
-                       << event->u.setup.wLength;
-        }
-
-        LOG(INFO) << "control request contents: " << buf;
-    }
-
-    return true;
-}
-
 bool open_functionfs(android::base::unique_fd* out_control, android::base::unique_fd* out_bulk_out,
                      android::base::unique_fd* out_bulk_in) {
     unique_fd control, bulk_out, bulk_in;
@@ -305,37 +255,11 @@ bool open_functionfs(android::base::unique_fd* out_control, android::base::uniqu
             PLOG(ERROR) << "failed to write USB strings";
             return false;
         }
-
-        // Signal init after we've written our descriptors.
+        // Signal only when writing the descriptors to ffs
         android::base::SetProperty("sys.usb.ffs.ready", "1");
         *out_control = std::move(control);
     }
 
-    // Read until we get FUNCTIONFS_BIND from the control endpoint.
-    while (true) {
-        struct usb_functionfs_event event;
-        ssize_t rc = TEMP_FAILURE_RETRY(adb_read(*out_control, &event, sizeof(event)));
-
-        if (rc == -1) {
-            PLOG(FATAL) << "failed to read from FFS control fd";
-        } else if (rc == 0) {
-            LOG(WARNING) << "hit EOF on functionfs control fd during initialization";
-        } else if (rc != sizeof(event)) {
-            LOG(FATAL) << "read functionfs event of unexpected size, expected " << sizeof(event)
-                       << ", got " << rc;
-        }
-
-        LOG(INFO) << "USB event: "
-                  << ffs_event_to_string(static_cast<usb_functionfs_event_type>(event.type));
-        if (event.type == FUNCTIONFS_BIND) {
-            break;
-        } else if (event.type == FUNCTIONFS_SETUP) {
-            read_functionfs_setup(out_control->get(), &event);
-        } else {
-            continue;
-        }
-    }
-
     bulk_out.reset(adb_open(USB_FFS_ADB_OUT, O_RDONLY));
     if (bulk_out < 0) {
         PLOG(ERROR) << "cannot open bulk-out endpoint " << USB_FFS_ADB_OUT;