From 0491f5aa0f3cc6b46bdf433bc900433b965943e2 Mon Sep 17 00:00:00 2001 From: Andrew Chant Date: Fri, 2 Mar 2018 13:18:18 -0800 Subject: [PATCH 1/2] UsbDescriptorParser: always parse in constructor UsbDescriptorParser::parseDescriptors always returned true. Remove the return value, and remove the one constructor that doesn't parse descriptors so the device is always in a parsed state. Bug: 74119682 Test: Built Change-Id: I2dd8d439405867d78102a9591dd1db36fe3959dc --- .../android/server/usb/UsbHostManager.java | 55 ++++++++----------- .../usb/descriptors/UsbDescriptorParser.java | 18 +----- .../server/usb/UsbDescriptorParserTests.java | 5 +- 3 files changed, 25 insertions(+), 53 deletions(-) diff --git a/services/usb/java/com/android/server/usb/UsbHostManager.java b/services/usb/java/com/android/server/usb/UsbHostManager.java index 0fcd075e8200d..e3e381ec6fe1d 100644 --- a/services/usb/java/com/android/server/usb/UsbHostManager.java +++ b/services/usb/java/com/android/server/usb/UsbHostManager.java @@ -329,40 +329,31 @@ public class UsbHostManager { return false; } - UsbDescriptorParser parser = new UsbDescriptorParser(deviceAddress); - if (parser.parseDescriptors(descriptors)) { - - UsbDevice newDevice = parser.toAndroidUsbDevice(); - if (newDevice == null) { - Slog.e(TAG, "Couldn't create UsbDevice object."); - // Tracking - addConnectionRecord(deviceAddress, ConnectionRecord.CONNECT_BADDEVICE, - parser.getRawDescriptors()); - } else { - mDevices.put(deviceAddress, newDevice); - - // It is fine to call this only for the current user as all broadcasts are - // sent to all profiles of the user and the dialogs should only show once. - ComponentName usbDeviceConnectionHandler = getUsbDeviceConnectionHandler(); - if (usbDeviceConnectionHandler == null) { - getCurrentUserSettings().deviceAttached(newDevice); - } else { - getCurrentUserSettings().deviceAttachedForFixedHandler(newDevice, - usbDeviceConnectionHandler); - } - - mUsbAlsaManager.usbDeviceAdded(deviceAddress, newDevice, parser); - - // Tracking - addConnectionRecord(deviceAddress, ConnectionRecord.CONNECT, - parser.getRawDescriptors()); - } - } else { - Slog.e(TAG, "Error parsing USB device descriptors for " + deviceAddress); + UsbDescriptorParser parser = new UsbDescriptorParser(deviceAddress, descriptors); + UsbDevice newDevice = parser.toAndroidUsbDevice(); + if (newDevice == null) { + Slog.e(TAG, "Couldn't create UsbDevice object."); // Tracking - addConnectionRecord(deviceAddress, ConnectionRecord.CONNECT_BADPARSE, + addConnectionRecord(deviceAddress, ConnectionRecord.CONNECT_BADDEVICE, + parser.getRawDescriptors()); + } else { + mDevices.put(deviceAddress, newDevice); + + // It is fine to call this only for the current user as all broadcasts are + // sent to all profiles of the user and the dialogs should only show once. + ComponentName usbDeviceConnectionHandler = getUsbDeviceConnectionHandler(); + if (usbDeviceConnectionHandler == null) { + getCurrentUserSettings().deviceAttached(newDevice); + } else { + getCurrentUserSettings().deviceAttachedForFixedHandler(newDevice, + usbDeviceConnectionHandler); + } + + mUsbAlsaManager.usbDeviceAdded(deviceAddress, newDevice, parser); + + // Tracking + addConnectionRecord(deviceAddress, ConnectionRecord.CONNECT, parser.getRawDescriptors()); - return false; } } diff --git a/services/usb/java/com/android/server/usb/descriptors/UsbDescriptorParser.java b/services/usb/java/com/android/server/usb/descriptors/UsbDescriptorParser.java index 956efc075d0a1..c704fe34866f8 100644 --- a/services/usb/java/com/android/server/usb/descriptors/UsbDescriptorParser.java +++ b/services/usb/java/com/android/server/usb/descriptors/UsbDescriptorParser.java @@ -43,11 +43,6 @@ public final class UsbDescriptorParser { // Obtained from the first AudioClass Header descriptor. private int mACInterfacesSpec = UsbDeviceDescriptor.USBSPEC_1_0; - public UsbDescriptorParser(String deviceAddr) { - mDeviceAddr = deviceAddr; - mDescriptors = new ArrayList(DESCRIPTORS_ALLOC_SIZE); - } - /** * Connect this parser to an existing set of already parsed descriptors. * This is useful for reporting. @@ -214,7 +209,7 @@ public final class UsbDescriptorParser { /** * @hide */ - public boolean parseDescriptors(byte[] descriptors) { + public void parseDescriptors(byte[] descriptors) { if (DEBUG) { Log.d(TAG, "parseDescriptors() - start"); } @@ -248,17 +243,6 @@ public final class UsbDescriptorParser { if (DEBUG) { Log.d(TAG, "parseDescriptors() - end " + mDescriptors.size() + " descriptors."); } - return true; - } - - /** - * @hide - */ - public boolean parseDevice() { - byte[] rawDescriptors = getRawDescriptors(); - - return rawDescriptors != null - ? parseDescriptors(rawDescriptors) : false; } public byte[] getRawDescriptors() { diff --git a/tests/UsbTests/src/com/android/server/usb/UsbDescriptorParserTests.java b/tests/UsbTests/src/com/android/server/usb/UsbDescriptorParserTests.java index f32395226f4a5..4b64083c2367d 100644 --- a/tests/UsbTests/src/com/android/server/usb/UsbDescriptorParserTests.java +++ b/tests/UsbTests/src/com/android/server/usb/UsbDescriptorParserTests.java @@ -61,10 +61,7 @@ public class UsbDescriptorParserTests { } // Testing same codepath as UsbHostManager.java:usbDeviceAdded - UsbDescriptorParser parser = new UsbDescriptorParser("test-usb-addr"); - if (!parser.parseDescriptors(descriptors)) { - fail("failed to parse descriptors."); - } + UsbDescriptorParser parser = new UsbDescriptorParser("test-usb-addr", descriptors); return parser; } From 608ec66d62647f60c3988922fead33fd7e07755e Mon Sep 17 00:00:00 2001 From: Andrew Chant Date: Fri, 2 Mar 2018 13:45:09 -0800 Subject: [PATCH 2/2] UsbHostManager: Restore inserted device logging Restore logcat logging of newly-added USB devices. Eliminate blacklist log lines. Test: Connected USB Storage and USB audio devices. Updated UsbDescriptorParserTests for device descriptor version. example output: UsbHostManager: USB device attached: vidpid 03eb:2433 mfg/product/ver/serial Libratone/Libratone_INEAR/1.00/Inear_mcu_app_0.2.1_20160304 hasAudio/HID/Storage: true/true/false UsbHostManager: USB device attached: vidpid 05dc:a82b mfg/product/ver/serial Lexar/ARA Storage /2.08/0024070163400215 hasAudio/HID/Storage: false/false/true UsbHostManager: USB device attached: vidpid 18d1:5029 mfg/product/ver/serial Google/USB-C to 3.5mm-Headphone Adapter/22.80/201405280001 hasAudio/HID/Storage: false/true/false UsbHostManager: USB device attached: vidpid 18d1:5025 mfg/product/ver/serial Google/USB-C to 3.5mm-Headphone Adapter/22.80/201405280001 hasAudio/HID/Storage: true/true/false Bug: 74119682 Change-Id: I72688f651c819d4bdc48f6d6316570ca5fc54d1e --- .../android/server/usb/UsbHostManager.java | 59 +++++++++-- .../usb/descriptors/UsbDescriptorParser.java | 24 ++++- .../usb/descriptors/UsbDeviceDescriptor.java | 39 ++++++-- .../res/raw/usbdescriptors_massstorage.bin | Bin 0 -> 50 bytes .../server/usb/UsbDescriptorParserTests.java | 92 +++++++++++++++++- 5 files changed, 188 insertions(+), 26 deletions(-) create mode 100644 tests/UsbTests/res/raw/usbdescriptors_massstorage.bin diff --git a/services/usb/java/com/android/server/usb/UsbHostManager.java b/services/usb/java/com/android/server/usb/UsbHostManager.java index e3e381ec6fe1d..67ad0907181b4 100644 --- a/services/usb/java/com/android/server/usb/UsbHostManager.java +++ b/services/usb/java/com/android/server/usb/UsbHostManager.java @@ -54,6 +54,7 @@ import java.util.LinkedList; public class UsbHostManager { private static final String TAG = UsbHostManager.class.getSimpleName(); private static final boolean DEBUG = false; + private static final int LINUX_FOUNDATION_VID = 0x1d6b; private final Context mContext; @@ -267,7 +268,6 @@ public class UsbHostManager { } private boolean isBlackListed(String deviceAddress) { - Slog.i(TAG, "isBlackListed(" + deviceAddress + ")"); int count = mHostBlacklist.length; for (int i = 0; i < count; i++) { if (deviceAddress.startsWith(mHostBlacklist[i])) { @@ -279,7 +279,6 @@ public class UsbHostManager { /* returns true if the USB device should not be accessible by applications */ private boolean isBlackListed(int clazz, int subClass) { - Slog.i(TAG, "isBlackListed(" + clazz + ", " + subClass + ")"); // blacklist hubs if (clazz == UsbConstants.USB_CLASS_HUB) return true; @@ -302,6 +301,40 @@ public class UsbHostManager { } } + private void logUsbDevice(UsbDescriptorParser descriptorParser) { + int vid = 0; + int pid = 0; + String mfg = ""; + String product = ""; + String version = ""; + String serial = ""; + + UsbDeviceDescriptor deviceDescriptor = descriptorParser.getDeviceDescriptor(); + if (deviceDescriptor != null) { + vid = deviceDescriptor.getVendorID(); + pid = deviceDescriptor.getProductID(); + mfg = deviceDescriptor.getMfgString(descriptorParser); + product = deviceDescriptor.getProductString(descriptorParser); + version = deviceDescriptor.getDeviceReleaseString(); + serial = deviceDescriptor.getSerialString(descriptorParser); + } + + if (vid == LINUX_FOUNDATION_VID) { + return; // don't care about OS-constructed virtual USB devices. + } + boolean hasAudio = descriptorParser.hasAudioInterface(); + boolean hasHid = descriptorParser.hasHIDInterface(); + boolean hasStorage = descriptorParser.hasStorageInterface(); + + String attachedString = "USB device attached: "; + attachedString += String.format("vidpid %04x:%04x", vid, pid); + attachedString += String.format(" mfg/product/ver/serial %s/%s/%s/%s", + mfg, product, version, serial); + attachedString += String.format(" hasAudio/HID/Storage: %b/%b/%b", + hasAudio, hasHid, hasStorage); + Slog.d(TAG, attachedString); + } + /* Called from JNI in monitorUsbHostBus() to report new USB devices Returns true if successful, i.e. the USB Audio device descriptors are correctly parsed and the unique device is added to the audio device list. @@ -313,10 +346,18 @@ public class UsbHostManager { Slog.d(TAG, "usbDeviceAdded(" + deviceAddress + ") - start"); } - // check class/subclass first as it is more likely to be blacklisted - if (isBlackListed(deviceClass, deviceSubclass) || isBlackListed(deviceAddress)) { + if (isBlackListed(deviceAddress)) { if (DEBUG) { - Slog.d(TAG, "device is black listed"); + Slog.d(TAG, "device address is black listed"); + } + return false; + } + UsbDescriptorParser parser = new UsbDescriptorParser(deviceAddress, descriptors); + logUsbDevice(parser); + + if (isBlackListed(deviceClass, deviceSubclass)) { + if (DEBUG) { + Slog.d(TAG, "device class is black listed"); } return false; } @@ -329,7 +370,6 @@ public class UsbHostManager { return false; } - UsbDescriptorParser parser = new UsbDescriptorParser(deviceAddress, descriptors); UsbDevice newDevice = parser.toAndroidUsbDevice(); if (newDevice == null) { Slog.e(TAG, "Couldn't create UsbDevice object."); @@ -338,6 +378,7 @@ public class UsbHostManager { parser.getRawDescriptors()); } else { mDevices.put(deviceAddress, newDevice); + Slog.d(TAG, "Added device " + newDevice); // It is fine to call this only for the current user as all broadcasts are // sent to all profiles of the user and the dialogs should only show once. @@ -367,18 +408,18 @@ public class UsbHostManager { /* Called from JNI in monitorUsbHostBus to report USB device removal */ @SuppressWarnings("unused") private void usbDeviceRemoved(String deviceAddress) { - if (DEBUG) { - Slog.d(TAG, "usbDeviceRemoved(" + deviceAddress + ") - start"); - } synchronized (mLock) { UsbDevice device = mDevices.remove(deviceAddress); if (device != null) { + Slog.d(TAG, "Removed device at " + deviceAddress + ": " + device.getProductName()); mUsbAlsaManager.usbDeviceRemoved(deviceAddress/*device*/); mSettingsManager.usbDeviceRemoved(device); getCurrentUserSettings().usbDeviceRemoved(device); // Tracking addConnectionRecord(deviceAddress, ConnectionRecord.DISCONNECT, null); + } else { + Slog.d(TAG, "Removed device at " + deviceAddress + " was already gone"); } } } diff --git a/services/usb/java/com/android/server/usb/descriptors/UsbDescriptorParser.java b/services/usb/java/com/android/server/usb/descriptors/UsbDescriptorParser.java index c704fe34866f8..e61542824083d 100644 --- a/services/usb/java/com/android/server/usb/descriptors/UsbDescriptorParser.java +++ b/services/usb/java/com/android/server/usb/descriptors/UsbDescriptorParser.java @@ -474,15 +474,33 @@ public final class UsbDescriptorParser { return hasSpeaker; } + /** + *@ hide + */ + public boolean hasAudioInterface() { + ArrayList descriptors = + getInterfaceDescriptorsForClass(UsbDescriptor.CLASSID_AUDIO); + return !descriptors.isEmpty(); + } + /** * @hide */ - public boolean hasHIDDescriptor() { + public boolean hasHIDInterface() { ArrayList descriptors = getInterfaceDescriptorsForClass(UsbDescriptor.CLASSID_HID); return !descriptors.isEmpty(); } + /** + * @hide + */ + public boolean hasStorageInterface() { + ArrayList descriptors = + getInterfaceDescriptorsForClass(UsbDescriptor.CLASSID_STORAGE); + return !descriptors.isEmpty(); + } + /** * @hide */ @@ -524,7 +542,7 @@ public final class UsbDescriptorParser { probability += 0.75f; } - if (hasMic && hasHIDDescriptor()) { + if (hasMic && hasHIDInterface()) { probability += 0.25f; } @@ -577,7 +595,7 @@ public final class UsbDescriptorParser { probability += 0.75f; } - if (hasSpeaker && hasHIDDescriptor()) { + if (hasSpeaker && hasHIDInterface()) { probability += 0.25f; } diff --git a/services/usb/java/com/android/server/usb/descriptors/UsbDeviceDescriptor.java b/services/usb/java/com/android/server/usb/descriptors/UsbDeviceDescriptor.java index e31e3a3126489..fae594ac49865 100644 --- a/services/usb/java/com/android/server/usb/descriptors/UsbDeviceDescriptor.java +++ b/services/usb/java/com/android/server/usb/descriptors/UsbDeviceDescriptor.java @@ -48,7 +48,7 @@ public final class UsbDeviceDescriptor extends UsbDescriptor { private int mDeviceRelease; // 12:2 Device Release number - BCD private byte mMfgIndex; // 14:1 Index of Manufacturer String Descriptor private byte mProductIndex; // 15:1 Index of Product String Descriptor - private byte mSerialNum; // 16:1 Index of Serial Number String Descriptor + private byte mSerialIndex; // 16:1 Index of Serial Number String Descriptor private byte mNumConfigs; // 17:1 Number of Possible Configurations private ArrayList mConfigDescriptors = @@ -91,16 +91,37 @@ public final class UsbDeviceDescriptor extends UsbDescriptor { return mDeviceRelease; } + // mDeviceRelease is binary-coded decimal, format DD.DD + public String getDeviceReleaseString() { + int hundredths = mDeviceRelease & 0xF; + int tenths = (mDeviceRelease & 0xF0) >> 4; + int ones = (mDeviceRelease & 0xF00) >> 8; + int tens = (mDeviceRelease & 0xF000) >> 12; + return String.format("%d.%d%d", tens * 10 + ones, tenths, hundredths); + } + public byte getMfgIndex() { return mMfgIndex; } + public String getMfgString(UsbDescriptorParser p) { + return p.getDescriptorString(mMfgIndex); + } + public byte getProductIndex() { return mProductIndex; } - public byte getSerialNum() { - return mSerialNum; + public String getProductString(UsbDescriptorParser p) { + return p.getDescriptorString(mProductIndex); + } + + public byte getSerialIndex() { + return mSerialIndex; + } + + public String getSerialString(UsbDescriptorParser p) { + return p.getDescriptorString(mSerialIndex); } public byte getNumConfigs() { @@ -119,16 +140,14 @@ public final class UsbDeviceDescriptor extends UsbDescriptor { Log.d(TAG, "toAndroid()"); } - String mfgName = parser.getDescriptorString(mMfgIndex); - String prodName = parser.getDescriptorString(mProductIndex); + String mfgName = getMfgString(parser); + String prodName = getProductString(parser); if (DEBUG) { Log.d(TAG, " mfgName:" + mfgName + " prodName:" + prodName); } - // Create version string in "%.%" format - String versionString = - Integer.toString(mDeviceRelease >> 8) + "." + (mDeviceRelease & 0xFF); - String serialStr = parser.getDescriptorString(mSerialNum); + String versionString = getDeviceReleaseString(); + String serialStr = getSerialString(parser); if (DEBUG) { Log.d(TAG, " versionString:" + versionString + " serialStr:" + serialStr); } @@ -159,7 +178,7 @@ public final class UsbDeviceDescriptor extends UsbDescriptor { mDeviceRelease = stream.unpackUsbShort(); mMfgIndex = stream.getByte(); mProductIndex = stream.getByte(); - mSerialNum = stream.getByte(); + mSerialIndex = stream.getByte(); mNumConfigs = stream.getByte(); return mLength; diff --git a/tests/UsbTests/res/raw/usbdescriptors_massstorage.bin b/tests/UsbTests/res/raw/usbdescriptors_massstorage.bin new file mode 100644 index 0000000000000000000000000000000000000000..1790369c50262e95399f8aa75fbe77cb40814044 GIT binary patch literal 50 zcmWe)WME=oU~ss@s=b1PiJ66!k&{V*fsv7+;TI