Commit: c745f09b33804242c43f823242f0112645ed3a98 - system-core (git)

system/core

Commit MetaInfo

• Revision: c745f09b33804242c43f823242f0112645ed3a98 (tree)
• Zeit: 2019-06-22 11:49:07
• Autor: Josh Gao <jmgao@goog...>
• Commiter: android-build-merger

Log Message

Merge changes Ibb3f46b8,I875b87a0,I2d005e17 into qt-dev
am: 64e64eb990

Change-Id: I9541e903885ed2fd5fc1b431f881d4aa33f03411

Ändern Zusammenfassung

• modified: adb/daemon/include/adbd/usb.h
• modified: adb/daemon/usb.cpp
• modified: adb/daemon/usb_ffs.cpp

Diff

--- 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 @@ 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);

--- a/adb/daemon/usb.cpp
+++ b/adb/daemon/usb.cpp
@@ -66,25 +66,6 @@ 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,
@@ -169,12 +150,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_(std::move(control)),
+          control_fd_(control),
           read_fd_(std::move(read)),
           write_fd_(std::move(write)) {
         LOG(INFO) << "UsbFfsConnection constructed";
@@ -183,11 +164,6 @@ 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);
     }
 
@@ -199,7 +175,6 @@ 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();
 
@@ -246,13 +221,6 @@ 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:
@@ -271,33 +239,24 @@ 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 = 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 @@ struct UsbFfsConnection : public Connection {
                 }
 
                 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 @@ 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?";
                         }
@@ -363,44 +301,12 @@ 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: {
-                        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 @@ 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";
@@ -462,6 +374,7 @@ struct UsbFfsConnection : public Connection {
         }
 
         worker_thread_.join();
+        worker_started_ = false;
     }
 
     void PrepareReadBlock(IoBlock* block, uint64_t id) {
@@ -509,14 +422,16 @@ struct UsbFfsConnection : public Connection {
             }
 
             if (id.direction == TransferDirection::READ) {
-                HandleRead(id, event.res);
+                if (!HandleRead(id, event.res)) {
+                    return;
+                }
             } else {
                 HandleWrite(id);
             }
         }
     }
 
-    void HandleRead(TransferId id, int64_t size) {
+    bool HandleRead(TransferId id, int64_t size) {
         uint64_t read_idx = id.id % kUsbReadQueueDepth;
         IoBlock* block = &read_requests_[read_idx];
         block->pending = false;
@@ -526,7 +441,7 @@ struct UsbFfsConnection : public Connection {
         if (block->id().id != needed_read_id_) {
             LOG(VERBOSE) << "read " << block->id().id << " completed while waiting for "
                          << needed_read_id_;
-            return;
+            return true;
         }
 
         for (uint64_t id = needed_read_id_;; ++id) {
@@ -535,15 +450,22 @@ struct UsbFfsConnection : public Connection {
             if (current_block->pending) {
                 break;
             }
-            ProcessRead(current_block);
+            if (!ProcessRead(current_block)) {
+                return false;
+            }
             ++needed_read_id_;
         }
+
+        return true;
     }
 
-    void ProcessRead(IoBlock* block) {
+    bool ProcessRead(IoBlock* block) {
         if (!block->payload->empty()) {
             if (!incoming_header_.has_value()) {
-                CHECK_EQ(sizeof(amessage), block->payload->size());
+                if (block->payload->size() != sizeof(amessage)) {
+                    HandleError("received packet of unexpected length while reading header");
+                    return false;
+                }
                 amessage msg;
                 memcpy(&msg, block->payload->data(), sizeof(amessage));
                 LOG(DEBUG) << "USB read:" << dump_header(&msg);
@@ -551,7 +473,10 @@ struct UsbFfsConnection : public Connection {
             } else {
                 size_t bytes_left = incoming_header_->data_length - incoming_payload_.size();
                 Block payload = std::move(*block->payload);
-                CHECK_LE(payload.size(), bytes_left);
+                if (block->payload->size() > bytes_left) {
+                    HandleError("received too many bytes while waiting for payload");
+                    return false;
+                }
                 incoming_payload_.append(std::make_unique<Block>(std::move(payload)));
             }
 
@@ -570,6 +495,7 @@ struct UsbFfsConnection : public Connection {
 
         PrepareReadBlock(block, block->id().id + kUsbReadQueueDepth);
         SubmitRead(block);
+        return true;
     }
 
     bool SubmitRead(IoBlock* block) {
@@ -679,10 +605,13 @@ struct UsbFfsConnection : public Connection {
     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 +640,16 @@ 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;
@@ -730,7 +660,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>(
-                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();

--- a/adb/daemon/usb_ffs.cpp
+++ b/adb/daemon/usb_ffs.cpp
@@ -208,6 +208,56 @@ 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;
@@ -255,8 +305,35 @@ bool open_functionfs(android::base::unique_fd* out_control, android::base::uniqu
             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->get(), &event);
+        } else {
+            continue;
+        }
     }
 
     bulk_out.reset(adb_open(USB_FFS_ADB_OUT, O_RDONLY));
@@ -271,7 +348,6 @@ bool open_functionfs(android::base::unique_fd* out_control, android::base::uniqu
         return false;
     }
 
-    *out_control = std::move(control);
     *out_bulk_in = std::move(bulk_in);
     *out_bulk_out = std::move(bulk_out);
     return true;