CHROMIUM: gobi: Fix races in qc_deregister() once and for all.

drivers/net/usb/gobi/qcusbnet.c

Index: drivers/net/usb/gobi/qcusbnet.c
diff --git a/drivers/net/usb/gobi/qcusbnet.c b/drivers/net/usb/gobi/qcusbnet.c
index 7e218a81fe3217e7a7595ceabd2aaabc251b3da6..240b5a9a25c88dad4333eba8cb6dcd29c5ec80d8 100644
--- a/drivers/net/usb/gobi/qcusbnet.c
+++ b/drivers/net/usb/gobi/qcusbnet.c
@@ -25,9 +25,47 @@
 #define DRIVER_AUTHOR "Qualcomm Innovation Center"
 #define DRIVER_DESC "QCUSBNet2k"
 
+static LIST_HEAD(qcusbnet_list);
+static DEFINE_MUTEX(qcusbnet_lock);
+
 int qcusbnet_debug;
 static struct class *devclass;
 
+static void free_dev(struct kref *ref)
+{
+	struct qcusbnet *dev = container_of(ref, struct qcusbnet, refcount);
+	list_del(&dev->node);
+	kfree(dev);
+}
+
+void qcusbnet_put(struct qcusbnet *dev)
+{
+	mutex_lock(&qcusbnet_lock);
+	kref_put(&dev->refcount, free_dev);
+	mutex_unlock(&qcusbnet_lock);
+}
+
+struct qcusbnet *qcusbnet_get(struct qcusbnet *key)
+{
+	/* Given a putative qcusbnet struct, return either the struct itself
+	 * (with a ref taken) if the struct is still visible, or NULL if it's
+	 * not. This prevents object-visibility races where someone is looking
+	 * up an object as the last ref gets dropped; dropping the last ref and
+	 * removing the object from the list are atomic with respect to getting
+	 * a new ref. */
+	struct qcusbnet *entry = NULL;
+	mutex_lock(&qcusbnet_lock);
+	list_for_each_entry(entry, &qcusbnet_list, node) {
+		if (entry == key) {
+			kref_get(&entry->refcount);
+			mutex_unlock(&qcusbnet_lock);
+			return entry;
+		}
+	}
+	mutex_unlock(&qcusbnet_lock);
+	return NULL;
+}
+
 int qc_suspend(struct usb_interface *iface, pm_message_t event)
 {
 	struct usbnet *usbnet;
@@ -180,12 +218,14 @@ static void qcnet_unbind(struct usbnet *usbnet, struct usb_interface *iface)
 	struct qcusbnet *dev = (struct qcusbnet *)usbnet->data[0];
 
 	netif_carrier_off(usbnet->net);
+	/* See the comments in qcusbnet_get() for an explanation of why we need

[Jason Glasgow, 2011/03/10 18:36:28]

comment should disappear.

[Elly Fong-Jones, 2011/03/10 19:15:46]

Done.

+	 * this lock. */
 	qc_deregister(dev);
 
-	kfree(usbnet->net->netdev_ops);
 	usbnet->net->netdev_ops = NULL;

[Jason Glasgow, 2011/03/10 18:36:28]

still wrong.

[Elly Fong-Jones, 2011/03/10 19:15:46]

Done.

-
-	kfree(dev);
+	kfree(usbnet->net->netdev_ops);
+	/* drop the list's ref */
+	qcusbnet_put(dev);
 }
 
 static void qcnet_urbhook(struct urb *urb)
@@ -590,10 +630,12 @@ int qcnet_probe(struct usb_interface *iface, const struct usb_device_id *vidpids
 	DBG("Mac Address: %pM\n", dev->usbnet->net->dev_addr);
 
 	dev->valid = false;
-	memset(&dev->qmi, 0, sizeof(struct qmidev));
+	memset(&dev->qmi, 0, sizeof(dev->qmi));
 
 	dev->qmi.devclass = devclass;
 
+	kref_init(&dev->refcount);
+	INIT_LIST_HEAD(&dev->node);
 	INIT_LIST_HEAD(&dev->qmi.clients);
 	init_completion(&dev->worker.work);
 	spin_lock_init(&dev->qmi.clients_lock);
@@ -605,6 +647,11 @@ int qcnet_probe(struct usb_interface *iface, const struct usb_device_id *vidpids
 	status = qc_register(dev);
 	if (status) {
 		qc_deregister(dev);
+	} else {
+		mutex_lock(&qcusbnet_lock);
+		/* Give our initial ref to the list */
+		list_add(&dev->node, &qcusbnet_list);
+		mutex_unlock(&qcusbnet_lock);
 	}
 
 	return status;