[PATCH net] cdc_ether: Ignore bogus union descriptor for RNDIS devices

[PATCH net] cdc_ether: Ignore bogus union descriptor for RNDIS devices

[Bjørn Mork]

Some RNDIS devices include a bogus CDC Union descriptor pointing
to non-existing interfaces.  The RNDIS code is already prepared
to handle devices without a CDC Union descriptor by hardwiring
the driver to use interfaces 0 and 1, which is correct for the
devices with the bogus descriptor as well. So we can reuse the
existing workaround.

Cc: Markus Kolb <linux-201011@tower-net.de>
Cc: Iker Salmón San Millán <shaola@esdebian.org>
Cc: Jonathan Nieder <jrnieder@gmail.com>
Cc: Oliver Neukum <oliver@neukum.org>
Cc: 655387@bugs.debian.org
Cc: stable@vger.kernel.org
Signed-off-by: Bjørn Mork <bjorn@mork.no>
---
Something like this, then?

This has only been build tested by me as I don't have any of the affected
devices.  It would be great if any of the reporters could verify that it
solves the problem.  I can provide prebuilt modules for Debian 3.2 based
kernels to simplify the testing (just rmmod and insmod - no need to build
or boot).  Send me an email if you want the module.

Sorry for the long delay in following up this.  I totally forgot about it.


Bjørn

 drivers/net/usb/cdc_ether.c |   12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/drivers/net/usb/cdc_ether.c b/drivers/net/usb/cdc_ether.c
index 90a3002..4ed5a38 100644
--- a/drivers/net/usb/cdc_ether.c
+++ b/drivers/net/usb/cdc_ether.c
@@ -83,6 +83,7 @@ int usbnet_generic_cdc_bind(struct usbnet *dev, struct usb_interface *intf)
 	struct cdc_state		*info = (void *) &dev->data;
 	int				status;
 	int				rndis;
+	bool				android_rndis_quirk = false;
 	struct usb_driver		*driver = driver_of(intf);
 	struct usb_cdc_mdlm_desc	*desc = NULL;
 	struct usb_cdc_mdlm_detail_desc *detail = NULL;
@@ -195,6 +196,11 @@ int usbnet_generic_cdc_bind(struct usbnet *dev, struct usb_interface *intf)
 					info->control,
 					info->u->bSlaveInterface0,
 					info->data);
+				/* fall back to hard-wiring for RNDIS */
+				if (rndis) {
+					android_rndis_quirk = true;
+					goto next_desc;
+				}
 				goto bad_desc;
 			}
 			if (info->control != intf) {
@@ -271,8 +277,12 @@ next_desc:
 	/* Microsoft ActiveSync based and some regular RNDIS devices lack the
 	 * CDC descriptors, so we'll hard-wire the interfaces and not check
 	 * for descriptors.
+	 *
+	 * Some Android RNDIS devices have a CDC Union descriptor pointing
+	 * to non-existing interfaces.  Ignore that and attempt the same
+	 * hard-wired 0 and 1 interfaces.
 	 */
-	if (rndis && !info->u) {
+	if (rndis && (!info->u || android_rndis_quirk)) {
 		info->control = usb_ifnum_to_if(dev->udev, 0);
 		info->data = usb_ifnum_to_if(dev->udev, 1);
 		if (!info->control || !info->data) {
-- 
1.7.10

Re: [PATCH net] cdc_ether: Ignore bogus union descriptor for RNDIS devices

[Oliver Neukum]

Am Donnerstag, 26. April 2012, 12:55:03 schrieb Bjørn Mork:
> Some RNDIS devices include a bogus CDC Union descriptor pointing
> to non-existing interfaces.  The RNDIS code is already prepared
> to handle devices without a CDC Union descriptor by hardwiring
> the driver to use interfaces 0 and 1, which is correct for the
> devices with the bogus descriptor as well. So we can reuse the
> existing workaround.

Very good, except that this is a bit risky, because we may use
interface 0 without checking that it belongs to us. Could you
add a sanity check that the interface for which probe() is called
is really the control interface in case a workaround is used?

	Regards
		Oliver

[PATCH net v2] cdc_ether: Ignore bogus union descriptor for RNDIS devices

[Bjørn Mork]

Some RNDIS devices include a bogus CDC Union descriptor pointing
to non-existing interfaces.  The RNDIS code is already prepared
to handle devices without a CDC Union descriptor by hardwiring
the driver to use interfaces 0 and 1, which is correct for the
devices with the bogus descriptor as well. So we can reuse the
existing workaround.

Cc: Markus Kolb <linux-201011@tower-net.de>
Cc: Iker Salmón San Millán <shaola@esdebian.org>
Cc: Jonathan Nieder <jrnieder@gmail.com>
Cc: Oliver Neukum <oliver@neukum.org>
Cc: 655387@bugs.debian.org
Cc: stable@vger.kernel.org
Signed-off-by: Bjørn Mork <bjorn@mork.no>
---
v2: adding a test to verify that the resulting control interface is the
    probed interface, as suggested by Oliver Neukum

The same comments as for v1 regarding testing applies.  This is build
tested only.  Should go through some functional testing before being
applied.


Bjørn

 drivers/net/usb/cdc_ether.c |   14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/drivers/net/usb/cdc_ether.c b/drivers/net/usb/cdc_ether.c
index 90a3002..00880ed 100644
--- a/drivers/net/usb/cdc_ether.c
+++ b/drivers/net/usb/cdc_ether.c
@@ -83,6 +83,7 @@ int usbnet_generic_cdc_bind(struct usbnet *dev, struct usb_interface *intf)
 	struct cdc_state		*info = (void *) &dev->data;
 	int				status;
 	int				rndis;
+	bool				android_rndis_quirk = false;
 	struct usb_driver		*driver = driver_of(intf);
 	struct usb_cdc_mdlm_desc	*desc = NULL;
 	struct usb_cdc_mdlm_detail_desc *detail = NULL;
@@ -195,6 +196,11 @@ int usbnet_generic_cdc_bind(struct usbnet *dev, struct usb_interface *intf)
 					info->control,
 					info->u->bSlaveInterface0,
 					info->data);
+				/* fall back to hard-wiring for RNDIS */
+				if (rndis) {
+					android_rndis_quirk = true;
+					goto next_desc;
+				}
 				goto bad_desc;
 			}
 			if (info->control != intf) {
@@ -271,11 +277,15 @@ next_desc:
 	/* Microsoft ActiveSync based and some regular RNDIS devices lack the
 	 * CDC descriptors, so we'll hard-wire the interfaces and not check
 	 * for descriptors.
+	 *
+	 * Some Android RNDIS devices have a CDC Union descriptor pointing
+	 * to non-existing interfaces.  Ignore that and attempt the same
+	 * hard-wired 0 and 1 interfaces.
 	 */
-	if (rndis && !info->u) {
+	if (rndis && (!info->u || android_rndis_quirk)) {
 		info->control = usb_ifnum_to_if(dev->udev, 0);
 		info->data = usb_ifnum_to_if(dev->udev, 1);
-		if (!info->control || !info->data) {
+		if (!info->control || !info->data || info->control != intf) {
 			dev_dbg(&intf->dev,
 				"rndis: master #0/%p slave #1/%p\n",
 				info->control,
-- 
1.7.10

Re: [PATCH net v2] cdc_ether: Ignore bogus union descriptor for RNDIS devices

[David Miller]

From: Bjørn Mork <bjorn-yOkvZcmFvRU@public.gmane.org>
Date: Thu, 26 Apr 2012 14:35:10 +0200

> The same comments as for v1 regarding testing applies.  This is build
> tested only.  Should go through some functional testing before being
> applied.

Well?  Is anyone gonna test this?
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH net v2] cdc_ether: Ignore bogus union descriptor for RNDIS devices

[Markus Kolb]

David Miller <davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org> schrieb:

>From: Bjørn Mork <bjorn-yOkvZcmFvRU@public.gmane.org>
>Date: Thu, 26 Apr 2012 14:35:10 +0200
>
>> The same comments as for v1 regarding testing applies.  This is build
>> tested only.  Should go through some functional testing before being
>> applied.
>
>Well?  Is anyone gonna test this?

I'll build it during next rainy day and will report its success after some usage ;-)

Markus
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH net v2] cdc_ether: Ignore bogus union descriptor for RNDIS devices

[David Miller]

From: Markus Kolb <linux-201011-TzK+PxyQ8t4hFhg+JK9F0w@public.gmane.org>
Date: Thu, 03 May 2012 06:57:39 +0200

> I'll build it during next rainy day and will report its success
> after some usage ;-)

Thank you.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH net v2] cdc_ether: Ignore bogus union descriptor for RNDIS devices

[Markus Kolb]

David Miller wrote on 03.05.2012 07:11:
> From: Markus Kolb<linux-201011-TzK+PxyQ8t4hFhg+JK9F0w@public.gmane.org>
> Date: Thu, 03 May 2012 06:57:39 +0200
>
>> I'll build it during next rainy day and will report its success
>> after some usage ;-)
>
> Thank you.

Works without any problem for me.
So maybe this patch will be included in the official kernels before I'll 
buy a new mobile phone?! ;-)
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH net v2] cdc_ether: Ignore bogus union descriptor for RNDIS devices

[David Miller]

From: Markus Kolb <markus-cHz7HMenPf4hFhg+JK9F0w@public.gmane.org>
Date: Sun, 06 May 2012 12:13:32 +0200

> David Miller wrote on 03.05.2012 07:11:
>> From: Markus Kolb<linux-201011-TzK+PxyQ8t4hFhg+JK9F0w@public.gmane.org>
>> Date: Thu, 03 May 2012 06:57:39 +0200
>>
>>> I'll build it during next rainy day and will report its success
>>> after some usage ;-)
>>
>> Thank you.
> 
> Works without any problem for me.
> So maybe this patch will be included in the official kernels before
> I'll buy a new mobile phone?! ;-)

Applied.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html