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

Commit 6c7940e4 authored by Sandro Meier's avatar Sandro Meier
Browse files

Fix virtual input device creation timeouts 

This change fixes a bug where a too long device name caused a timeout
during the creation of the virtual input device. Names longer than 80
bytes where truncated.
It also adds a new check that device names must be unique. This solves
multiple problems:
- Debugging is easier as device names are logged and otherwise
  indistinguishable
- If multiple devices with the same name are created and the creation of
  one of them fails it was not possible to identify which one

Bug: 263519472
Bug: 266049753
Bug: 266049980
Test: atest FrameworksServicesTests:InputControllerTest
Test: atest CtsHardwareTestCases
Change-Id: I040f859253f4f544e23cf01a88afb9a2bac26b9f
parent d2b9f813

services/companion/java/com/android/server/companion/virtual/InputController.java

+46 −4
Original line number Diff line number Diff line
@@ -46,6 +46,7 @@ import com.android.server.input.InputManagerInternal; 
import java.io.PrintWriter;

import java.lang.annotation.Retention;

import java.lang.annotation.RetentionPolicy;

import java.nio.charset.StandardCharsets;

import java.util.Iterator;

import java.util.Map;

import java.util.Objects;
@@ -79,6 +80,14 @@ class InputController { 
    @interface PhysType {

    }



    /**

     * The maximum length of a device name (in bytes in UTF-8 encoding).

     *

     * This limitation comes directly from uinput.

     * See also UINPUT_MAX_NAME_SIZE in linux/uinput.h

     */

    private static final int DEVICE_NAME_MAX_LENGTH = 80;



    final Object mLock;



    /* Token -> file descriptor associations. */
@@ -312,6 +321,34 @@ class InputController { 
        }

    }



    /**

     * Validates a device name by checking length and whether a device with the same name

     * already exists. Throws exceptions if the validation fails.

     * @param deviceName The name of the device to be validated

     * @throws DeviceCreationException if {@code deviceName} is not valid.

     */

    private void validateDeviceName(String deviceName) throws DeviceCreationException {

        // Comparison is greater or equal because the device name must fit into a const char*

        // including the \0-terminator. Therefore the actual number of bytes that can be used

        // for device name is DEVICE_NAME_MAX_LENGTH - 1

        if (deviceName.getBytes(StandardCharsets.UTF_8).length >= DEVICE_NAME_MAX_LENGTH) {

            throw new DeviceCreationException(

                    "Input device name exceeds maximum length of " + DEVICE_NAME_MAX_LENGTH

                            + "bytes: " + deviceName);

        }



        synchronized (mLock) {

            InputDeviceDescriptor[] values = mInputDeviceDescriptors.values().toArray(

                    new InputDeviceDescriptor[0]);

            for (InputDeviceDescriptor value : values) {

                if (value.mName.equals(deviceName)) {

                    throw new DeviceCreationException(

                            "Input device name already in use: " + deviceName);

                }

            }

        }

    }



    private static String createPhys(@PhysType String type) {

        return String.format("virtual%s:%d", type, sNextPhysId.getAndIncrement());

    }
@@ -451,10 +488,10 @@ class InputController { 


    @VisibleForTesting

    void addDeviceForTesting(IBinder deviceToken, int fd, int type, int displayId, String phys,

            int inputDeviceId) {

            String deviceName, int inputDeviceId) {

        synchronized (mLock) {

            mInputDeviceDescriptors.put(deviceToken, new InputDeviceDescriptor(fd, () -> {

            }, type, displayId, phys, inputDeviceId));

            }, type, displayId, phys, deviceName, inputDeviceId));

        }

    }
@@ -565,6 +602,9 @@ class InputController { 
        private final @Type int mType;

        private final int mDisplayId;

        private final String mPhys;

        // The name given to this device by the client. Enforced to be unique within

        // InputController.

        private final String mName;

        // The input device id that was associated to the device by the InputReader on device

        // creation.

        private final int mInputDeviceId;
@@ -572,12 +612,13 @@ class InputController { 
        private final long mCreationOrderNumber;



        InputDeviceDescriptor(int fd, IBinder.DeathRecipient deathRecipient, @Type int type,

                int displayId, String phys, int inputDeviceId) {

                int displayId, String phys, String name, int inputDeviceId) {

            mFd = fd;

            mDeathRecipient = deathRecipient;

            mType = type;

            mDisplayId = displayId;

            mPhys = phys;

            mName = name;

            mInputDeviceId = inputDeviceId;

            mCreationOrderNumber = sNextCreationOrderNumber.getAndIncrement();

        }
@@ -733,6 +774,7 @@ class InputController { 
                    "Virtual device creation should happen on an auxiliary thread (e.g. binder "

                            + "thread) and not from the handler's thread.");

        }

        validateDeviceName(deviceName);



        final int fd;

        final BinderDeathRecipient binderDeathRecipient;
@@ -769,7 +811,7 @@ class InputController { 
        synchronized (mLock) {

            mInputDeviceDescriptors.put(deviceToken,

                    new InputDeviceDescriptor(fd, binderDeathRecipient, type, displayId, phys,

                            inputDeviceId));

                            deviceName, inputDeviceId));

        }

    }

services/tests/servicestests/src/com/android/server/companion/virtual/InputControllerTest.java

+69 −5
Original line number Diff line number Diff line
@@ -18,12 +18,12 @@ package com.android.server.companion.virtual; 


import static com.google.common.truth.Truth.assertWithMessage;



import static org.junit.Assert.assertThrows;

import static org.mockito.ArgumentMatchers.anyInt;

import static org.mockito.ArgumentMatchers.anyString;

import static org.mockito.ArgumentMatchers.eq;

import static org.mockito.ArgumentMatchers.startsWith;

import static org.mockito.Mockito.doReturn;

import static org.mockito.Mockito.times;

import static org.mockito.Mockito.verify;



import android.hardware.display.DisplayManagerInternal;
@@ -133,14 +133,14 @@ public class InputControllerTest { 
    @Test

    public void unregisterInputDevice_anotherMouseExists_setPointerDisplayIdOverride() {

        final IBinder deviceToken = new Binder();

        mInputController.createMouse("name", /*vendorId= */ 1, /*productId= */ 1, deviceToken,

        mInputController.createMouse("mouse1", /*vendorId= */ 1, /*productId= */ 1, deviceToken,

                /* displayId= */ 1);

        verify(mNativeWrapperMock).openUinputMouse(eq("name"), eq(1), eq(1), anyString());

        verify(mNativeWrapperMock).openUinputMouse(eq("mouse1"), eq(1), eq(1), anyString());

        verify(mInputManagerInternalMock).setVirtualMousePointerDisplayId(eq(1));

        final IBinder deviceToken2 = new Binder();

        mInputController.createMouse("name", /*vendorId= */ 1, /*productId= */ 1, deviceToken2,

        mInputController.createMouse("mouse2", /*vendorId= */ 1, /*productId= */ 1, deviceToken2,

                /* displayId= */ 2);

        verify(mNativeWrapperMock, times(2)).openUinputMouse(eq("name"), eq(1), eq(1), anyString());

        verify(mNativeWrapperMock).openUinputMouse(eq("mouse2"), eq(1), eq(1), anyString());

        verify(mInputManagerInternalMock).setVirtualMousePointerDisplayId(eq(2));

        mInputController.unregisterInputDevice(deviceToken);

        verify(mInputManagerInternalMock).setVirtualMousePointerDisplayId(eq(1));
@@ -193,4 +193,68 @@ public class InputControllerTest { 
        mInputController.unregisterInputDevice(deviceToken);

        verify(mInputManagerInternalMock).removeKeyboardLayoutAssociation(anyString());

    }



    @Test

    public void createInputDevice_tooLongNameRaisesException() {

        final IBinder deviceToken = new Binder("device");

        // The underlying uinput implementation only supports device names up to 80 bytes. This

        // string is all ASCII characters, therefore if we have more than 80 ASCII characters we

        // will have more than 80 bytes.

        String deviceName =

                "This.is.a.very.long.device.name.that.exceeds.the.maximum.length.of.80.bytes"

                        + ".by.a.couple.bytes";



        assertThrows(RuntimeException.class, () -> {

            mInputController.createDpad(deviceName, /*vendorId= */3, /*productId=*/3, deviceToken,

                    1);

        });

    }



    @Test

    public void createInputDevice_tooLongDeviceNameRaisesException() {

        final IBinder deviceToken = new Binder("device");

        // The underlying uinput implementation only supports device names up to 80 bytes (including

        // a 0-byte terminator).

        // This string is 79 characters and 80 bytes (including the 0-byte terminator)

        String deviceName =

                "This.is.a.very.long.device.name.that.exceeds.the.maximum.length01234567890123456";



        assertThrows(RuntimeException.class, () -> {

            mInputController.createDpad(deviceName, /*vendorId= */3, /*productId=*/3, deviceToken,

                    1);

        });

    }



    @Test

    public void createInputDevice_stringWithLessThanMaxCharsButMoreThanMaxBytesRaisesException() {

        final IBinder deviceToken = new Binder("device1");



        // Has only 39 characters but is 109 bytes as utf-8

        String device_name =

                "░▄▄▄▄░\n" +

                "▀▀▄██►\n" +

                "▀▀███►\n" +

                "░▀███►░█►\n" +

                "▒▄████▀▀";



        assertThrows(RuntimeException.class, () -> {

            mInputController.createDpad(device_name, /*vendorId= */5, /*productId=*/5,

                    deviceToken, 1);

        });

    }



    @Test

    public void createInputDevice_duplicateNamesAreNotAllowed() {

        final IBinder deviceToken1 = new Binder("deviceToken1");

        final IBinder deviceToken2 = new Binder("deviceToken2");



        final String sharedDeviceName = "DeviceName";



        mInputController.createDpad(sharedDeviceName, /*vendorId= */4, /*productId=*/4,

                deviceToken1, 1);

        assertThrows("Device names need to be unique", RuntimeException.class, () -> {

            mInputController.createDpad(sharedDeviceName, /*vendorId= */5, /*productId=*/5,

                    deviceToken2, 2);

        });

    }

}

services/tests/servicestests/src/com/android/server/companion/virtual/VirtualDeviceManagerServiceTest.java

+53 −41

File changed.

Preview size limit exceeded, changes collapsed.