usb: Read string descriptor length first

This change adds an additional step when reading USB string descriptors.
Instead of passing a buffer length large enough for the largest possible
descriptor the first transfer reads the length and descriptor type from
the device and then submits a second transfer to read exactly the length
provided by the device.

This works around potentially buggy devices and drivers which cannot
handle control transfers with a larger than expected length.

Bug: 1239949
Change-Id: Ia32b31ae49c3b40f180bcef547ae6ca15be73e7d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3102759
Commit-Queue: Reilly Grant <reillyg@chromium.org>
Commit-Queue: Matt Reynolds <mattreynolds@chromium.org>
Auto-Submit: Reilly Grant <reillyg@chromium.org>
Reviewed-by: Matt Reynolds <mattreynolds@chromium.org>
Cr-Commit-Position: refs/heads/main@{#913141}

services/device/usb/usb_descriptors.cc

@@ -199,27 +199,52 @@ void OnReadStringDescriptor(
     size_t length) {
   std::u16string string;
   if (status == UsbTransferStatus::COMPLETED &&
-      ParseUsbStringDescriptor(
-          std::vector<uint8_t>(buffer->front(), buffer->front() + length),
-          &string)) {
+      ParseUsbStringDescriptor(base::make_span(buffer->front(), length),
+                               &string)) {
     std::move(callback).Run(string);
   } else {
     std::move(callback).Run(std::u16string());
   }
 }
 
+void OnReadStringDescriptorHeader(
+    scoped_refptr<UsbDeviceHandle> device_handle,
+    uint8_t index,
+    uint16_t language_id,
+    base::OnceCallback<void(const std::u16string&)> callback,
+    UsbTransferStatus status,
+    scoped_refptr<base::RefCountedBytes> header,
+    size_t length) {
+  if (status == UsbTransferStatus::COMPLETED && length == 2) {
+    const uint8_t* data = header->front();
+    uint8_t actual_length = data[0];
+    auto buffer = base::MakeRefCounted<base::RefCountedBytes>(actual_length);
+    device_handle->ControlTransfer(
+        UsbTransferDirection::INBOUND, UsbControlTransferType::STANDARD,
+        UsbControlTransferRecipient::DEVICE, kGetDescriptorRequest,
+        kStringDescriptorType << 8 | index, language_id, buffer,
+        kControlTransferTimeoutMs,
+        base::BindOnce(&OnReadStringDescriptor, std::move(callback)));
+  } else {
+    LOG(ERROR) << "Failed to read length for string " << static_cast<int>(index)
+               << ".";
+    std::move(callback).Run(std::u16string());
+  }
+}
+
 void ReadStringDescriptor(
     scoped_refptr<UsbDeviceHandle> device_handle,
     uint8_t index,
     uint16_t language_id,
     base::OnceCallback<void(const std::u16string&)> callback) {
-  auto buffer = base::MakeRefCounted<base::RefCountedBytes>(255);
+  auto buffer = base::MakeRefCounted<base::RefCountedBytes>(2);
   device_handle->ControlTransfer(
       UsbTransferDirection::INBOUND, UsbControlTransferType::STANDARD,
       UsbControlTransferRecipient::DEVICE, kGetDescriptorRequest,
       kStringDescriptorType << 8 | index, language_id, buffer,
       kControlTransferTimeoutMs,
-      base::BindOnce(&OnReadStringDescriptor, std::move(callback)));
+      base::BindOnce(&OnReadStringDescriptorHeader, device_handle, index,
+                     language_id, std::move(callback)));
 }
 
 void OnReadLanguageIds(scoped_refptr<UsbDeviceHandle> device_handle,
@@ -361,7 +386,7 @@ void ReadUsbDescriptors(
                      std::move(callback)));
 }
 
-bool ParseUsbStringDescriptor(const std::vector<uint8_t>& descriptor,
+bool ParseUsbStringDescriptor(base::span<const uint8_t> descriptor,
                               std::u16string* output) {
   if (descriptor.size() < 2 || descriptor[1] != kStringDescriptorType)
     return false;

services/device/usb/usb_descriptors.h

@@ -10,9 +10,9 @@
 #include <map>
 #include <memory>
 #include <string>
-#include <vector>
 
 #include "base/callback_forward.h"
+#include "base/containers/span.h"
 #include "base/memory/ref_counted.h"
 #include "services/device/public/mojom/usb_device.mojom.h"
 
@@ -54,7 +54,7 @@ void ReadUsbDescriptors(
     scoped_refptr<UsbDeviceHandle> device_handle,
     base::OnceCallback<void(std::unique_ptr<UsbDeviceDescriptor>)> callback);
 
-bool ParseUsbStringDescriptor(const std::vector<uint8_t>& descriptor,
+bool ParseUsbStringDescriptor(base::span<const uint8_t> descriptor,
                               std::u16string* output);
 
 void ReadUsbStringDescriptors(

services/device/usb/usb_descriptors_unittest.cc

@@ -427,7 +427,9 @@ TEST_F(UsbDescriptorsTest, ReadStringDescriptors) {
                                       UsbControlTransferType::STANDARD,
                                       UsbControlTransferRecipient::DEVICE, 0x06,
                                       0x0300, 0x0000, _, _, _))
+      .WillOnce(InvokeCallback(kStringDescriptor0, size_t{2}))
       .WillOnce(InvokeCallback(kStringDescriptor0, sizeof(kStringDescriptor0)));
+
   static const uint8_t kStringDescriptor1[] = {0x12, 0x03, 'S', 0, 't', 0,
                                                'r',  0,    'i', 0, 'n', 0,
                                                'g',  0,    ' ', 0, '1', 0};
@@ -436,7 +438,9 @@ TEST_F(UsbDescriptorsTest, ReadStringDescriptors) {
                                       UsbControlTransferType::STANDARD,
                                       UsbControlTransferRecipient::DEVICE, 0x06,
                                       0x0301, 0x4321, _, _, _))
+      .WillOnce(InvokeCallback(kStringDescriptor1, size_t{2}))
       .WillOnce(InvokeCallback(kStringDescriptor1, sizeof(kStringDescriptor1)));
+
   static const uint8_t kStringDescriptor2[] = {0x12, 0x03, 'S', 0, 't', 0,
                                                'r',  0,    'i', 0, 'n', 0,
                                                'g',  0,    ' ', 0, '2', 0};
@@ -445,7 +449,9 @@ TEST_F(UsbDescriptorsTest, ReadStringDescriptors) {
                                       UsbControlTransferType::STANDARD,
                                       UsbControlTransferRecipient::DEVICE, 0x06,
                                       0x0302, 0x4321, _, _, _))
+      .WillOnce(InvokeCallback(kStringDescriptor2, size_t{2}))
       .WillOnce(InvokeCallback(kStringDescriptor2, sizeof(kStringDescriptor2)));
+
   static const uint8_t kStringDescriptor3[] = {0x12, 0x03, 'S', 0, 't', 0,
                                                'r',  0,    'i', 0, 'n', 0,
                                                'g',  0,    ' ', 0, '3', 0};
@@ -454,6 +460,7 @@ TEST_F(UsbDescriptorsTest, ReadStringDescriptors) {
                                       UsbControlTransferType::STANDARD,
                                       UsbControlTransferRecipient::DEVICE, 0x06,
                                       0x0303, 0x4321, _, _, _))
+      .WillOnce(InvokeCallback(kStringDescriptor3, size_t{2}))
       .WillOnce(InvokeCallback(kStringDescriptor3, sizeof(kStringDescriptor3)));
 
   ReadUsbStringDescriptors(device_handle, std::move(string_map),