Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 7a8d56b2 authored by Khoa Hong's avatar Khoa Hong
Browse files

[RESTRICT AUTOMERGE] Add protections agains use-after-free issues if cancel()...

[RESTRICT AUTOMERGE] 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
parent 600ecb01
Loading
Loading
Loading
Loading
+64 −7
Original line number Diff line number Diff line
@@ -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,6 +64,8 @@ public class UsbDeviceConnection {

    /* package */ boolean open(String name, ParcelFileDescriptor pfd, @NonNull Context context) {
        mContext = context.getApplicationContext();

        synchronized (mLock) {
            boolean wasOpened = native_open(name, pfd.getFileDescriptor());

            if (wasOpened) {
@@ -70,6 +74,14 @@ public class UsbDeviceConnection {

            return wasOpened;
        }
    }

    /***
     * @return If this connection is currently open and usable.
     */
    boolean isOpen() {
        return mNativeContext != 0;
    }

    /**
     * @return The application context the connection was created for.
@@ -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,11 +142,13 @@ public class UsbDeviceConnection {
     * to retrieve a new instance to reestablish communication with the device.
     */
    public void close() {
        if (mNativeContext != 0) {
        synchronized (mLock) {
            if (isOpen()) {
                native_close();
                mCloseGuard.close();
            }
        }
    }

    /**
     * Returns the native file descriptor for the device, or
+79 −7
Original line number Diff line number Diff line
@@ -112,6 +112,7 @@ public class UsbRequest {
     * Releases all resources related to this request.
     */
    public void close() {
        synchronized (mLock) {
            if (mNativeContext != 0) {
                mEndpoint = null;
                mConnection = null;
@@ -119,6 +120,7 @@ public class UsbRequest {
                mCloseGuard.close();
            }
        }
    }

    @Override
    protected void finalize() throws Throwable {
@@ -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();
    }