NetBSD Problem Report #56762

From ryo_on@yk.rim.or.jp  Fri Mar 18 16:28:02 2022
Return-Path: <ryo_on@yk.rim.or.jp>
Received: from mail.netbsd.org (mail.netbsd.org [199.233.217.200])
	(using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits))
	(Client CN "mail.NetBSD.org", Issuer "mail.NetBSD.org CA" (not verified))
	by mollari.NetBSD.org (Postfix) with ESMTPS id 51CD81A921F
	for <gnats-bugs@gnats.NetBSD.org>; Fri, 18 Mar 2022 16:28:02 +0000 (UTC)
Message-Id: <4KKnTH2QnJz5pxP@mail.SiriusCloud.jp>
Date: Sat, 19 Mar 2022 00:08:31 +0900
From: ryoon@NetBSD.org
Reply-To: ryoon@NetBSD.org
To: gnats-bugs@NetBSD.org
Subject: urndis(4) autoconfiguration errors with recent usbdi.c
X-Send-Pr-Version: 3.95

>Number:         56762
>Category:       kern
>Synopsis:       urndis(4) autoconfiguration errors with recent usbdi.c
>Confidential:   no
>Severity:       serious
>Priority:       medium
>Responsible:    kern-bug-people
>State:          closed
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Fri Mar 18 16:30:01 +0000 2022
>Closed-Date:    Tue Mar 22 09:47:06 +0000 2022
>Last-Modified:  Tue Mar 22 09:47:06 +0000 2022
>Originator:     Ryo ONODERA
>Release:        NetBSD 9.99.94
>Organization:
Ryo ONODERA // ryo@tetera.org
PGP fingerprint = 82A2 DC91 76E0 A10A 8ABB  FD1B F404 27FA C7D1 15F3
>Environment:


System: NetBSD brownie 9.99.94 NetBSD 9.99.94 (DTRACE7) #31: Thu Mar 17 19:28:39 JST 2022 ryoon@brownie:/usr/world/9.99/amd64/obj/sys/arch/amd64/compile/DTRACE7 amd64
Architecture: x86_64
Machine: amd64
>Description:
sys/dev/usb/usbdi.c r1.237 has two problem in usbd_get_no_alts()
function for my urndis(4) device in Samsung Galaxy A7 (2018) SM-A750C
Android smartphone and cdce(4) devices.

Problem #1:
Error messages are as follows:

urndis0 at uhub4 port 1 configuration 1 interface 0
urndis0: SAMSUNG (0x04e8) SAMSUNG_Android (0x6863), rev 2.00/4.00, addr 9
autoconfiguration error: urndis0: could not find data bulk in
autoconfiguration error: urndis0: could not find data bulk out

bLength of USB interface descriptor is 9 (=sizeof(*d)) or lower.
The USB interface descriptor longer than 9 should be rejected.
In usbd_get_no_alts() function,

	if (d->bLength < sizeof(*d) || d->bLength > end - p)
	   break;

should be

	if (d->bLength > sizeof(*d) || d->bLength > end - p)
	   break;

Problem #2:
For cdce(4) device, valid USB interface descriptor
appears after longer non-interface descriptor.
The current logic in usbd_get_no_alts() function does not check
type of the descriptors before length check.
So 'd->bLength > sizeof(*d)' is true for
the longer non-interface descriptor and usbd_get_no_alts()
function returns 0. And cdce(4) device emits autoconfiguration errors.
The non-interface descriptors should be skipped.
(With xhci chips in my Dell XPS 13 9300 laptop, cdce(4) device
does not work at all. However it is another problem.)

Two problems may be fixed with the following patch.

Index: sys/dev/usb/usbdi.c
===================================================================
RCS file: /cvsroot/src/sys/dev/usb/usbdi.c,v
retrieving revision 1.238
diff -u -r1.238 usbdi.c
--- sys/dev/usb/usbdi.c	13 Mar 2022 13:07:39 -0000	1.238
+++ sys/dev/usb/usbdi.c	17 Mar 2022 13:26:59 -0000
@@ -978,10 +978,11 @@

 	for (n = 0; end - p >= sizeof(*d); p += d->bLength) {
 		d = (usb_interface_descriptor_t *)p;
-		if (d->bLength < sizeof(*d) || d->bLength > end - p)
+		if (d->bDescriptorType != UDESC_INTERFACE)
+			continue;
+		if (d->bLength > sizeof(*d) || d->bLength > end - p)
 			break;
-		if (d->bDescriptorType == UDESC_INTERFACE &&
-		    d->bInterfaceNumber == ifaceno)
+		if (d->bInterfaceNumber == ifaceno)
 			n++;
 	}
 	return n;

Thank you.

>How-To-Repeat:
Plug urndis(4) and cdce(4) devices to NetBSD 9.99.94.
If your cdce(4) device is deteted ure(4), disabling ure(4) in 'boot -c'
will enable cdce(4) for your device.

>Fix:

See above or download my patch from:
https://www.ryoon.net/~ryoon/src-sys-dev-usb-usbdi.c-urndis-fix-20220317.diff .

>Release-Note:

>Audit-Trail:
From: "Taylor R Campbell" <riastradh@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/56762 CVS commit: src/sys/dev/usb
Date: Sat, 19 Mar 2022 10:05:52 +0000

 Module Name:	src
 Committed By:	riastradh
 Date:		Sat Mar 19 10:05:52 UTC 2022

 Modified Files:
 	src/sys/dev/usb: usbdi.c

 Log Message:
 usbdi(9): Fix usbd_get_no_alts.

 This incorrectly rejected the configuration as invalid if any
 descriptor is not large enough to be interface descriptors.

 Instead, it should reject the configuration only if any descriptor is
 not large enough to be a _descriptor_, or if any interface-type
 descriptor is not large enough to be an interface descriptor, but
 skip over descriptors of other types even if they're smaller than
 interface descriptors.

 Candidate fix for PR kern/56762.


 To generate a diff of this commit:
 cvs rdiff -u -r1.238 -r1.239 src/sys/dev/usb/usbdi.c

 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.

State-Changed-From-To: open->feedback
State-Changed-By: riastradh@NetBSD.org
State-Changed-When: Mon, 21 Mar 2022 22:44:12 +0000
State-Changed-Why:
Committed a candidate fix to HEAD -- can you see if it works now?


From: Ryo ONODERA <ryo@tetera.org>
To: gnats-bugs@netbsd.org, riastradh@NetBSD.org
Cc: 
Subject: Re: kern/56762 (urndis(4) autoconfiguration errors with recent
 usbdi.c)
Date: Tue, 22 Mar 2022 13:02:23 +0900

 Thanks for your quick fix.
 Your fix works fine for me.

 -- 
 Ryo ONODERA // ryo@tetera.org
 PGP fingerprint = 82A2 DC91 76E0 A10A 8ABB  FD1B F404 27FA C7D1 15F3

State-Changed-From-To: feedback->closed
State-Changed-By: riastradh@NetBSD.org
State-Changed-When: Tue, 22 Mar 2022 09:47:06 +0000
State-Changed-Why:
fixed


>Unformatted:

Home	PR Database Search