USB: rndis_host, various cleanups

Cleanups to the rndis_host code, and a tweak that helps talking to
PXA hardware.  Mostly from Ole André Vadla Ravnås <oleavr@gmail.com>

  - Prevent SET_INTERFACE requests, they give PXA hardware bad indigestion
  - For paranoia, null a pointer after freeing its data
  - Wrap up ActiveSync oddities for RNDIS_QUERY in one routine
  - Use that wrapper when getting the Ethernet address
  - Whitespace fixes

Plus add a comment noting the open issues about some RNDIS clients still
needing TBD kinds of browbeating to accept non-jumbogram packets.

Signed-off-by: David Brownell <dbrownell@users.sourceforge.net>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>

1 files changed, 84 insertions, 28 deletions

diff --git a/drivers/usb/net/rndis_host.c b/drivers/usb/net/rndis_host.c
index 1d36772ba6e1..980e4aaa97aa 100644
--- a/drivers/usb/net/rndis_host.c
+++ b/drivers/usb/net/rndis_host.c
@@ -253,6 +253,7 @@ struct rndis_keepalive_c {	/* IN (optionally OUT) */
  * of that mess as possible.
  */
 #define OID_802_3_PERMANENT_ADDRESS	ccpu2(0x01010101)
+#define OID_GEN_MAXIMUM_FRAME_SIZE	ccpu2(0x00010106)
 #define OID_GEN_CURRENT_PACKET_FILTER	ccpu2(0x0001010e)
 
 /*
@@ -349,7 +350,7 @@ static int rndis_command(struct usbnet *dev, struct rndis_msg_hdr *buf)
 			case RNDIS_MSG_INDICATE: {	/* fault */
 				// struct rndis_indicate *msg = (void *)buf;
 				dev_info(&info->control->dev,
-					 "rndis fault indication\n");
+					"rndis fault indication\n");
 				}
 				break;
 			case RNDIS_MSG_KEEPALIVE: {	/* ping */
@@ -387,6 +388,71 @@ static int rndis_command(struct usbnet *dev, struct rndis_msg_hdr *buf)
 	return -ETIMEDOUT;
 }
 
+/*
+ * rndis_query:
+ *
+ * Performs a query for @oid along with 0 or more bytes of payload as
+ * specified by @in_len. If @reply_len is not set to -1 then the reply
+ * length is checked against this value, resulting in an error if it
+ * doesn't match.
+ *
+ * NOTE: Adding a payload exactly or greater than the size of the expected
+ * response payload is an evident requirement MSFT added for ActiveSync.
+ *
+ * The only exception is for OIDs that return a variably sized response,
+ * in which case no payload should be added.  This undocumented (and
+ * nonsensical!) issue was found by sniffing protocol requests from the
+ * ActiveSync 4.1 Windows driver.
+ */
+static int rndis_query(struct usbnet *dev, struct usb_interface *intf,
+		void *buf, u32 oid, u32 in_len,
+		void **reply, int *reply_len)
+{
+	int retval;
+	union {
+		void			*buf;
+		struct rndis_msg_hdr	*header;
+		struct rndis_query	*get;
+		struct rndis_query_c	*get_c;
+	} u;
+	u32 off, len;
+
+	u.buf = buf;
+
+	memset(u.get, 0, sizeof *u.get + in_len);
+	u.get->msg_type = RNDIS_MSG_QUERY;
+	u.get->msg_len = cpu_to_le32(sizeof *u.get + in_len);
+	u.get->oid = oid;
+	u.get->len = cpu_to_le32(in_len);
+	u.get->offset = ccpu2(20);
+
+	retval = rndis_command(dev, u.header);
+	if (unlikely(retval < 0)) {
+		dev_err(&intf->dev, "RNDIS_MSG_QUERY(0x%08x) failed, %d\n",
+				oid, retval);
+		return retval;
+	}
+
+	off = le32_to_cpu(u.get_c->offset);
+	len = le32_to_cpu(u.get_c->len);
+	if (unlikely((8 + off + len) > CONTROL_BUFFER_SIZE))
+		goto response_error;
+
+	if (*reply_len != -1 && len != *reply_len)
+		goto response_error;
+
+	*reply = (unsigned char *) &u.get_c->request_id + off;
+	*reply_len = len;
+
+	return retval;
+
+response_error:
+	dev_err(&intf->dev, "RNDIS_MSG_QUERY(0x%08x) "
+			"invalid response - off %d len %d\n",
+		oid, off, len);
+	return -EDOM;
+}
+
 static int rndis_bind(struct usbnet *dev, struct usb_interface *intf)
 {
 	int			retval;
@@ -403,6 +469,8 @@ static int rndis_bind(struct usbnet *dev, struct usb_interface *intf)
 		struct rndis_set_c	*set_c;
 	} u;
 	u32			tmp;
+	int			reply_len;
+	unsigned char		*bp;
 
 	/* we can't rely on i/o from stack working, or stack allocation */
 	u.buf = kmalloc(CONTROL_BUFFER_SIZE, GFP_KERNEL);
@@ -421,6 +489,12 @@ static int rndis_bind(struct usbnet *dev, struct usb_interface *intf)
 	 * TX we'll stick to one Ethernet packet plus RNDIS framing.
 	 * For RX we handle drivers that zero-pad to end-of-packet.
 	 * Don't let userspace change these settings.
+	 *
+	 * NOTE: there still seems to be wierdness here, as if we need
+	 * to do some more things to make sure WinCE targets accept this.
+	 * They default to jumbograms of 8KB or 16KB, which is absurd
+	 * for such low data rates and which is also more than Linux
+	 * can usually expect to allocate for SKB data...
 	 */
 	net->hard_header_len += sizeof (struct rndis_data_hdr);
 	dev->hard_mtu = net->mtu + net->hard_header_len;
@@ -434,7 +508,7 @@ static int rndis_bind(struct usbnet *dev, struct usb_interface *intf)
 	if (unlikely(retval < 0)) {
 		/* it might not even be an RNDIS device!! */
 		dev_err(&intf->dev, "RNDIS init failed, %d\n", retval);
- 		goto fail_and_release;
+		goto fail_and_release;
 	}
 	tmp = le32_to_cpu(u.init_c->max_transfer_size);
 	if (tmp < dev->hard_mtu) {
@@ -450,34 +524,15 @@ static int rndis_bind(struct usbnet *dev, struct usb_interface *intf)
 		dev->hard_mtu, tmp, dev->rx_urb_size,
 		1 << le32_to_cpu(u.init_c->packet_alignment));
 
-	/* Get designated host ethernet address.
-	 *
-	 * Adding a payload exactly the same size as the expected response
-	 * payload is an evident requirement MSFT added for ActiveSync.
-	 * This undocumented (and nonsensical) issue was found by sniffing
-	 * protocol requests from the ActiveSync 4.1 Windows driver.
-	 */
-	memset(u.get, 0, sizeof *u.get + 48);
-	u.get->msg_type = RNDIS_MSG_QUERY;
-	u.get->msg_len = ccpu2(sizeof *u.get + 48);
-	u.get->oid = OID_802_3_PERMANENT_ADDRESS;
-	u.get->len = ccpu2(48);
-	u.get->offset = ccpu2(20);
-
-	retval = rndis_command(dev, u.header);
-	if (unlikely(retval < 0)) {
+	/* Get designated host ethernet address */
+	reply_len = ETH_ALEN;
+	retval = rndis_query(dev, intf, u.buf, OID_802_3_PERMANENT_ADDRESS,
+			48, (void **) &bp, &reply_len);
+	if (unlikely(retval< 0)) {
 		dev_err(&intf->dev, "rndis get ethaddr, %d\n", retval);
 		goto fail_and_release;
 	}
-	tmp = le32_to_cpu(u.get_c->offset);
-	if (unlikely((tmp + 8) > (CONTROL_BUFFER_SIZE - ETH_ALEN)
-			|| u.get_c->len != ccpu2(ETH_ALEN))) {
-		dev_err(&intf->dev, "rndis ethaddr off %d len %d ?\n",
-			tmp, le32_to_cpu(u.get_c->len));
-		retval = -EDOM;
-		goto fail_and_release;
-	}
-	memcpy(net->dev_addr, tmp + (char *)&u.get_c->request_id, ETH_ALEN);
+	memcpy(net->dev_addr, bp, ETH_ALEN);
 
 	/* set a nonzero filter to enable data transfers */
 	memset(u.set, 0, sizeof *u.set);
@@ -502,6 +557,7 @@ static int rndis_bind(struct usbnet *dev, struct usb_interface *intf)
 fail_and_release:
 	usb_set_intfdata(info->data, NULL);
 	usb_driver_release_interface(driver_of(intf), info->data);
+	info->data = NULL;
 fail:
 	kfree(u.buf);
 	return retval;
@@ -618,7 +674,7 @@ fill:
 
 static const struct driver_info	rndis_info = {
 	.description =	"RNDIS device",
-	.flags =	FLAG_ETHER | FLAG_FRAMING_RN,
+	.flags =	FLAG_ETHER | FLAG_FRAMING_RN | FLAG_NO_SETINT,
 	.bind =		rndis_bind,
 	.unbind =	rndis_unbind,
 	.status =	rndis_status,