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

Read until we receive FUNCTIONFS_BIND after every USB descriptor setup,
instead of only after initial creation of the control fd, so we properly
handle a rebind caused by a change of the USB configuration.

This reverts commit 55b31933f41b1c550b788fa2054010c51f56111b.

Bug: http://b/133132986
Bug: http://b/135155652
Test: manually enabled/disabled MTP with nonblocking_ffs enabled and disabled
Change-Id: Ibb3f46b8f84854ede47a669a85e07e16a5dd6008
diff --git a/adb/daemon/include/adbd/usb.h b/adb/daemon/include/adbd/usb.h
index fca3c58..85a5711 100644
--- a/adb/daemon/include/adbd/usb.h
+++ b/adb/daemon/include/adbd/usb.h
@@ -16,6 +16,8 @@
  * limitations under the License.
  */
 
+#include <linux/usb/functionfs.h>
+
 #include <atomic>
 #include <condition_variable>
 #include <mutex>
@@ -62,5 +64,9 @@
 };
 
 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(android::base::borrowed_fd 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 1abae87..73d5dba 100644
--- a/adb/daemon/usb.cpp
+++ b/adb/daemon/usb.cpp
@@ -66,25 +66,6 @@
 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,
@@ -169,12 +150,12 @@
 };
 
 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_(std::move(control)),
+          control_fd_(control),
           read_fd_(std::move(read)),
           write_fd_(std::move(write)) {
         LOG(INFO) << "UsbFfsConnection constructed";
@@ -183,11 +164,6 @@
             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);
     }
 
@@ -199,7 +175,6 @@
         // 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();
 
@@ -246,13 +221,6 @@
             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:
@@ -271,33 +239,24 @@
         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 = monitor_event_fd_.get(), .events = POLLIN, .revents = 0 },
+                        {.fd = control_fd_->get(), .events = POLLIN, .revents = 0},
                 };
 
-                // 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));
+                int rc = TEMP_FAILURE_RETRY(adb_poll(pfd, 2, -1));
                 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.
-                    break;
+                    // We were told to die, continue reading until FUNCTIONFS_UNBIND.
                 }
 
                 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) {
@@ -309,32 +268,15 @@
                 }
 
                 LOG(INFO) << "USB event: "
-                          << to_string(static_cast<usb_functionfs_event_type>(event.type));
+                          << ffs_event_to_string(
+                                     static_cast<usb_functionfs_event_type>(event.type));
 
                 switch (event.type) {
                     case FUNCTIONFS_BIND:
-                        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;
+                        LOG(FATAL) << "received FUNCTIONFS_BIND after already opened?";
                         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;
@@ -346,10 +288,6 @@
                         break;
 
                     case FUNCTIONFS_DISABLE:
-                        if (!bound) {
-                            LOG(WARNING) << "received FUNCTIONFS_DISABLE while not bound?";
-                        }
-
                         if (!enabled) {
                             LOG(WARNING) << "received FUNCTIONFS_DISABLE while not enabled?";
                         }
@@ -363,44 +301,12 @@
                             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: {
-                        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;
-                        }
+                        read_functionfs_setup(*control_fd_, &event);
+                        break;
                     }
                 }
             }
@@ -426,6 +332,12 @@
                 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";
@@ -462,6 +374,7 @@
         }
 
         worker_thread_.join();
+        worker_started_ = false;
     }
 
     void PrepareReadBlock(IoBlock* block, uint64_t id) {
@@ -679,10 +592,13 @@
     std::once_flag error_flag_;
 
     unique_fd worker_event_fd_;
-    unique_fd monitor_event_fd_;
 
     ScopedAioContext aio_context_;
-    unique_fd control_fd_;
+
+    // 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 read_fd_;
     unique_fd write_fd_;
 
@@ -711,15 +627,16 @@
 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;
@@ -730,7 +647,7 @@
         std::promise<void> destruction_notifier;
         std::future<void> future = destruction_notifier.get_future();
         transport->SetConnection(std::make_unique<UsbFfsConnection>(
-                std::move(control), std::move(bulk_out), std::move(bulk_in),
+                &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 a64ce40..5fbca3b 100644
--- a/adb/daemon/usb_ffs.cpp
+++ b/adb/daemon/usb_ffs.cpp
@@ -248,6 +248,56 @@
 };
 // 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(borrowed_fd 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.get(), "", 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.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;
+    }
+
+    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;
@@ -297,11 +347,37 @@
             PLOG(ERROR) << "failed to write USB strings";
             return false;
         }
-        // Signal only when writing the descriptors to ffs
+
+        // Signal init after we've written our descriptors.
         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, &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;