mirror of
https://github.com/Divested-Mobile/DivestOS-Build.git
synced 2024-10-01 01:35:54 -04:00
241 lines
8.6 KiB
Diff
241 lines
8.6 KiB
Diff
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
|
|
From: Khoa Hong <khoahong@google.com>
|
|
Date: Wed, 19 Oct 2022 16:29:18 +0800
|
|
Subject: [PATCH] Add protections agains use-after-free issues if cancel() or
|
|
queue() is called after a device connection has been closed.
|
|
|
|
This is a backport of ag/7528082 and ag/20033068.
|
|
|
|
Bug: 132319116
|
|
Bug: 130571162
|
|
Bug: 204584366
|
|
Test: CTS Verifier: USB Accessory Test & USB Device Test
|
|
Change-Id: I952ab566e26a808997e362dc85ebd1d8eb4574b9
|
|
(cherry picked from commit 7a8d56b2fe3496f7717ad1afe45d2ef523b7e252)
|
|
Merged-In: I952ab566e26a808997e362dc85ebd1d8eb4574b9
|
|
---
|
|
.../hardware/usb/UsbDeviceConnection.java | 71 ++++++++++++++--
|
|
.../java/android/hardware/usb/UsbRequest.java | 82 +++++++++++++++++--
|
|
2 files changed, 141 insertions(+), 12 deletions(-)
|
|
|
|
diff --git a/core/java/android/hardware/usb/UsbDeviceConnection.java b/core/java/android/hardware/usb/UsbDeviceConnection.java
|
|
index 5b15c0d2fd9c..43a1b39d6be4 100644
|
|
--- a/core/java/android/hardware/usb/UsbDeviceConnection.java
|
|
+++ b/core/java/android/hardware/usb/UsbDeviceConnection.java
|
|
@@ -50,6 +50,8 @@ public class UsbDeviceConnection {
|
|
|
|
private final CloseGuard mCloseGuard = CloseGuard.get();
|
|
|
|
+ private final Object mLock = new Object();
|
|
+
|
|
/**
|
|
* UsbDevice should only be instantiated by UsbService implementation
|
|
* @hide
|
|
@@ -60,13 +62,23 @@ public class UsbDeviceConnection {
|
|
|
|
/* package */ boolean open(String name, ParcelFileDescriptor pfd, @NonNull Context context) {
|
|
mContext = context.getApplicationContext();
|
|
- boolean wasOpened = native_open(name, pfd.getFileDescriptor());
|
|
|
|
- if (wasOpened) {
|
|
- mCloseGuard.open("close");
|
|
+ synchronized (mLock) {
|
|
+ boolean wasOpened = native_open(name, pfd.getFileDescriptor());
|
|
+
|
|
+ if (wasOpened) {
|
|
+ mCloseGuard.open("close");
|
|
+ }
|
|
+
|
|
+ return wasOpened;
|
|
}
|
|
+ }
|
|
|
|
- return wasOpened;
|
|
+ /***
|
|
+ * @return If this connection is currently open and usable.
|
|
+ */
|
|
+ boolean isOpen() {
|
|
+ return mNativeContext != 0;
|
|
}
|
|
|
|
/**
|
|
@@ -78,6 +90,49 @@ public class UsbDeviceConnection {
|
|
return mContext;
|
|
}
|
|
|
|
+ /**
|
|
+ * Cancel a request which relates to this connection.
|
|
+ *
|
|
+ * @return true if the request was successfully cancelled.
|
|
+ */
|
|
+ /* package */ boolean cancelRequest(UsbRequest request) {
|
|
+ synchronized (mLock) {
|
|
+ if (!isOpen()) {
|
|
+ return false;
|
|
+ }
|
|
+
|
|
+ return request.cancelIfOpen();
|
|
+ }
|
|
+ }
|
|
+
|
|
+ /**
|
|
+ * This is meant to be called by UsbRequest's queue() in order to synchronize on
|
|
+ * UsbDeviceConnection's mLock to prevent the connection being closed while queueing.
|
|
+ */
|
|
+ /* package */ boolean queueRequest(UsbRequest request, ByteBuffer buffer, int length) {
|
|
+ synchronized (mLock) {
|
|
+ if (!isOpen()) {
|
|
+ return false;
|
|
+ }
|
|
+
|
|
+ return request.queueIfConnectionOpen(buffer, length);
|
|
+ }
|
|
+ }
|
|
+
|
|
+ /**
|
|
+ * This is meant to be called by UsbRequest's queue() in order to synchronize on
|
|
+ * UsbDeviceConnection's mLock to prevent the connection being closed while queueing.
|
|
+ */
|
|
+ /* package */ boolean queueRequest(UsbRequest request, @Nullable ByteBuffer buffer) {
|
|
+ synchronized (mLock) {
|
|
+ if (!isOpen()) {
|
|
+ return false;
|
|
+ }
|
|
+
|
|
+ return request.queueIfConnectionOpen(buffer);
|
|
+ }
|
|
+ }
|
|
+
|
|
/**
|
|
* Releases all system resources related to the device.
|
|
* Once the object is closed it cannot be used again.
|
|
@@ -85,9 +140,11 @@ public class UsbDeviceConnection {
|
|
* to retrieve a new instance to reestablish communication with the device.
|
|
*/
|
|
public void close() {
|
|
- if (mNativeContext != 0) {
|
|
- native_close();
|
|
- mCloseGuard.close();
|
|
+ synchronized (mLock) {
|
|
+ if (isOpen()) {
|
|
+ native_close();
|
|
+ mCloseGuard.close();
|
|
+ }
|
|
}
|
|
}
|
|
|
|
diff --git a/core/java/android/hardware/usb/UsbRequest.java b/core/java/android/hardware/usb/UsbRequest.java
|
|
index 2e8f8e12b508..2c77647a26be 100644
|
|
--- a/core/java/android/hardware/usb/UsbRequest.java
|
|
+++ b/core/java/android/hardware/usb/UsbRequest.java
|
|
@@ -107,11 +107,13 @@ public class UsbRequest {
|
|
* Releases all resources related to this request.
|
|
*/
|
|
public void close() {
|
|
- if (mNativeContext != 0) {
|
|
- mEndpoint = null;
|
|
- mConnection = null;
|
|
- native_close();
|
|
- mCloseGuard.close();
|
|
+ synchronized (mLock) {
|
|
+ if (mNativeContext != 0) {
|
|
+ mEndpoint = null;
|
|
+ mConnection = null;
|
|
+ native_close();
|
|
+ mCloseGuard.close();
|
|
+ }
|
|
}
|
|
}
|
|
|
|
@@ -183,6 +185,28 @@ public class UsbRequest {
|
|
*/
|
|
@Deprecated
|
|
public boolean queue(ByteBuffer buffer, int length) {
|
|
+ UsbDeviceConnection connection = mConnection;
|
|
+ if (connection == null) {
|
|
+ // The expected exception by CTS Verifier - USB Device test
|
|
+ throw new NullPointerException("invalid connection");
|
|
+ }
|
|
+
|
|
+ // Calling into the underlying UsbDeviceConnection to synchronize on its lock, to prevent
|
|
+ // the connection being closed while queueing.
|
|
+ return connection.queueRequest(this, buffer, length);
|
|
+ }
|
|
+
|
|
+ /**
|
|
+ * This is meant to be called from UsbDeviceConnection after synchronizing using the lock over
|
|
+ * there, to prevent the connection being closed while queueing.
|
|
+ */
|
|
+ /* package */ boolean queueIfConnectionOpen(ByteBuffer buffer, int length) {
|
|
+ UsbDeviceConnection connection = mConnection;
|
|
+ if (connection == null || !connection.isOpen()) {
|
|
+ // The expected exception by CTS Verifier - USB Device test
|
|
+ throw new NullPointerException("invalid connection");
|
|
+ }
|
|
+
|
|
boolean out = (mEndpoint.getDirection() == UsbConstants.USB_DIR_OUT);
|
|
boolean result;
|
|
|
|
@@ -227,6 +251,28 @@ public class UsbRequest {
|
|
* @return true if the queueing operation succeeded
|
|
*/
|
|
public boolean queue(@Nullable ByteBuffer buffer) {
|
|
+ UsbDeviceConnection connection = mConnection;
|
|
+ if (connection == null) {
|
|
+ // The expected exception by CTS Verifier - USB Device test
|
|
+ throw new IllegalStateException("invalid connection");
|
|
+ }
|
|
+
|
|
+ // Calling into the underlying UsbDeviceConnection to synchronize on its lock, to prevent
|
|
+ // the connection being closed while queueing.
|
|
+ return connection.queueRequest(this, buffer);
|
|
+ }
|
|
+
|
|
+ /**
|
|
+ * This is meant to be called from UsbDeviceConnection after synchronizing using the lock over
|
|
+ * there, to prevent the connection being closed while queueing.
|
|
+ */
|
|
+ /* package */ boolean queueIfConnectionOpen(@Nullable ByteBuffer buffer) {
|
|
+ UsbDeviceConnection connection = mConnection;
|
|
+ if (connection == null || !connection.isOpen()) {
|
|
+ // The expected exception by CTS Verifier - USB Device test
|
|
+ throw new IllegalStateException("invalid connection");
|
|
+ }
|
|
+
|
|
// Request need to be initialized
|
|
Preconditions.checkState(mNativeContext != 0, "request is not initialized");
|
|
|
|
@@ -344,6 +390,32 @@ public class UsbRequest {
|
|
* @return true if cancelling succeeded
|
|
*/
|
|
public boolean cancel() {
|
|
+ UsbDeviceConnection connection = mConnection;
|
|
+ if (connection == null) {
|
|
+ return false;
|
|
+ }
|
|
+
|
|
+ return connection.cancelRequest(this);
|
|
+ }
|
|
+
|
|
+ /**
|
|
+ * Cancels a pending queue operation (for use when the UsbDeviceConnection associated
|
|
+ * with this request is synchronized). This ensures we don't have a race where the
|
|
+ * device is closed and then the request is canceled which would lead to a
|
|
+ * use-after-free because the cancel operation uses the device connection
|
|
+ * information freed in the when UsbDeviceConnection is closed.<br/>
|
|
+ *
|
|
+ * This method assumes the connected is not closed while this method is executed.
|
|
+ *
|
|
+ * @return true if cancelling succeeded.
|
|
+ */
|
|
+ /* package */ boolean cancelIfOpen() {
|
|
+ UsbDeviceConnection connection = mConnection;
|
|
+ if (mNativeContext == 0 || (connection != null && !connection.isOpen())) {
|
|
+ Log.w(TAG,
|
|
+ "Detected attempt to cancel a request on a connection which isn't open");
|
|
+ return false;
|
|
+ }
|
|
return native_cancel();
|
|
}
|
|
|