Android/frameworks_base
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

[RESTRICT AUTOMERGE] Add protections agains use-after-free issues if cancel() or queue() is called after a device connection has been closed.

Browse Source 
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
This commit is contained in:
Khoa Hong
2022-10-19 16:29:18 +08:00
parent 600ecb01d0
commit 7a8d56b2fe
2 changed files with 143 additions and 14 deletions
Download Patch File Download Diff File Expand all files Collapse all files

71
core/java/android/hardware/usb/UsbDeviceConnection.java
View File

@@ -52,6 +52,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
@@ -62,13 +64,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;
}
/**
@@ -80,6 +92,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.
@@ -87,9 +142,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();
}
}
}

86
core/java/android/hardware/usb/UsbRequest.java
View File

@@ -112,11 +112,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();
}
}
}
@@ -190,10 +192,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;
}
@@ -242,6 +266,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");
@@ -259,7 +305,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,
@@ -362,6 +408,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();
}