From: Pete Zaitcev <zaitcev@redhat.com> Date: Mon, 10 Dec 2007 21:35:31 -0800 Subject: [usb] fix for error path in rndis Message-id: 20071210213531.0c37bc29.zaitcev@redhat.com O-Subject: [RHEL 5.2] Patch for error path in rndis (236719) Bugzilla: 236719 The explanation is going to be unusually long, so if someone is not particularly interested, please skip to the clause "Testing". https://bugzilla.redhat.com/show_bug.cgi?id=236719 Story ===== Nokia ships a number of phones which support so-called "RNDIS", which stands for "Remote NDIS". Roughly, it's Microsoft NIC driver API over USB. If you plug such a phone into a RHEL-5 box, the system oopses. This happens because the phone has two modes: a Hayes modem and RNDIS. If Hayes mode is in effect, the phone advertises that it's RNDIS capable through proper USB descriptors, but an attempt to initialize RNDIS ends with an error (which is fine). The system oopses when usbnet and its subdriver rndis_host finish the failed ->probe method. In most USB drivers, driver itself is not involved into the binding to an interface (USB drivers bind to interfaces, not devices; this way you can have a keyboard with an SD slot served by both hid and usb-storage simultaneously). Framework binds, then calls ->probe, then unbinds in case of an error. However, RNDIS requires a driver to attach to two interfaces. The rndis_host looks to which interface it got bound by the framework, and binds to the other one explicitly. When subsequent initia- lization fails, it has to unbind explicitly. Unfortunately, in normal circumstances unbinding disconnects a driver, and so the ->disconnect method is invoked. In our case, it leads to ->disconnect called from ->probe (through an unbind). I thought it would be neater to find a way for an unbind operation not to invoke the disconnect at all, but it seems too involved. So, I went with the upstream way of doing it. The disconnect checks if its interface has a private structure attached, and if it's not, it quietly exits. Thus, the main part of the fix is usb_set_intfdata(data_intf, NULL). You can see that the old code had usb_driver_release_interface near the label "fail:", but it was not enough, in fact it caused an oops. Testing ======= I built the patch on a branch. The customer tested the branch RPM. Upstream ======== This is the upstream backport, although I only took one specific fix. Please ack. -- Pete Acked-by: "David S. Miller" <davem@redhat.com> Acked-by: Prarit Bhargava <prarit@redhat.com> Acked-by: Alan Cox <alan@redhat.com> diff --git a/drivers/usb/net/rndis_host.c b/drivers/usb/net/rndis_host.c index c2a28d8..48d8ed3 100644 --- a/drivers/usb/net/rndis_host.c +++ b/drivers/usb/net/rndis_host.c @@ -379,6 +379,7 @@ static int rndis_bind(struct usbnet *dev, struct usb_interface *intf) { int retval; struct net_device *net = dev->net; + struct cdc_state *info = (void *) &dev->data; union { void *buf; struct rndis_msg_hdr *header; @@ -397,7 +398,7 @@ static int rndis_bind(struct usbnet *dev, struct usb_interface *intf) return -ENOMEM; retval = usbnet_generic_cdc_bind(dev, intf); if (retval < 0) - goto done; + goto fail; net->hard_header_len += sizeof (struct rndis_data_hdr); @@ -412,10 +413,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); -fail: - usb_driver_release_interface(driver_of(intf), - ((struct cdc_state *)&(dev->data))->data); - goto done; + goto fail_and_release; } dev->hard_mtu = le32_to_cpu(u.init_c->max_transfer_size); /* REVISIT: peripheral "alignment" request is ignored ... */ @@ -431,7 +429,7 @@ fail: retval = rndis_command(dev, u.header); if (unlikely(retval < 0)) { dev_err(&intf->dev, "rndis get ethaddr, %d\n", retval); - goto fail; + goto fail_and_release; } tmp = le32_to_cpu(u.get_c->offset); if (unlikely((tmp + 8) > (1024 - ETH_ALEN) @@ -439,7 +437,7 @@ fail: dev_err(&intf->dev, "rndis ethaddr off %d len %d ?\n", tmp, le32_to_cpu(u.get_c->len)); retval = -EDOM; - goto fail; + goto fail_and_release; } memcpy(net->dev_addr, tmp + (char *)&u.get_c->request_id, ETH_ALEN); @@ -455,11 +453,16 @@ fail: retval = rndis_command(dev, u.header); if (unlikely(retval < 0)) { dev_err(&intf->dev, "rndis set packet filter, %d\n", retval); - goto fail; + goto fail_and_release; } - retval = 0; -done: + kfree(u.buf); + return 0; + +fail_and_release: + usb_set_intfdata(info->data, NULL); + usb_driver_release_interface(driver_of(intf), info->data); +fail: kfree(u.buf); return retval; }