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

drivers/net/usb/gobi/qmidevice.c

 /* qmidevice.c - gobi QMI device
  * Copyright (c) 2010, Code Aurora Forum. All rights reserved.
 
  * This program is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License version 2 and
  * only version 2 as published by the Free Software Foundation.
 
  * This program is distributed in the hope that it will be useful,
  * but WITHOUT ANY WARRANTY; without even the implied warranty of
  * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
@@ skipping 718 matching lines @@
                                 urb = client_delurb(dev, cid);
                         }
 
                         while (client_delread(dev, cid, 0, &data, &size))
                                 kfree(data);
 
                         list_del(&client->node);
                         kfree(client);
                 }
         }
-
         spin_unlock_irqrestore(&dev->qmi.clients_lock, flags);
 }
 
 struct client *client_bycid(struct qcusbnet *dev, u16 cid)
 {
         struct list_head *node;
         struct client *client;
 
         if (!device_valid(dev)) {
                 DBG("Invalid device\n");
@@ skipping 193 matching lines @@
         kfree(req);
         return urb;
 }
 
 static int devqmi_open(struct inode *inode, struct file *file)
 {
         struct qmihandle *handle;
         struct qmidev *qmidev = container_of(inode->i_cdev, struct qmidev, cdev);
         struct qcusbnet *dev = container_of(qmidev, struct qcusbnet, qmi);
 
-        if (!device_valid(dev)) {
-                DBG("Invalid device\n");
+        /* We need an extra ref on the device per fd, since we stash a ref
+         * inside the handle. If qcusbnet_get() returns NULL, that means the
+         * device has been removed from the list - no new refs for us. */
+        struct qcusbnet *ref = qcusbnet_get(dev);
+
+        if (!ref)
                 return -ENXIO;
-        }
 
         file->private_data = kmalloc(sizeof(struct qmihandle), GFP_KERNEL);
         if (!file->private_data) {
                 DBG("Mem error\n");
                 return -ENOMEM;
         }
 
         handle = (struct qmihandle *)file->private_data;
         handle->cid = (u16)-1;
-        handle->dev = dev;
+        handle->dev = ref;
 
         DBG("%p %04x", handle, handle->cid);
 
         return 0;
 }
 
 static int devqmi_ioctl(struct inode *inode, struct file *file, unsigned int cmd, unsigned long arg)
 {
         int result;
         u32 vidpid;
 
         struct qmihandle *handle = (struct qmihandle *)file->private_data;
 
         DBG("%p %04x %08x", handle, handle->cid, cmd);
 
         if (!handle) {
                 DBG("Bad file data\n");
                 return -EBADF;
         }
 
+        if (handle->dev->dying) {

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

Dying never set to true.

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

Oops. Supposed to be done early in qc_deregister(); fixed.

+                DBG("Dying device");
+                return -ENXIO;
+        }
+
         if (!device_valid(handle->dev)) {
                 DBG("Invalid device! Updating f_ops\n");
                 file->f_op = file->f_dentry->d_inode->i_fop;
                 return -ENXIO;
         }
 
         switch (cmd) {
         case IOCTL_QMI_GET_SERVICE_FILE:
 
                 DBG("Setting up QMI for service %lu\n", arg);
                 if (!(u8)arg) {
                         DBG("Cannot use QMICTL from userspace\n");
                         return -EINVAL;
                 }
 
                 if (handle->cid != (u16)-1) {
                         DBG("Close the current connection before opening a new one\n");
                         return -EBADR;
                 }
 
                 result = client_alloc(handle->dev, (u8)arg);
                 if (result < 0)
                         return result;
                 handle->cid = result;
 
                 return 0;
                 break;
 
-
         case IOCTL_QMI_GET_DEVICE_VIDPID:
                 if (!arg) {
                         DBG("Bad VIDPID buffer\n");
                         return -EINVAL;
                 }
 
                 if (!handle->dev->usbnet) {
                         DBG("Bad usbnet\n");
                         return -ENOMEM;
                 }
@@ skipping 76 matching lines @@
                 file->f_op = file->f_dentry->d_inode->i_fop;
                 return -ENXIO;
         }
 
         DBG("0x%04X\n", handle->cid);
 
         file->private_data = NULL;
 
         if (handle->cid != (u16)-1)
                 client_free(handle->dev, handle->cid);
-
+        qcusbnet_put(handle->dev);
         kfree(handle);
         return 0;
 }
 
 static ssize_t devqmi_read(struct file *file, char __user *buf, size_t size,
                            loff_t *pos)
 {
         int result;
         void *data = NULL;
         void *smalldata;
         struct qmihandle *handle = (struct qmihandle *)file->private_data;
 
         if (!handle) {
                 DBG("Bad file data\n");
                 return -EBADF;
         }
 
+        if (handle->dev->dying) {
+                DBG("Dying device");
+                return -ENXIO;
+        }
+
         if (!device_valid(handle->dev)) {
                 DBG("Invalid device! Updating f_ops\n");
                 file->f_op = file->f_dentry->d_inode->i_fop;
                 return -ENXIO;
         }
 
         if (handle->cid == (u16)-1) {
                 DBG("Client ID must be set before reading 0x%04X\n",
                     handle->cid);
                 return -EBADR;
@@ skipping 65 matching lines @@
 }
 
 int qc_register(struct qcusbnet *dev)
 {
         int result;
         int qmiidx = 0;
         dev_t devno;
         char *name;
 
         dev->valid = true;
+        dev->dying = false;
         result = client_alloc(dev, QMICTL);
         if (result) {
                 dev->valid = false;
                 return result;
         }
         atomic_set(&dev->qmi.qmitid, 1);
 
         result = qc_startread(dev);
         if (result) {
                 dev->valid = false;
@@ skipping 47 matching lines @@
         device_create(dev->qmi.devclass, NULL, devno, NULL, "qcqmi%d", qmiidx);
 
         dev->qmi.devnum = devno;
         return 0;
 }
 
 void qc_deregister(struct qcusbnet *dev)
 {
         struct list_head *node, *tmp;
         struct client *client;
-        struct inode *inode;
-        struct list_head *inodes;
-        struct task_struct *task;
-        struct fdtable *fdtable;
-        struct file *file;
-        unsigned long flags;
-        int count = 0;
 
         if (!device_valid(dev)) {
                 DBG("wrong device\n");
                 return;
         }
 
         list_for_each_safe(node, tmp, &dev->qmi.clients) {
                 client = list_entry(node, struct client, node);
                 DBG("release 0x%04X\n", client->cid);
                 client_free(dev, client->cid);
         }
 
         qc_stopread(dev);
         dev->valid = false;
-        list_for_each(inodes, &dev->qmi.cdev.list) {
-                inode = container_of(inodes, struct inode, i_devices);
-                if (inode != NULL && !IS_ERR(inode)) {
-                        rcu_read_lock();
-                        for_each_process(task) {
-                                if (!task || !task->files)
-                                        continue;
-                                spin_lock_irqsave(&task->files->file_lock, flags);
-                                fdtable = files_fdtable(task->files);
-                                for (count = 0; count < fdtable->max_fds; count++) {
-                                        file = fdtable->fd[count];
-                                        if (file != NULL &&  file->f_dentry != NULL) {
-                                                if (file->f_dentry->d_inode == inode) {
-                                                        rcu_assign_pointer(fdtable->fd[count], NULL);
-                                                        spin_unlock_irqrestore(&task->files->file_lock, flags);
-                                                        DBG("forcing close of open file handle\n");
-                                                        filp_close(file, task->files);
-                                                        spin_lock_irqsave(&task->files->file_lock, flags);
-                                                }
-                                        }
-                                }
-                                spin_unlock_irqrestore(&task->files->file_lock, flags);
-                        }
-                        rcu_read_unlock();
-                }
-        }
-
         if (!IS_ERR(dev->qmi.devclass))
                 device_destroy(dev->qmi.devclass, dev->qmi.devnum);
         cdev_del(&dev->qmi.cdev);
         unregister_chrdev_region(dev->qmi.devnum, 1);
 }
 
 static bool qmi_ready(struct qcusbnet *dev, u16 timeout)
 {
         int result;
         void *wbuf;
@@ skipping 243 matching lines @@
                 DBG("bad get MEID resp\n");
                 memset(&dev->meid[0], '0', 14);
         }
 
         client_free(dev, cid);
         return 0;
 }
 
 module_param(qcusbnet2k_fwdelay, int, S_IRUGO | S_IWUSR);
 MODULE_PARM_DESC(qcusbnet2k_fwdelay, "Delay for old firmware");