Rewrite handling of interfaces to be configuration-specific

We now wait for the (mandatory) SET_CONFIGURATION request to claim interfaces.
MisterTechnik · May 14, 2024 · eda2de1 · eda2de1
1 parent 15b02ab
commit eda2de1
Show file tree

Hide file tree

Showing 3 changed files with 101 additions and 49 deletions.
diff --git a/app/src/main/java/org/cgutman/usbip/service/AttachedDeviceContext.java 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<UsbIpSubmitUrb> 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
@@ -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<UsbIpSubmitUrb> 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
@@ -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,31 +73,71 @@ 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;
 		value &= 0xFFFF;
 		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!");
 			}
 		}