From: David Brownell Date: Wed, 18 Apr 2007 00:53:20 +0000 (-0700) Subject: USB: rndis_host, various cleanups X-Git-Tag: v2.6.22-rc1~1117^2~22 X-Git-Url: https://err.no/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=ddda08624013e8435e9f7cfc34a35bd7b3520b6d;p=linux-2.6 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 - 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 Signed-off-by: Greg Kroah-Hartman --- diff --git a/drivers/usb/net/rndis_host.c b/drivers/usb/net/rndis_host.c index 1d36772ba6..980e4aaa97 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,