option: Do not try to bind to ADB interfaces
Subsystems (3)
PATCH 1/1 option: Do not try to bind to ADB interfaces
Some modems now use the Android Debug Bridge to provide a debugging interface, and some phones can also export serial ports managed by the "option" driver.
The ADB daemon running in userspace tries to use USB interfaces with bDeviceClass=0xFF, bDeviceSubClass=0x42, bDeviceProtocol=1
Prevent the option driver from binding to those interfaces, as they will not be serial ports.
This can fix issues like:
https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=781256
--- drivers/usb/serial/option.c | 6 ++++++ 1 file changed, 6 insertions(+)
drivers/usb/serial/option.c +6 -0
@@ -1987,6 +1987,12 @@ static int option_probe(struct usb_serial *serial,if (iface_desc->bInterfaceClass ==USB_CLASS_MASS_STORAGE )return -ENODEV ; +/* Do not bind Android Debug Bridge interfaces */ +if (iface_desc->bInterfaceClass ==USB_CLASS_VENDOR_SPEC && + iface_desc->bInterfaceSubClass ==0x42 && + iface_desc->bInterfaceProtocol ==1 ) +return -ENODEV ; +/* * Don't bind reserved interfaces (like network ones) which often have * the same class/subclass/protocol as the serial interfaces. Look at
Greg Kroah-Hartman
maintainerQuoted 24 lines · Romain Izard
Some modems now use the Android Debug Bridge to provide a debugging interface, and some phones can also export serial ports managed by the "option" driver.
The ADB daemon running in userspace tries to use USB interfaces with bDeviceClass=0xFF, bDeviceSubClass=0x42, bDeviceProtocol=1
Prevent the option driver from binding to those interfaces, as they will not be serial ports.
This can fix issues like:
https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=781256Signed-off-by Romain Izard <[email protected]>Cc stable <[email protected]>--- drivers/usb/serial/option.c | 6 ++++++ 1 file changed, 6 insertions(+)drivers/usb/serial/option.c +0
-0
drivers/usb/serial/option.c static int option_probe(struct usb_serial *serial, · line 1994
Quoted 8 lines
drivers/usb/serial/option.c +5
-0@@ -1987,6 +1987,12 @@ static int option_probe(struct usb_serial *serial,if (iface_desc->bInterfaceClass ==USB_CLASS_MASS_STORAGE )return -ENODEV ; +/* Do not bind Android Debug Bridge interfaces */ +if (iface_desc->bInterfaceClass ==USB_CLASS_VENDOR_SPEC && + iface_desc->bInterfaceSubClass ==0x42 && + iface_desc->bInterfaceProtocol ==1 ) +return -ENODEV ;
Shouldn't you also check the vendor/product id as well? Otherwise this has the potential to match random devices that are not really adb devices.
thanks,
greg k-h
---
To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
2018-07-23 16:08 GMT+02:00 Greg Kroah-Hartman <[email protected]>:
Quoted 28 lines · Romain Izard
Quoted 27 lines
Some modems now use the Android Debug Bridge to provide a debugging interface, and some phones can also export serial ports managed by the "option" driver.
The ADB daemon running in userspace tries to use USB interfaces with bDeviceClass=0xFF, bDeviceSubClass=0x42, bDeviceProtocol=1
Prevent the option driver from binding to those interfaces, as they will not be serial ports.
This can fix issues like:
https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=781256Signed-off-by Romain Izard <[email protected]>Cc stable <[email protected]>--- drivers/usb/serial/option.c | 6 ++++++ 1 file changed, 6 insertions(+)drivers/usb/serial/option.c +0
-0@@ -1987,6 +1987,12 @@ static int option_probe(struct usb_serial *serial,if (iface_desc->bInterfaceClass ==USB_CLASS_MASS_STORAGE )return -ENODEV ;
Quoted 9 lines
drivers/usb/serial/option.c static int option_probe(struct usb_serial *serial, · line 1994
Quoted 5 lines
drivers/usb/serial/option.c +5
-0+/* Do not bind Android Debug Bridge interfaces */ +if (iface_desc->bInterfaceClass ==USB_CLASS_VENDOR_SPEC && + iface_desc->bInterfaceSubClass ==0x42 && + iface_desc->bInterfaceProtocol ==1 ) +return -ENODEV ;Shouldn't you also check the vendor/product id as well? Otherwise this has the potential to match random devices that are not really adb devices.
The only random devices are those that already match with the option driver, either with the whole device or the whole reserved class. It reduces the amount of potentially affected devices.
Among those, I do not expect any of them to use 0xff,0x42,0x01 for a serial port. But if it occurred, it would be necessary to revert this change as no userspace hack would allow to rebind the interface.
As this is only an intuition, please discard this patch if you have any doubt about this.
Best regards,
Johan Hovold
maintainerQuoted 39 lines · Romain Izard
2018-07-23 16:08 GMT+02:00 Greg Kroah-Hartman <[email protected]>:
Quoted 28 lines · Romain Izard
Quoted 27 lines
Some modems now use the Android Debug Bridge to provide a debugging interface, and some phones can also export serial ports managed by the "option" driver.
The ADB daemon running in userspace tries to use USB interfaces with bDeviceClass=0xFF, bDeviceSubClass=0x42, bDeviceProtocol=1
Prevent the option driver from binding to those interfaces, as they will not be serial ports.
This can fix issues like:
https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=781256Signed-off-by Romain Izard <[email protected]>Cc stable <[email protected]>--- drivers/usb/serial/option.c | 6 ++++++ 1 file changed, 6 insertions(+)drivers/usb/serial/option.c +0
-0@@ -1987,6 +1987,12 @@ static int option_probe(struct usb_serial *serial,if (iface_desc->bInterfaceClass ==USB_CLASS_MASS_STORAGE )return -ENODEV ;Quoted 9 lines
drivers/usb/serial/option.c static int option_probe(struct usb_serial *serial, · line 1994
Quoted 5 lines
drivers/usb/serial/option.c +5
-0+/* Do not bind Android Debug Bridge interfaces */ +if (iface_desc->bInterfaceClass ==USB_CLASS_VENDOR_SPEC && + iface_desc->bInterfaceSubClass ==0x42 && + iface_desc->bInterfaceProtocol ==1 ) +return -ENODEV ;Shouldn't you also check the vendor/product id as well? Otherwise this has the potential to match random devices that are not really adb devices.
Quoted 7 lines
The only random devices are those that already match with the option driver, either with the whole device or the whole reserved class. It reduces the amount of potentially affected devices.
Among those, I do not expect any of them to use 0xff,0x42,0x01 for a serial port. But if it occurred, it would be necessary to revert this change as no userspace hack would allow to rebind the interface.
Yeah, but by that time we may have added (or enabled) devices that rely on this general rule so we'd need to start adding exceptions to this negative matching rule instead...
Quoted 2 lines
As this is only an intuition, please discard this patch if you have any doubt about this.
Bjørn also suggested that we at least consider adding a rule like this a few months ago (when I changed the blacklist implementation). I just never got around to look into it.
It would allow for simpler device-id entries, at least when ADB is the only blacklisted interface, and may enable ADB for some older entries.
On the other hand, interface class 0xff is indeed supposed to be vendor specific as Lars and Greg pointed out, and with status quo we don't cause any regressions. If ADB isn't currently available for some device due to option binding to that interface, we'll just blacklist it as soon we get a report.
So personally I'm not sure it's worth it, but I don't have a strong opinion on the matter either.
Johan
Quoted 11 lines · Johan Hovold
It would allow for simpler device-id entries, at least when ADB is the only blacklisted interface, and may enable ADB for some older entries.
On the other hand, interface class
0xffis indeed supposed to be vendor specific as Lars and Greg pointed out, and with status quo we don't cause any regressions. If ADB isn't currently available for some device due to option binding to that interface, we'll just blacklist it as soon we get a report.So personally I'm not sure it's worth it, but I don't have a strong opinion on the matter either.
+1
The adb userspace application is also free to unbind any conflicting driver, so I don't think blacklisting is strictly necessary. Except to prevent any confusion caused by bogus ttyUSBx devices.
Bjørn
Johan Hovold
maintainerQuoted 11 lines · Bjørn Mork
Quoted 9 lines · Johan Hovold
It would allow for simpler device-id entries, at least when ADB is the only blacklisted interface, and may enable ADB for some older entries.
On the other hand, interface class
0xffis indeed supposed to be vendor specific as Lars and Greg pointed out, and with status quo we don't cause any regressions. If ADB isn't currently available for some device due to option binding to that interface, we'll just blacklist it as soon we get a report.
Quoted 8 lines
Quoted 2 lines
So personally I'm not sure it's worth it, but I don't have a strong opinion on the matter either.
+1
The adb userspace application is also free to unbind any conflicting driver, so I don't think blacklisting is strictly necessary. Except to prevent any confusion caused by bogus ttyUSBx devices.
Right. Let's leave things as they are then.
Thanks,
Johan
Quoted 9 lines · Romain Izard
Some modems now use the Android Debug Bridge to provide a debugging interface, and some phones can also export serial ports managed by the "option" driver.
The ADB daemon running in userspace tries to use USB interfaces with bDeviceClass=0xFF, bDeviceSubClass=0x42, bDeviceProtocol=1
Prevent the option driver from binding to those interfaces, as they will not be serial ports.
You are assuming that an interface with these attributes are always a ADB interface - that is wrong. Vendor specific class (0xff) is not standardized to be something specific.
Quoted 3 lines
This can fix issues like:
https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=781256
You are trying to solve a 4++ years old bug report where it was assumed that the option driver was the culprit. The device in question, a Qualcomm modem with vid/pid 05c6:9025 has never been included in option.
rgds
Lars
---
To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
2018-07-23 17:15 GMT+02:00 Lars Melin <[email protected]>:
Quoted 9 lines · Romain Izard
Quoted 8 lines
Some modems now use the Android Debug Bridge to provide a debugging interface, and some phones can also export serial ports managed by the "option" driver.
The ADB daemon running in userspace tries to use USB interfaces with bDeviceClass=0xFF, bDeviceSubClass=0x42, bDeviceProtocol=1
Quoted 7 lines
Quoted 2 lines
Prevent the option driver from binding to those interfaces, as they will not be serial ports.
You are assuming that an interface with these attributes are always a ADB interface - that is wrong. Vendor specific class (
0xff) is not standardized to be something specific.
Yes. And the option driver binds to all the vendor-specific interfaces for many devices, assuming all the vendor-specific interfaces are serial ports, unless they are blacklisted.
Quoted 8 lines
Quoted 4 lines
This can fix issues like:
https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=781256You are trying to solve a 4++ years old bug report where it was assumed that the option driver was the culprit. The device in question, a Qualcomm modem with vid/pid 05c6:9025 has never been included in option.
While I'm not going to investigate why the 3.16 kernel did it, in the bug report the logs indicated that 05c6:9025 was bound to the option driver. But I did not try to solve this issue directly, I reported it as it led me to the solution for a similar problem that I encountered last week.
A vendor for an SDK based on a modem was using an old Ubuntu 16.04, and everything was working correctly. But on my 18.04, and on an up-to-date 16.04 LTS, it was impossible to use ADB with the vendor's SDK.
As ADB has many other failure modes, it took a lot of time to see the source of the problem. The option driver in the old Ubuntu did not know about the modem's vid/pid, so the driver was not bound by default and the ADB interface was free. But the modem was added in the stable branch of the kernel, so the driver was greedily binding to all interfaces that were not blacklisted. As a result, it was not possible to use ADB as long as the interface was bound to the option driver.
One of the reasons for the problem, of course, was the reuse by the modem manufacturer of the same vid/pid for the standard version of the modem and for the SDK version.
I see my change as a useful heuristic. ADB is a quite common protocol, using one of the 65536 vendor subclass/protocol combinations. As you can see in
https://github.com/apkudo/adbusbini more than 3000 vendors have been spotted using it in the wild, including more than half of those mentioned in the option driver.
And on the other hand, I don't expect any existing modem to use 0xff,0x42,0x01 for its serial port interface, and a USB facedancer would not gain anything by doing so.
Best regards,