Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit ef3d3432 authored by Josh Gao's avatar Josh Gao
Browse files

adb: use the actual wMaxPacketSize for usb endpoints. 

Previously, adb was assuming a fixed maximum packet size of 1024 bytes
(the value for an endpoint connected via USB 3.0). When connected to an
endpoint that has an actual maximum packet size of 512 bytes (i.e.
every single device over USB 2.0), the following could occur:

    device sends amessage with 512 byte payload
    client reads amessage
    client tries to read payload with a length of 1024

In this scenario, the kernel will block, waiting for an additional
packet which won't arrive until something else gets sent across the
wire, which will result in the previous read failing, and the new
packet being dropped.

Bug: http://b/37783561
Test: python test_device.py on linux/darwin, with native/libusb
Change-Id: I556f5344945e22dd1533b076f662a97eea24628e
parent 483d2f9a

adb/client/usb_dispatch.cpp

+6 −0
Original line number Diff line number Diff line
@@ -48,3 +48,9 @@ void usb_kick(usb_handle* h) { 
    should_use_libusb() ? libusb::usb_kick(reinterpret_cast<libusb::usb_handle*>(h))

                        : native::usb_kick(reinterpret_cast<native::usb_handle*>(h));

}



size_t usb_get_max_packet_size(usb_handle* h) {

    return should_use_libusb()

               ? libusb::usb_get_max_packet_size(reinterpret_cast<libusb::usb_handle*>(h))

               : native::usb_get_max_packet_size(reinterpret_cast<native::usb_handle*>(h));

}

adb/client/usb_libusb.cpp

+24 −6
Original line number Diff line number Diff line
@@ -91,7 +91,7 @@ namespace libusb { 
struct usb_handle : public ::usb_handle {

    usb_handle(const std::string& device_address, const std::string& serial,

               unique_device_handle&& device_handle, uint8_t interface, uint8_t bulk_in,

               uint8_t bulk_out, size_t zero_mask)

               uint8_t bulk_out, size_t zero_mask, size_t max_packet_size)

        : device_address(device_address),

          serial(serial),

          closing(false),
@@ -100,7 +100,8 @@ struct usb_handle : public ::usb_handle { 
          write("write", zero_mask, true),

          interface(interface),

          bulk_in(bulk_in),

          bulk_out(bulk_out) {}

          bulk_out(bulk_out),

          max_packet_size(max_packet_size) {}



    ~usb_handle() {

        Close();
@@ -143,6 +144,8 @@ struct usb_handle : public ::usb_handle { 
    uint8_t interface;

    uint8_t bulk_in;

    uint8_t bulk_out;



    size_t max_packet_size;

};



static auto& usb_handles = *new std::unordered_map<std::string, std::unique_ptr<usb_handle>>();
@@ -206,6 +209,7 @@ static void poll_for_devices() { 
            size_t interface_num;

            uint16_t zero_mask;

            uint8_t bulk_in = 0, bulk_out = 0;

            size_t packet_size = 0;

            bool found_adb = false;



            for (interface_num = 0; interface_num < config->bNumInterfaces; ++interface_num) {
@@ -252,6 +256,14 @@ static void poll_for_devices() { 
                        found_in = true;

                        bulk_in = endpoint_addr;

                    }



                    size_t endpoint_packet_size = endpoint_desc.wMaxPacketSize;

                    CHECK(endpoint_packet_size != 0);

                    if (packet_size == 0) {

                        packet_size = endpoint_packet_size;

                    } else {

                        CHECK(packet_size == endpoint_packet_size);

                    }

                }



                if (found_in && found_out) {
@@ -280,7 +292,7 @@ static void poll_for_devices() { 
            }



            libusb_device_handle* handle_raw;

            rc = libusb_open(list[i], &handle_raw);

            rc = libusb_open(device, &handle_raw);

            if (rc != 0) {

                LOG(WARNING) << "failed to open usb device at " << device_address << ": "

                             << libusb_error_name(rc);
@@ -324,9 +336,9 @@ static void poll_for_devices() { 
                }

            }



            auto result =

                std::make_unique<usb_handle>(device_address, device_serial, std::move(handle),

                                             interface_num, bulk_in, bulk_out, zero_mask);

            auto result = std::make_unique<usb_handle>(device_address, device_serial,

                                                       std::move(handle), interface_num, bulk_in,

                                                       bulk_out, zero_mask, packet_size);

            usb_handle* usb_handle_raw = result.get();



            {
@@ -507,4 +519,10 @@ int usb_close(usb_handle* h) { 
void usb_kick(usb_handle* h) {

    h->Close();

}



size_t usb_get_max_packet_size(usb_handle* h) {

    CHECK(h->max_packet_size != 0);

    return h->max_packet_size;

}



} // namespace libusb

adb/client/usb_linux.cpp

+18 −11
Original line number Diff line number Diff line
@@ -65,6 +65,7 @@ struct usb_handle : public ::usb_handle { 
    unsigned char ep_in;

    unsigned char ep_out;



    size_t max_packet_size;

    unsigned zero_mask;

    unsigned writeable = 1;
@@ -120,9 +121,9 @@ static inline bool contains_non_digit(const char* name) { 
}



static void find_usb_device(const std::string& base,

        void (*register_device_callback)

                (const char*, const char*, unsigned char, unsigned char, int, int, unsigned))

{

                            void (*register_device_callback)(const char*, const char*,

                                                             unsigned char, unsigned char, int, int,

                                                             unsigned, size_t)) {

    std::unique_ptr<DIR, int(*)(DIR*)> bus_dir(opendir(base.c_str()), closedir);

    if (!bus_dir) return;
@@ -144,6 +145,7 @@ static void find_usb_device(const std::string& base, 
            struct usb_interface_descriptor* interface;

            struct usb_endpoint_descriptor *ep1, *ep2;

            unsigned zero_mask = 0;

            size_t max_packet_size = 0;

            unsigned vid, pid;



            if (contains_non_digit(de->d_name)) continue;
@@ -252,6 +254,7 @@ static void find_usb_device(const std::string& base, 
                        }

                            /* aproto 01 needs 0 termination */

                        if (interface->bInterfaceProtocol == 0x01) {

                            max_packet_size = ep1->wMaxPacketSize;

                            zero_mask = ep1->wMaxPacketSize - 1;

                        }
@@ -281,9 +284,9 @@ static void find_usb_device(const std::string& base, 
                            }

                        }



                        register_device_callback(dev_name.c_str(), devpath,

                                local_ep_in, local_ep_out,

                                interface->bInterfaceNumber, device->iSerialNumber, zero_mask);

                        register_device_callback(dev_name.c_str(), devpath, local_ep_in,

                                                 local_ep_out, interface->bInterfaceNumber,

                                                 device->iSerialNumber, zero_mask, max_packet_size);

                        break;

                    }

                } else {
@@ -497,10 +500,13 @@ int usb_close(usb_handle* h) { 
    return 0;

}



static void register_device(const char* dev_name, const char* dev_path,

                            unsigned char ep_in, unsigned char ep_out,

                            int interface, int serial_index,

                            unsigned zero_mask) {

size_t usb_get_max_packet_size(usb_handle* h) {

    return h->max_packet_size;

}



static void register_device(const char* dev_name, const char* dev_path, unsigned char ep_in,

                            unsigned char ep_out, int interface, int serial_index,

                            unsigned zero_mask, size_t max_packet_size) {

    // Since Linux will not reassign the device ID (and dev_name) as long as the

    // device is open, we can add to the list here once we open it and remove

    // from the list when we're finally closed and everything will work out
@@ -523,6 +529,7 @@ static void register_device(const char* dev_name, const char* dev_path, 
    usb->ep_in = ep_in;

    usb->ep_out = ep_out;

    usb->zero_mask = zero_mask;

    usb->max_packet_size = max_packet_size;



    // Initialize mark so we don't get garbage collected after the device scan.

    usb->mark = true;

adb/client/usb_osx.cpp

+15 −3
Original line number Diff line number Diff line
@@ -51,15 +51,21 @@ struct usb_handle 
    UInt8 bulkOut;

    IOUSBInterfaceInterface190** interface;

    unsigned int zero_mask;

    size_t max_packet_size;



    // For garbage collecting disconnected devices.

    bool mark;

    std::string devpath;

    std::atomic<bool> dead;



    usb_handle() : bulkIn(0), bulkOut(0), interface(nullptr),

        zero_mask(0), mark(false), dead(false) {

    }

    usb_handle()

        : bulkIn(0),

          bulkOut(0),

          interface(nullptr),

          zero_mask(0),

          max_packet_size(0),

          mark(false),

          dead(false) {}

};



static std::atomic<bool> usb_inited_flag;
@@ -390,6 +396,7 @@ CheckInterface(IOUSBInterfaceInterface190 **interface, UInt16 vendor, UInt16 pro 
        }



        handle->zero_mask = maxPacketSize - 1;

        handle->max_packet_size = maxPacketSize;

    }



    handle->interface = interface;
@@ -558,4 +565,9 @@ void usb_kick(usb_handle *handle) { 
    std::lock_guard<std::mutex> lock_guard(g_usb_handles_mutex);

    usb_kick_locked(handle);

}



size_t usb_get_max_packet_size(usb_handle* handle) {

    return handle->max_packet_size;

}



} // namespace native

adb/client/usb_windows.cpp

+8 −0
Original line number Diff line number Diff line
@@ -65,6 +65,9 @@ struct usb_handle { 
  /// Interface name

  wchar_t*      interface_name;



  /// Maximum packet size.

  unsigned max_packet_size;



  /// Mask for determining when to use zero length packets

  unsigned zero_mask;

};
@@ -522,6 +525,10 @@ int usb_close(usb_handle* handle) { 
  return 0;

}



size_t usb_get_max_packet_size(usb_handle* handle) {

    return handle->max_packet_size;

}



int recognized_device(usb_handle* handle) {

  if (NULL == handle)

    return 0;
@@ -557,6 +564,7 @@ int recognized_device(usb_handle* handle) { 
      AdbEndpointInformation endpoint_info;

      // assuming zero is a valid bulk endpoint ID

      if (AdbGetEndpointInformation(handle->adb_interface, 0, &endpoint_info)) {

        handle->max_packet_size = endpoint_info.max_packet_size;

        handle->zero_mask = endpoint_info.max_packet_size - 1;

        D("device zero_mask: 0x%x", handle->zero_mask);

      } else {