netdev - [RFC] net: cdc_ncm: workaround NTB input size firmware bug

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <87mwziu21c.fsf_-_@nemi.mork.no>
Date:	Fri, 19 Oct 2012 13:36:15 +0200
From:	Bjørn Mork <bjorn@...k.no>
To:	Alexey Orishko <alexey.orishko@...il.com>
Cc:	Oliver Neukum <oliver@...kum.org>, netdev@...r.kernel.org,
	linux-usb@...r.kernel.org,
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	Greg Suarez <gpsuarez2512@...il.com>,
	"Fangxiaozhi \(Franko\)" <fangxiaozhi@...wei.com>,
	Dan Williams <dcbw@...hat.com>,
	Aleksander Morgado <aleksander@...edo.com>
Subject: [RFC] net: cdc_ncm: workaround NTB input size firmware bug

Some devices do not support the 8 byte variants of
the NTB input size control messages despite announcing
such support in the functional descriptor. This can be
detected by reading the current input size, looking
at how many bytes the device returns.

Signed-off-by: Bjørn Mork <bjorn@...k.no>
---
Bjørn Mork <bjorn@...k.no> writes:

> I'll cook up an alternative patch implementing this so you can evaluate
> the impact.

So how about something like this?

I couldn't help myself not combining the two paths here while at it.
But I have intentionally not done the other obvious cleanups, like using
the standard timeout constants and using dev_xxx for instead of pr_xxx,
because they must and should be done separately over the whole driver.

This works for me:

Oct 19 13:19:43 nemi kernel: [304987.350138] dwNtbInMaxSize=131072 dwNtbOutMaxSize=32768 wNdpOutPayloadRemainder=0 wNdpOutDivisor=4 wNdpOutAlignment=4 wNtbOutMaxDatagrams=0 flags=0x20
Oct 19 13:19:43 nemi kernel: [304987.350144] Using default maximum receive length=32768
Oct 19 13:19:43 nemi kernel: [304987.350507] firmware bug: flags=0x20, but USB_CDC_GET_NTB_INPUT_SIZE returned 4 bytes


Bjørn

 drivers/net/usb/cdc_ncm.c |   64 +++++++++++++++++++++++----------------------
 1 file changed, 33 insertions(+), 31 deletions(-)

diff --git a/drivers/net/usb/cdc_ncm.c b/drivers/net/usb/cdc_ncm.c
index 4cd582a..26d31d7 100644
--- a/drivers/net/usb/cdc_ncm.c
+++ b/drivers/net/usb/cdc_ncm.c
@@ -215,44 +215,46 @@ static u8 cdc_ncm_setup(struct cdc_ncm_ctx *ctx)
 
 	/* inform device about NTB input size changes */
 	if (ctx->rx_max != le32_to_cpu(ctx->ncm_parm.dwNtbInMaxSize)) {
+		struct usb_cdc_ncm_ndp_input_size *ndp_in_sz;
+		size_t request_len = 4;
 
+		ndp_in_sz = kzalloc(sizeof(*ndp_in_sz), GFP_KERNEL);
+		if (!ndp_in_sz) {
+			err = -ENOMEM;
+			goto size_err;
+		}
 		if (flags & USB_CDC_NCM_NCAP_NTB_INPUT_SIZE) {
-			struct usb_cdc_ncm_ndp_input_size *ndp_in_sz;
-
-			ndp_in_sz = kzalloc(sizeof(*ndp_in_sz), GFP_KERNEL);
-			if (!ndp_in_sz) {
-				err = -ENOMEM;
-				goto size_err;
-			}
-
 			err = usb_control_msg(ctx->udev,
-					usb_sndctrlpipe(ctx->udev, 0),
-					USB_CDC_SET_NTB_INPUT_SIZE,
-					USB_TYPE_CLASS | USB_DIR_OUT
-					 | USB_RECIP_INTERFACE,
-					0, iface_no, ndp_in_sz, 8, 1000);
-			kfree(ndp_in_sz);
-		} else {
-			__le32 *dwNtbInMaxSize;
-			dwNtbInMaxSize = kzalloc(sizeof(*dwNtbInMaxSize),
-					GFP_KERNEL);
-			if (!dwNtbInMaxSize) {
-				err = -ENOMEM;
-				goto size_err;
+					      usb_rcvctrlpipe(ctx->udev, 0),
+					      USB_CDC_GET_NTB_INPUT_SIZE,
+					      USB_TYPE_CLASS | USB_DIR_OUT
+					      | USB_RECIP_INTERFACE,
+					      0, iface_no, ndp_in_sz, 8, 1000);
+			switch (err) {
+			case 4:
+				pr_debug("firmware bug: flags=0x%02x, but USB_CDC_GET_NTB_INPUT_SIZE returned 4 bytes\n", flags);
+				break;
+			case 8:
+				/* bmNetworkCapabilities is correct */
+				request_len = 8;
+				break;
+			default:
+				goto size_err_free;
 			}
-			*dwNtbInMaxSize = cpu_to_le32(ctx->rx_max);
-
-			err = usb_control_msg(ctx->udev,
-					usb_sndctrlpipe(ctx->udev, 0),
-					USB_CDC_SET_NTB_INPUT_SIZE,
-					USB_TYPE_CLASS | USB_DIR_OUT
-					 | USB_RECIP_INTERFACE,
-					0, iface_no, dwNtbInMaxSize, 4, 1000);
-			kfree(dwNtbInMaxSize);
 		}
+		ndp_in_sz->dwNtbInMaxSize = cpu_to_le32(ctx->rx_max);
+		ndp_in_sz->wNtbInMaxDatagrams = 0;	/* unlimited */
+		err = usb_control_msg(ctx->udev,
+				      usb_sndctrlpipe(ctx->udev, 0),
+				      USB_CDC_SET_NTB_INPUT_SIZE,
+				      USB_TYPE_CLASS | USB_DIR_OUT
+				      | USB_RECIP_INTERFACE,
+				      0, iface_no, ndp_in_sz, request_len, 1000);
+size_err_free:
+		kfree(ndp_in_sz);
 size_err:
 		if (err < 0)
-			pr_debug("Setting NTB Input Size failed\n");
+			pr_debug("Setting NTB Input Size failed: %d\n", err);
 	}
 
 	/* verify maximum size of transmitted NTB in bytes */
-- 
1.7.10.4

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html