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

Commit 56b6db32 authored by Piotr Wilczyński's avatar Piotr Wilczyński
Browse files

Fix no. of virtual displays per package 

A virtual display might be released by the system process and not the process that created the display. We shouldn't be using Binder.getCallingUid() to find the ownerUid that created the display.

Bug: 261791612
Flag: com.android.server.display.feature.flags.virtual_display_limit
Test: atest DisplayManagerServiceTest
Test: atest VirtualDisplayAdapterTest
Change-Id: I1651b4a668bc77fd954fba99f7b6a50315511bfa
parent 6266b5cb

services/core/java/com/android/server/display/DisplayManagerService.java

+5 −6
Original line number Diff line number Diff line
@@ -1997,7 +1997,7 @@ public final class DisplayManagerService extends SystemService { 
                        // handles stopping the projection.

                        Slog.w(TAG, "Content Recording: failed to start mirroring - "

                                + "releasing virtual display " + displayId);

                        releaseVirtualDisplayInternal(callback.asBinder(), callingUid);

                        releaseVirtualDisplayInternal(callback.asBinder());

                        return Display.INVALID_DISPLAY;

                    } else if (projection != null) {

                        // Indicate that this projection has been used to record, and can't be used
@@ -2086,7 +2086,7 @@ public final class DisplayManagerService extends SystemService { 
        // Something weird happened and the logical display was not created.

        Slog.w(TAG, "Rejecting request to create virtual display "

                + "because the logical display was not created.");

        mVirtualDisplayAdapter.releaseVirtualDisplayLocked(callback.asBinder(), callingUid);

        mVirtualDisplayAdapter.releaseVirtualDisplayLocked(callback.asBinder());

        mDisplayDeviceRepo.onDisplayDeviceEvent(device,

                DisplayAdapter.DISPLAY_DEVICE_EVENT_REMOVED);

        return -1;
@@ -2113,14 +2113,14 @@ public final class DisplayManagerService extends SystemService { 
        }

    }



    private void releaseVirtualDisplayInternal(IBinder appToken, int callingUid) {

    private void releaseVirtualDisplayInternal(IBinder appToken) {

        synchronized (mSyncRoot) {

            if (mVirtualDisplayAdapter == null) {

                return;

            }



            DisplayDevice device =

                    mVirtualDisplayAdapter.releaseVirtualDisplayLocked(appToken, callingUid);

                    mVirtualDisplayAdapter.releaseVirtualDisplayLocked(appToken);

            Slog.d(TAG, "Virtual Display: Display Device released");

            if (device != null) {

                // TODO: multi-display - handle virtual displays the same as other display adapters.
@@ -4744,10 +4744,9 @@ public final class DisplayManagerService extends SystemService { 


        @Override // Binder call

        public void releaseVirtualDisplay(IVirtualDisplayCallback callback) {

            final int callingUid = Binder.getCallingUid();

            final long token = Binder.clearCallingIdentity();

            try {

                releaseVirtualDisplayInternal(callback.asBinder(), callingUid);

                releaseVirtualDisplayInternal(callback.asBinder());

            } finally {

                Binder.restoreCallingIdentity(token);

            }

services/core/java/com/android/server/display/VirtualDisplayAdapter.java

+16 −8
Original line number Diff line number Diff line
@@ -91,6 +91,13 @@ public class VirtualDisplayAdapter extends DisplayAdapter { 


    private final ArrayMap<IBinder, VirtualDisplayDevice> mVirtualDisplayDevices = new ArrayMap<>();



    // When a virtual display is created, the mapping (appToken -> ownerUid) is stored here. That

    // way, when the display is released later, we can retrieve the ownerUid and decrement

    // the number of virtual displays that exist for that ownerUid. We can't use

    // Binder.getCallingUid() because the display might be released by the system process and not

    // the process that created the display.

    private final ArrayMap<IBinder, Integer> mOwnerUids = new ArrayMap<>();



    private final int mMaxDevices;

    private final int mMaxDevicesPerPackage;

    private final SparseIntArray mNoOfDevicesPerPackage = new SparseIntArray();
@@ -194,6 +201,7 @@ public class VirtualDisplayAdapter extends DisplayAdapter { 
        mVirtualDisplayDevices.put(appToken, device);

        if (getFeatureFlags().isVirtualDisplayLimitEnabled()) {

            mNoOfDevicesPerPackage.put(ownerUid, noOfDevices + 1);

            mOwnerUids.put(appToken, ownerUid);

        }



        try {
@@ -205,7 +213,7 @@ public class VirtualDisplayAdapter extends DisplayAdapter { 
            appToken.linkToDeath(device, 0);

        } catch (RemoteException ex) {

            Slog.e(TAG, "Virtual Display: error while setting up VirtualDisplayDevice", ex);

            removeVirtualDisplayDeviceLocked(appToken, ownerUid);

            removeVirtualDisplayDeviceLocked(appToken);

            device.destroyLocked(false);

            return null;

        }
@@ -252,12 +260,10 @@ public class VirtualDisplayAdapter extends DisplayAdapter { 
    /**

     * Release a virtual display that was previously created

     * @param appToken The token to identify the display

     * @param ownerUid The UID of the package, used to keep track of and limit the number of

     *                 displays created per package

     * @return The display device that has been removed

     */

    public DisplayDevice releaseVirtualDisplayLocked(IBinder appToken, int ownerUid) {

        VirtualDisplayDevice device = removeVirtualDisplayDeviceLocked(appToken, ownerUid);

    public DisplayDevice releaseVirtualDisplayLocked(IBinder appToken) {

        VirtualDisplayDevice device = removeVirtualDisplayDeviceLocked(appToken);

        if (device != null) {

            Slog.v(TAG, "Release VirtualDisplay " + device.mName);

            device.destroyLocked(true);
@@ -299,11 +305,13 @@ public class VirtualDisplayAdapter extends DisplayAdapter { 
        }

    }



    private VirtualDisplayDevice removeVirtualDisplayDeviceLocked(IBinder appToken, int ownerUid) {

        int noOfDevices = mNoOfDevicesPerPackage.get(ownerUid, /* valueIfKeyNotFound= */ 0);

    private VirtualDisplayDevice removeVirtualDisplayDeviceLocked(IBinder appToken) {

        if (getFeatureFlags().isVirtualDisplayLimitEnabled()) {

            int ownerUid = mOwnerUids.get(appToken);

            int noOfDevices = mNoOfDevicesPerPackage.get(ownerUid, /* valueIfKeyNotFound= */ 0);

            if (noOfDevices <= 1) {

                mNoOfDevicesPerPackage.delete(ownerUid);

                mOwnerUids.remove(appToken);

            } else {

                mNoOfDevicesPerPackage.put(ownerUid, noOfDevices - 1);

            }
@@ -376,7 +384,7 @@ public class VirtualDisplayAdapter extends DisplayAdapter { 
        @Override

        public void binderDied() {

            synchronized (getSyncRoot()) {

                removeVirtualDisplayDeviceLocked(mAppToken, mOwnerUid);

                removeVirtualDisplayDeviceLocked(mAppToken);

                Slog.i(TAG, "Virtual display device released because application token died: "

                    + mOwnerPackageName);

                destroyLocked(false);

services/tests/displayservicetests/src/com/android/server/display/DisplayManagerServiceTest.java

+1 −1
Original line number Diff line number Diff line
@@ -3708,7 +3708,7 @@ public class DisplayManagerServiceTest { 
                eq(config));



        bs.releaseVirtualDisplay(mMockAppToken);

        verify(mMockVirtualDisplayAdapter).releaseVirtualDisplayLocked(binder, callingUid);

        verify(mMockVirtualDisplayAdapter).releaseVirtualDisplayLocked(binder);

    }



    @Test

services/tests/displayservicetests/src/com/android/server/display/VirtualDisplayAdapterTest.java

+7 −13
Original line number Diff line number Diff line
@@ -111,14 +111,13 @@ public class VirtualDisplayAdapterTest { 
    public void testCreateAndReleaseVirtualDisplay() {

        VirtualDisplayConfig config = new VirtualDisplayConfig.Builder("test", /* width= */ 1,

                /* height= */ 1, /* densityDpi= */ 1).build();

        int ownerUid = 10;



        DisplayDevice result = mAdapter.createVirtualDisplayLocked(mMockCallback,

                /* projection= */ null, ownerUid, /* packageName= */ "testpackage",

                /* projection= */ null, /* ownerUid= */ 10, /* packageName= */ "testpackage",

                /* uniqueId= */ "uniqueId", /* surface= */ null, /* flags= */ 0, config);

        assertNotNull(result);



        result = mAdapter.releaseVirtualDisplayLocked(mMockBinder, ownerUid);

        result = mAdapter.releaseVirtualDisplayLocked(mMockBinder);

        assertNotNull(result);

    }
@@ -165,7 +164,6 @@ public class VirtualDisplayAdapterTest { 


        // Displays for the same package

        for (int i = 0; i < MAX_DEVICES_PER_PACKAGE * 2; i++) {

            // Same owner UID

            IVirtualDisplayCallback callback = createCallback();

            DisplayDevice device = mAdapter.createVirtualDisplayLocked(callback,

                    mMediaProjectionMock, 1234, "test.package", "123",
@@ -175,7 +173,6 @@ public class VirtualDisplayAdapterTest { 


        // Displays for different packages

        for (int i = 0; i < MAX_DEVICES * 2; i++) {

            // Same owner UID

            IVirtualDisplayCallback callback = createCallback();

            DisplayDevice device = mAdapter.createVirtualDisplayLocked(callback,

                    mMediaProjectionMock, 1234 + i, "test.package", "123",
@@ -205,8 +202,7 @@ public class VirtualDisplayAdapterTest { 
        }



        // Release one display

        DisplayDevice device = mAdapter.releaseVirtualDisplayLocked(callbacks.get(0).asBinder(),

                ownerUid);

        DisplayDevice device = mAdapter.releaseVirtualDisplayLocked(callbacks.get(0).asBinder());

        assertNotNull(device);

        callbacks.remove(0);
@@ -227,7 +223,7 @@ public class VirtualDisplayAdapterTest { 


        // Release all the displays

        for (IVirtualDisplayCallback cb : callbacks) {

            device = mAdapter.releaseVirtualDisplayLocked(cb.asBinder(), ownerUid);

            device = mAdapter.releaseVirtualDisplayLocked(cb.asBinder());

            assertNotNull(device);

        }

        callbacks.clear();
@@ -277,8 +273,7 @@ public class VirtualDisplayAdapterTest { 
        }



        // Release one display

        DisplayDevice device = mAdapter.releaseVirtualDisplayLocked(callbacks.get(0).asBinder(),

                firstOwnerUid);

        DisplayDevice device = mAdapter.releaseVirtualDisplayLocked(callbacks.get(0).asBinder());

        assertNotNull(device);

        callbacks.remove(0);
@@ -298,9 +293,8 @@ public class VirtualDisplayAdapterTest { 
        assertNull(device);



        // Release all the displays

        for (int i = 0; i < callbacks.size(); i++) {

            device = mAdapter.releaseVirtualDisplayLocked(callbacks.get(i).asBinder(),

                    firstOwnerUid + i);

        for (IVirtualDisplayCallback iVirtualDisplayCallback : callbacks) {

            device = mAdapter.releaseVirtualDisplayLocked(iVirtualDisplayCallback.asBinder());

            assertNotNull(device);

        }

        callbacks.clear();