DivestOS/Patches/LineageOS-16.0/android_frameworks_base/347051.patch
Tavi 082bc48c32
16.0: Import and verify picks
https://review.lineageos.org/q/topic:P_asb_2022-05
https://review.lineageos.org/q/topic:P_asb_2022-06
https://review.lineageos.org/q/topic:P_asb_2022-07
https://review.lineageos.org/q/topic:P_asb_2022-08
https://review.lineageos.org/q/topic:P_asb_2022-09
https://review.lineageos.org/q/topic:P_asb_2022-10
https://review.lineageos.org/q/topic:P_asb_2022-11
https://review.lineageos.org/q/topic:P_asb_2022-12
https://review.lineageos.org/q/topic:P_asb_2023-01
https://review.lineageos.org/q/topic:P_asb_2023-02
https://review.lineageos.org/q/topic:P_asb_2023-03
https://review.lineageos.org/q/topic:P_asb_2023-04
https://review.lineageos.org/q/topic:P_asb_2023-05
https://review.lineageos.org/q/topic:P_asb_2023-06
https://review.lineageos.org/q/topic:P_asb_2023-07
	accounted for via manifest change:
	https://review.lineageos.org/c/LineageOS/android_external_freetype/+/361250
https://review.lineageos.org/q/topic:P_asb_2023-08
	accounted for via manifest change:
	https://review.lineageos.org/c/LineageOS/android_external_freetype/+/364606
	accounted for via patches:
	https://review.lineageos.org/c/LineageOS/android_system_ca-certificates/+/365328
https://review.lineageos.org/q/topic:P_asb_2023-09
https://review.lineageos.org/q/topic:P_asb_2023-10
https://review.lineageos.org/q/topic:P_asb_2023-11
	accounted for via patches:
	https://review.lineageos.org/c/LineageOS/android_system_ca-certificates/+/374916
https://review.lineageos.org/q/topic:P_asb_2023-12
https://review.lineageos.org/q/topic:P_asb_2024-01
https://review.lineageos.org/q/topic:P_asb_2024-02
https://review.lineageos.org/q/topic:P_asb_2024-03
https://review.lineageos.org/q/topic:P_asb_2024-04

Signed-off-by: Tavi <tavi@divested.dev>
2024-05-07 19:43:19 -04:00

255 lines
9.5 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 | 86 +++++++++++++++++--
2 files changed, 143 insertions(+), 14 deletions(-)
diff --git a/core/java/android/hardware/usb/UsbDeviceConnection.java b/core/java/android/hardware/usb/UsbDeviceConnection.java
index 9e5174ad93a8..7a521166f35c 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 f59c87eecfcb..441d718b6067 100644
--- a/core/java/android/hardware/usb/UsbRequest.java
+++ b/core/java/android/hardware/usb/UsbRequest.java
@@ -108,11 +108,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();
+ }
}
}
@@ -186,10 +188,32 @@ 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;
- if (mConnection.getContext().getApplicationInfo().targetSdkVersion < Build.VERSION_CODES.P
+ if (connection.getContext().getApplicationInfo().targetSdkVersion < Build.VERSION_CODES.P
&& length > MAX_USBFS_BUFFER_SIZE) {
length = MAX_USBFS_BUFFER_SIZE;
}
@@ -238,6 +262,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");
@@ -255,7 +301,7 @@ public class UsbRequest {
mIsUsingNewQueue = true;
wasQueued = native_queue(null, 0, 0);
} else {
- if (mConnection.getContext().getApplicationInfo().targetSdkVersion
+ if (connection.getContext().getApplicationInfo().targetSdkVersion
< Build.VERSION_CODES.P) {
// Can only send/receive MAX_USBFS_BUFFER_SIZE bytes at once
Preconditions.checkArgumentInRange(buffer.remaining(), 0, MAX_USBFS_BUFFER_SIZE,
@@ -358,6 +404,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();
}