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

drivers/net/usb/gobi/qmidevice.c

Index: drivers/net/usb/gobi/qmidevice.c
diff --git a/drivers/net/usb/gobi/qmidevice.c b/drivers/net/usb/gobi/qmidevice.c
index 82cb0fb3c1cad2227d14e146bf70c652639a459c..e7a87b5c5dfb9ab22d3685bbe8b83bc85ff2f140 100644
--- a/drivers/net/usb/gobi/qmidevice.c
+++ b/drivers/net/usb/gobi/qmidevice.c
@@ -736,7 +736,6 @@ static void client_free(struct qcusbnet *dev, u16 cid)
 			kfree(client);
 		}
 	}
-
 	spin_unlock_irqrestore(&dev->qmi.clients_lock, flags);
 }
 
@@ -950,10 +949,13 @@ static int devqmi_open(struct inode *inode, struct file *file)
 	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) {
@@ -963,7 +965,7 @@ static int devqmi_open(struct inode *inode, struct file *file)
 
 	handle = (struct qmihandle *)file->private_data;
 	handle->cid = (u16)-1;
-	handle->dev = dev;
+	handle->dev = ref;
 
 	DBG("%p %04x", handle, handle->cid);
 
@@ -984,6 +986,11 @@ static int devqmi_ioctl(struct inode *inode, struct file *file, unsigned int cmd
 		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;
@@ -1012,7 +1019,6 @@ static int devqmi_ioctl(struct inode *inode, struct file *file, unsigned int cmd
 		return 0;
 		break;
 
-
 	case IOCTL_QMI_GET_DEVICE_VIDPID:
 		if (!arg) {
 			DBG("Bad VIDPID buffer\n");
@@ -1109,7 +1115,7 @@ static int devqmi_close(struct file *file, fl_owner_t ftable)
 
 	if (handle->cid != (u16)-1)
 		client_free(handle->dev, handle->cid);
-
+	qcusbnet_put(handle->dev);
 	kfree(handle);
 	return 0;
 }
@@ -1127,6 +1133,11 @@ static ssize_t devqmi_read(struct file *file, char __user *buf, size_t size,
 		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;
@@ -1212,6 +1223,7 @@ int qc_register(struct qcusbnet *dev)
 	char *name;
 
 	dev->valid = true;
+	dev->dying = false;
 	result = client_alloc(dev, QMICTL);
 	if (result) {
 		dev->valid = false;
@@ -1279,13 +1291,6 @@ 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");
@@ -1300,33 +1305,6 @@ void qc_deregister(struct qcusbnet *dev)
 
 	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);