From eda2de12fb3a279b297a1dc031c9753df393d228 Mon Sep 17 00:00:00 2001 From: Cameron Gutman Date: Mon, 13 May 2024 19:23:26 -0500 Subject: [PATCH] Rewrite handling of interfaces to be configuration-specific We now wait for the (mandatory) SET_CONFIGURATION request to claim interfaces. --- .../usbip/service/AttachedDeviceContext.java | 18 +++++ .../cgutman/usbip/service/UsbIpService.java | 68 ++++++++----------- .../cgutman/usbip/usb/UsbControlHelper.java | 64 ++++++++++++++--- 3 files changed, 101 insertions(+), 49 deletions(-) create mode 100644 app/src/main/java/org/cgutman/usbip/service/AttachedDeviceContext.java diff --git a/app/src/main/java/org/cgutman/usbip/service/AttachedDeviceContext.java b/app/src/main/java/org/cgutman/usbip/service/AttachedDeviceContext.java new file mode 100644 index 0000000..23ffa4f --- /dev/null +++ b/app/src/main/java/org/cgutman/usbip/service/AttachedDeviceContext.java @@ -0,0 +1,18 @@ +package org.cgutman.usbip.service; + +import android.hardware.usb.UsbConfiguration; +import android.hardware.usb.UsbDevice; +import android.hardware.usb.UsbDeviceConnection; + +import org.cgutman.usbip.server.protocol.dev.UsbIpSubmitUrb; + +import java.util.HashSet; +import java.util.concurrent.ThreadPoolExecutor; + +public class AttachedDeviceContext { + public UsbDevice device; + public UsbDeviceConnection devConn; + public UsbConfiguration activeConfiguration; + public ThreadPoolExecutor requestPool; + public HashSet activeMessages; +} diff --git a/app/src/main/java/org/cgutman/usbip/service/UsbIpService.java b/app/src/main/java/org/cgutman/usbip/service/UsbIpService.java index ef93c91..75aa4df 100644 --- a/app/src/main/java/org/cgutman/usbip/service/UsbIpService.java +++ b/app/src/main/java/org/cgutman/usbip/service/UsbIpService.java @@ -577,7 +577,7 @@ public void submitUrbRequest(Socket s, UsbIpSubmitUrb msg) { // We have to handle certain control requests (SET_CONFIGURATION/SET_INTERFACE) by calling // Android APIs rather than just submitting the URB directly to the device - if (!UsbControlHelper.handleInternalControlTransfer(dev, devConn, requestType, request, value, index)) { + if (!UsbControlHelper.handleInternalControlTransfer(context, requestType, request, value, index)) { do { res = XferUtils.doControlTransfer(devConn, requestType, request, value, index, (requestType & 0x80) != 0 ? reply.inData : msg.outData, length, 1000); @@ -613,35 +613,40 @@ public void submitUrbRequest(Socket s, UsbIpSubmitUrb msg) { else { // Find the correct endpoint UsbEndpoint selectedEndpoint = null; - for (int i = 0; i < dev.getInterfaceCount(); i++) { - // Check each interface - UsbInterface iface = dev.getInterface(i); - for (int j = 0; j < iface.getEndpointCount(); j++) { - // Check the endpoint number - UsbEndpoint endpoint = iface.getEndpoint(j); - if (msg.ep == endpoint.getEndpointNumber()) { - // Check the direction - if (msg.direction == UsbIpDevicePacket.USBIP_DIR_IN) { - if (endpoint.getDirection() != UsbConstants.USB_DIR_IN) { - continue; + if (context.activeConfiguration != null) { + for (int i = 0; i < context.activeConfiguration.getInterfaceCount(); i++) { + // Check each interface + UsbInterface iface = context.activeConfiguration.getInterface(i); + for (int j = 0; j < iface.getEndpointCount(); j++) { + // Check the endpoint number + UsbEndpoint endpoint = iface.getEndpoint(j); + if (msg.ep == endpoint.getEndpointNumber()) { + // Check the direction + if (msg.direction == UsbIpDevicePacket.USBIP_DIR_IN) { + if (endpoint.getDirection() != UsbConstants.USB_DIR_IN) { + continue; + } } - } - else { - if (endpoint.getDirection() != UsbConstants.USB_DIR_OUT) { - continue; + else { + if (endpoint.getDirection() != UsbConstants.USB_DIR_OUT) { + continue; + } } + + // This the right endpoint + selectedEndpoint = endpoint; + break; } - - // This the right endpoint - selectedEndpoint = endpoint; + } + + // Check if we found the endpoint on the last interface + if (selectedEndpoint != null) { break; } } - - // Check if we found the endpoint on the last interface - if (selectedEndpoint != null) { - break; - } + } + else { + System.err.println("Attempted to transfer to non-control EP before SET_CONFIGURATION!"); } if (selectedEndpoint == null) { @@ -735,13 +740,6 @@ public boolean attachToDevice(Socket s, String busId) { return false; } - // Claim all interfaces since we don't know which one the client wants - for (int i = 0; i < dev.getInterfaceCount(); i++) { - if (!devConn.claimInterface(dev.getInterface(i), true)) { - System.err.println("Unable to claim interface "+dev.getInterface(i).getId()); - } - } - // Create a context for this attachment AttachedDeviceContext context = new AttachedDeviceContext(); context.devConn = devConn; @@ -841,11 +839,5 @@ public void abortUrbRequest(Socket s, UsbIpUnlinkUrb msg) { found ? UsbIpSubmitUrb.USBIP_STATUS_URB_ABORTED : -22); // EINVAL } - - static class AttachedDeviceContext { - public UsbDevice device; - public UsbDeviceConnection devConn; - public ThreadPoolExecutor requestPool; - public HashSet activeMessages; - } + } diff --git a/app/src/main/java/org/cgutman/usbip/usb/UsbControlHelper.java b/app/src/main/java/org/cgutman/usbip/usb/UsbControlHelper.java index 4feeb0f..a29c822 100644 --- a/app/src/main/java/org/cgutman/usbip/usb/UsbControlHelper.java +++ b/app/src/main/java/org/cgutman/usbip/usb/UsbControlHelper.java @@ -7,6 +7,8 @@ import android.hardware.usb.UsbInterface; import android.os.Build; +import org.cgutman.usbip.service.AttachedDeviceContext; + public class UsbControlHelper { private static final int GET_DESCRIPTOR_REQUEST_TYPE = 0x80; @@ -71,7 +73,7 @@ public static boolean clearHaltCondition(UsbDeviceConnection devConn, UsbEndpoin return true; } - public static boolean handleInternalControlTransfer(UsbDevice dev, UsbDeviceConnection devConn, int requestType, int request, int value, int index) { + public static boolean handleInternalControlTransfer(AttachedDeviceContext deviceContext, int requestType, int request, int value, int index) { // Mask out possible sign expansions requestType &= 0xFF; request &= 0xFF; @@ -79,23 +81,63 @@ public static boolean handleInternalControlTransfer(UsbDevice dev, UsbDeviceConn index &= 0xFFFF; if (requestType == SET_CONFIGURATION_REQUEST_TYPE && request == SET_CONFIGURATION_REQUEST) { - for (int i = 0; i < dev.getConfigurationCount(); i++) { - UsbConfiguration config = dev.getConfiguration(i); + System.out.println("Handling SET_CONFIGURATION via Android API"); + + for (int i = 0; i < deviceContext.device.getConfigurationCount(); i++) { + UsbConfiguration config = deviceContext.device.getConfiguration(i); if (config.getId() == value) { - devConn.setConfiguration(config); - System.out.println("Handled SET_CONFIGURATION via Android API"); + // If we have a current config, we need unclaim all interfaces to allow the + // configuration change to work properly. + if (deviceContext.activeConfiguration != null) { + System.out.println("Unclaiming all interfaces from old configuration: "+deviceContext.activeConfiguration.getId()); + for (int j = 0; j < deviceContext.activeConfiguration.getInterfaceCount(); j++) { + UsbInterface iface = deviceContext.activeConfiguration.getInterface(j); + deviceContext.devConn.releaseInterface(iface); + } + } + + if (!deviceContext.devConn.setConfiguration(config)) { + // This can happen for certain types of devices where Android itself + // has set the configuration for us. Let's just hope that whatever the + // client wanted is also what Android selected :/ + System.err.println("Failed to set configuration! Proceeding anyway!"); + } + + // This is now the active configuration + deviceContext.activeConfiguration = config; + + System.out.println("Claiming all interfaces from new configuration: "+deviceContext.activeConfiguration.getId()); + for (int j = 0; j < deviceContext.activeConfiguration.getInterfaceCount(); j++) { + UsbInterface iface = deviceContext.activeConfiguration.getInterface(j); + if (!deviceContext.devConn.claimInterface(iface, true)) { + System.err.println("Unable to claim interface: "+iface.getId()); + } + } + return true; } } + + System.err.printf("SET_CONFIGURATION specified invalid configuration: %d\n", value); } else if (requestType == SET_INTERFACE_REQUEST_TYPE && request == SET_INTERFACE_REQUEST) { - for (int i = 0; i < dev.getInterfaceCount(); i++) { - UsbInterface iface = dev.getInterface(i); - if (iface.getId() == index && iface.getAlternateSetting() == value) { - devConn.setInterface(iface); - System.out.println("Handled SET_INTERFACE via Android API"); - return true; + System.out.println("Handling SET_INTERFACE via Android API"); + + if (deviceContext.activeConfiguration != null) { + for (int i = 0; i < deviceContext.activeConfiguration.getInterfaceCount(); i++) { + UsbInterface iface = deviceContext.activeConfiguration.getInterface(i); + if (iface.getId() == index && iface.getAlternateSetting() == value) { + if (!deviceContext.devConn.setInterface(iface)) { + System.err.println("Unable to set interface: "+iface.getId()); + } + return true; + } } + + System.err.printf("SET_INTERFACE specified invalid interface: %d %d\n", index, value); + } + else { + System.err.println("Attempted to use SET_INTERFACE before SET_CONFIGURATION!"); } }