gamepad: Avoid haptics-related UaF

AbstractHapticGamepad can cause a use-after-free crash when
using an Xbox 360 or Xbox One controller on macOS if an
I/O error occurs when sending a vibration command to the
controller. This CL avoids the crash by treating vibration
errors as recoverable.

Some other errors have also been changed to no longer call
IOError:

* Failure to set the LED state.
* Incoming packets larger than |read_buffer_size_|.
* Failure to acknowledge the Guide button state.
* Failure to send the Xbox One S initialization packet.

These errors are still considered fatal I/O errors:

* ReadPipeAsync failure in QueueRead.
* Failure return code in ReadPipeAsync callback.

Bug: 1166312
Change-Id: I43c101f4c5bbc84a6edab3a8dff3ee5425fbb543
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2674344
Reviewed-by: James Hollyer <jameshollyer@chromium.org>
Commit-Queue: Matt Reynolds <mattreynolds@chromium.org>
Cr-Commit-Position: refs/heads/master@{#859626}

device/gamepad/xbox_controller_mac.h

@@ -107,8 +107,15 @@ class XboxControllerMac final : public AbstractHapticGamepad {
   explicit XboxControllerMac(Delegate* delegate);
   ~XboxControllerMac() override;
 
+  // Open the Xbox controller represented by |service| and perform any necessary
+  // initialization. Returns OPEN_SUCCEEDED if the device was opened
+  // successfully or OPEN_FAILED on failure. Returns
+  // OPEN_FAILED_EXCLUSIVE_ACCESS if the device is already opened by another
+  // process.
   OpenDeviceResult OpenDevice(io_service_t service);
 
+  // Send a command to an Xbox 360 controller to set the player indicator LED
+  // |pattern|.
   void SetLEDPattern(LEDPattern pattern);
 
   // AbstractHapticGamepad implementation.
@@ -126,18 +133,52 @@ class XboxControllerMac final : public AbstractHapticGamepad {
   bool SupportsVibration() const;
 
  private:
+  // Callback to be called when outgoing packets are sent to the device.
+  // |context| is a pointer to the XboxControllerMac and |result| is the error
+  // code for the write operation. |arg0| is unused.
   static void WriteComplete(void* context, IOReturn result, void* arg0);
+
+  // Callback to be called when incoming packets are received from the device.
+  // |context| is a pointer to the XboxControllerMac, |result| is the error
+  // code for the read operation, and |*arg0| contains the number of bytes
+  // received.
+  //
+  // GotData calls IOError if |result| indicates the current read operation
+  // failed, or if scheduling the next read operation fails.
   static void GotData(void* context, IOReturn result, void* arg0);
 
+  // Process the incoming packet in |read_buffer_| as an Xbox 360 packet.
+  // |length| is the size of the packet in bytes.
   void ProcessXbox360Packet(size_t length);
+
+  // Process the incoming packet in |read_buffer_| as an Xbox One packet.
+  // |length| is the size of the packet in bytes.
   void ProcessXboxOnePacket(size_t length);
-  void QueueRead();
 
+  // Queue a read from the device. Returns true if the read was queued, or false
+  // on I/O error.
+  bool QueueRead();
+
+  // Notify the delegate that a fatal I/O error occurred.
   void IOError();
 
+  // Send an Xbox 360 rumble packet to the device, where |strong_magnitude| and
+  // |weak_magnitude| are values in the range [0,255] that represent the
+  // vibration intensity for the strong and weak rumble motors.
   void WriteXbox360Rumble(uint8_t strong_magnitude, uint8_t weak_magnitude);
-  void WriteXboxOneInit();
+
+  // Send an Xbox One S initialization packet to the device. Returns true if the
+  // packet was sent successfully, or false on I/O error.
+  bool WriteXboxOneInit();
+
+  // Send an Xbox One rumble packet to the device, where |strong_magnitude| and
+  // |weak_magnitude| are values in the range [0,255] that represent the
+  // vibration intensity for the strong and weak rumble motors.
   void WriteXboxOneRumble(uint8_t strong_magnitude, uint8_t weak_magnitude);
+
+  // Send an Xbox One packet to the device acknowledging that the Xbox button
+  // was pressed or released. |sequence_number| must match the value in the
+  // incoming report containing the new button state.
   void WriteXboxOneAckGuide(uint8_t sequence_number);
 
   // Handle for the USB device. IOUSBDeviceStruct320 is the latest version of

device/gamepad/xbox_controller_mac.mm

@@ -304,6 +304,20 @@ void NormalizeXboxOneButtonData(const XboxOneButtonData& data,
   return XboxControllerMac::UNKNOWN_CONTROLLER;
 }
 
+bool ControllerNeedsXboxOneInit(XboxControllerMac::ControllerType type) {
+  switch (type) {
+    case XboxControllerMac::XBOX_ONE_CONTROLLER_2013:
+    case XboxControllerMac::XBOX_ONE_CONTROLLER_2015:
+    case XboxControllerMac::XBOX_ONE_ELITE_CONTROLLER:
+    case XboxControllerMac::XBOX_ONE_ELITE_CONTROLLER_2:
+    case XboxControllerMac::XBOX_ONE_S_CONTROLLER:
+    case XboxControllerMac::XBOX_ADAPTIVE_CONTROLLER:
+      return true;
+    default:
+      return false;
+  }
+}
+
 }  // namespace
 
 XboxControllerMac::XboxControllerMac(Delegate* delegate)
@@ -532,17 +546,14 @@ void NormalizeXboxOneButtonData(const XboxOneButtonData& data,
         return OPEN_FAILED;
       read_buffer_.reset(new uint8_t[max_packet_size]);
       read_buffer_size_ = max_packet_size;
-      QueueRead();
+      if (!QueueRead())
+        return OPEN_FAILED;
     } else if (i == control_endpoint_) {
       if (direction != kUSBOut)
         return OPEN_FAILED;
-      if (controller_type_ == XBOX_ONE_CONTROLLER_2013 ||
-          controller_type_ == XBOX_ONE_CONTROLLER_2015 ||
-          controller_type_ == XBOX_ONE_ELITE_CONTROLLER ||
-          controller_type_ == XBOX_ONE_ELITE_CONTROLLER_2 ||
-          controller_type_ == XBOX_ONE_S_CONTROLLER ||
-          controller_type_ == XBOX_ADAPTIVE_CONTROLLER)
-        WriteXboxOneInit();
+
+      if (ControllerNeedsXboxOneInit(controller_type_) && !WriteXboxOneInit())
+        return OPEN_FAILED;
     }
   }
 
@@ -571,9 +582,8 @@ void NormalizeXboxOneButtonData(const XboxOneButtonData& data,
           ->WritePipeAsync(interface_, control_endpoint_, buffer,
                            (UInt32)length, WriteComplete, buffer);
   if (kr != KERN_SUCCESS) {
+    DLOG(ERROR) << "Write error: Failed to send Xbox 360 LED command.";
     delete[] buffer;
-    IOError();
-    return;
   }
 }
 
@@ -666,6 +676,7 @@ void NormalizeXboxOneButtonData(const XboxOneButtonData& data,
   if (result != kIOReturnSuccess) {
     // This will happen if the device was disconnected. The gamepad has
     // probably been destroyed by a meteorite.
+    DLOG(ERROR) << "Read error: Failed to read from the device.";
     controller->IOError();
     return;
   }
@@ -676,17 +687,18 @@ void NormalizeXboxOneButtonData(const XboxOneButtonData& data,
     controller->ProcessXboxOnePacket(bytes_read);
 
   // Queue up another read.
-  controller->QueueRead();
+  if (!controller->QueueRead())
+    controller->IOError();
 }
 
 void XboxControllerMac::ProcessXbox360Packet(size_t length) {
   if (length < kXbox360HeaderBytes)
     return;
-  DCHECK(length <= read_buffer_size_);
-  if (length > read_buffer_size_) {
-    IOError();
+
+  DCHECK_LE(length, read_buffer_size_);
+  if (length > read_buffer_size_)
     return;
-  }
+
   uint8_t* buffer = read_buffer_.get();
 
   if (buffer[1] != length)
@@ -724,11 +736,11 @@ void NormalizeXboxOneButtonData(const XboxOneButtonData& data,
 void XboxControllerMac::ProcessXboxOnePacket(size_t length) {
   if (length < kXboxOneHeaderBytes)
     return;
-  DCHECK(length <= read_buffer_size_);
-  if (length > read_buffer_size_) {
-    IOError();
+
+  DCHECK_LE(length, read_buffer_size_);
+  if (length > read_buffer_size_)
     return;
-  }
+
   uint8_t* buffer = read_buffer_.get();
   uint8_t type = buffer[0];
   bool needs_ack = (buffer[1] == 0x30);
@@ -767,13 +779,14 @@ void NormalizeXboxOneButtonData(const XboxOneButtonData& data,
   }
 }
 
-void XboxControllerMac::QueueRead() {
+bool XboxControllerMac::QueueRead() {
   kern_return_t kr =
       (*interface_)
           ->ReadPipeAsync(interface_, read_endpoint_, read_buffer_.get(),
                           read_buffer_size_, GotData, this);
   if (kr != KERN_SUCCESS)
-    IOError();
+    DLOG(ERROR) << "Read error: Failed to queue next read.";
+  return kr == KERN_SUCCESS;
 }
 
 void XboxControllerMac::IOError() {
@@ -804,13 +817,12 @@ void NormalizeXboxOneButtonData(const XboxOneButtonData& data,
           ->WritePipeAsync(interface_, control_endpoint_, buffer,
                            (UInt32)length, WriteComplete, buffer);
   if (kr != KERN_SUCCESS) {
+    DLOG(ERROR) << "Write error: Failed to send Xbox 360 rumble command.";
     delete[] buffer;
-    IOError();
-    return;
   }
 }
 
-void XboxControllerMac::WriteXboxOneInit() {
+bool XboxControllerMac::WriteXboxOneInit() {
   const UInt8 length = 5;
 
   // This buffer will be released in WriteComplete when WritePipeAsync
@@ -826,10 +838,12 @@ void NormalizeXboxOneButtonData(const XboxOneButtonData& data,
           ->WritePipeAsync(interface_, control_endpoint_, buffer,
                            (UInt32)length, WriteComplete, buffer);
   if (kr != KERN_SUCCESS) {
+    DLOG(ERROR)
+        << "Write error: Failed to send Xbox One initialization packet.";
     delete[] buffer;
-    IOError();
-    return;
+    return false;
   }
+  return true;
 }
 
 void XboxControllerMac::WriteXboxOneRumble(uint8_t strong_magnitude,
@@ -862,9 +876,8 @@ void NormalizeXboxOneButtonData(const XboxOneButtonData& data,
           ->WritePipeAsync(interface_, control_endpoint_, buffer,
                            (UInt32)length, WriteComplete, buffer);
   if (kr != KERN_SUCCESS) {
+    DLOG(ERROR) << "Write error: Failed to send Xbox One rumble command.";
     delete[] buffer;
-    IOError();
-    return;
   }
 }
 
@@ -894,9 +907,8 @@ void NormalizeXboxOneButtonData(const XboxOneButtonData& data,
           ->WritePipeAsync(interface_, control_endpoint_, buffer,
                            (UInt32)length, WriteComplete, buffer);
   if (kr != KERN_SUCCESS) {
+    DLOG(ERROR) << "Write error: Failed to send Xbox One mode report reply.";
     delete[] buffer;
-    IOError();
-    return;
   }
 }