firewire: nosy: fix device shutdown with active client

Fix race between nosy_open() and remove_card() by replacing the
unprotected array of card pointers by a mutex-protected list of cards.

Make card instances reference-counted and let each client hold a
reference.

Notify clients about card removal via POLLHUP in poll()'s events
bitmap; also let read() fail with errno=ENODEV if the card was removed
and everything in the buffer was read.

Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>

1 files changed, 82 insertions, 29 deletions

diff --git a/drivers/firewire/nosy.c b/drivers/firewire/nosy.c
index ccf9c46..edd729a 100644
--- a/drivers/firewire/nosy.c
+++ b/drivers/firewire/nosy.c
@@ -23,8 +23,10 @@
 #include <linux/interrupt.h>
 #include <linux/io.h>
 #include <linux/kernel.h>
+#include <linux/kref.h>
 #include <linux/miscdevice.h>
 #include <linux/module.h>
+#include <linux/mutex.h>
 #include <linux/pci.h>
 #include <linux/poll.h>
 #include <linux/sched.h> /* required for linux/wait.h */
@@ -104,8 +106,30 @@ struct pcilynx {
 	struct list_head client_list;
 
 	struct miscdevice misc;
+	struct list_head link;
+	struct kref kref;
 };
 
+static inline struct pcilynx *
+lynx_get(struct pcilynx *lynx)
+{
+	kref_get(&lynx->kref);
+
+	return lynx;
+}
+
+static void
+lynx_release(struct kref *kref)
+{
+	kfree(container_of(kref, struct pcilynx, kref));
+}
+
+static inline void
+lynx_put(struct pcilynx *lynx)
+{
+	kref_put(&lynx->kref, lynx_release);
+}
+
 struct client {
 	struct pcilynx *lynx;
 	u32 tcode_mask;
@@ -113,8 +137,8 @@ struct client {
 	struct list_head link;
 };
 
-#define MAX_MINORS 64
-static struct pcilynx *minors[MAX_MINORS];
+static DEFINE_MUTEX(card_mutex);
+static LIST_HEAD(card_list);
 
 static int
 packet_buffer_init(struct packet_buffer *buffer, size_t capacity)
@@ -139,15 +163,20 @@ packet_buffer_destroy(struct packet_buffer *buffer)
 }
 
 static int
-packet_buffer_get(struct packet_buffer *buffer, void *data, size_t user_length)
+packet_buffer_get(struct client *client, void *data, size_t user_length)
 {
+	struct packet_buffer *buffer = &client->buffer;
 	size_t length;
 	char *end;
 
 	if (wait_event_interruptible(buffer->wait,
-				     atomic_read(&buffer->size) > 0))
+				     atomic_read(&buffer->size) > 0) ||
+				     list_empty(&client->lynx->link))
 		return -ERESTARTSYS;
 
+	if (atomic_read(&buffer->size) == 0)
+		return -ENODEV;
+
 	/* FIXME: Check length <= user_length. */
 
 	end = buffer->data + buffer->capacity;
@@ -265,39 +294,52 @@ nosy_open(struct inode *inode, struct file *file)
 {
 	int minor = iminor(inode);
 	struct client *client;
-
-	if (minor > MAX_MINORS || minors[minor] == NULL)
+	struct pcilynx *tmp, *lynx = NULL;
+
+	mutex_lock(&card_mutex);
+	list_for_each_entry(tmp, &card_list, link)
+		if (tmp->misc.minor == minor) {
+			lynx = lynx_get(tmp);
+			break;
+		}
+	mutex_unlock(&card_mutex);
+	if (lynx == NULL)
 		return -ENODEV;
 
 	client = kmalloc(sizeof *client, GFP_KERNEL);
 	if (client == NULL)
-		return -ENOMEM;
+		goto fail;
 
 	client->tcode_mask = ~0;
-	client->lynx = minors[minor];
+	client->lynx = lynx;
 	INIT_LIST_HEAD(&client->link);
 
-	if (packet_buffer_init(&client->buffer, 128 * 1024) < 0) {
-		kfree(client);
-		return -ENOMEM;
-	}
+	if (packet_buffer_init(&client->buffer, 128 * 1024) < 0)
+		goto fail;
 
 	file->private_data = client;
 
 	return 0;
+fail:
+	kfree(client);
+	lynx_put(lynx);
+
+	return -ENOMEM;
 }
 
 static int
 nosy_release(struct inode *inode, struct file *file)
 {
 	struct client *client = file->private_data;
+	struct pcilynx *lynx = client->lynx;
 
-	spin_lock_irq(&client->lynx->client_list_lock);
+	spin_lock_irq(&lynx->client_list_lock);
 	list_del_init(&client->link);
-	spin_unlock_irq(&client->lynx->client_list_lock);
+	spin_unlock_irq(&lynx->client_list_lock);
 
 	packet_buffer_destroy(&client->buffer);
 	kfree(client);
+	lynx_put(lynx);
 
 	return 0;
 }
@@ -306,13 +348,17 @@ static unsigned int
 nosy_poll(struct file *file, poll_table *pt)
 {
 	struct client *client = file->private_data;
+	unsigned int ret = 0;
 
 	poll_wait(file, &client->buffer.wait, pt);
 
 	if (atomic_read(&client->buffer.size) > 0)
-		return POLLIN | POLLRDNORM;
-	else
-		return 0;
+		ret = POLLIN | POLLRDNORM;
+
+	if (list_empty(&client->lynx->link))
+		ret |= POLLHUP;
+
+	return ret;
 }
 
 static ssize_t
@@ -320,7 +366,7 @@ nosy_read(struct file *file, char *buffer, size_t count, loff_t *offset)
 {
 	struct client *client = file->private_data;
 
-	return packet_buffer_get(&client->buffer, buffer, count);
+	return packet_buffer_get(client, buffer, count);
 }
 
 static long
@@ -479,16 +525,22 @@ irq_handler(int irq, void *device)
 static void
 remove_card(struct pci_dev *dev)
 {
-	struct pcilynx *lynx;
+	struct pcilynx *lynx = pci_get_drvdata(dev);
+	struct client *client;
 
-	lynx = pci_get_drvdata(dev);
-	if (!lynx)
-		return;
-	pci_set_drvdata(dev, NULL);
+	mutex_lock(&card_mutex);
+	list_del_init(&lynx->link);
+	misc_deregister(&lynx->misc);
+	mutex_unlock(&card_mutex);
 
 	reg_write(lynx, PCI_INT_ENABLE, 0);
 	free_irq(lynx->pci_device->irq, lynx);
 
+	spin_lock_irq(&lynx->client_list_lock);
+	list_for_each_entry(client, &lynx->client_list, link)
+		wake_up_interruptible(&client->buffer.wait);
+	spin_unlock_irq(&lynx->client_list_lock);
+
 	pci_free_consistent(lynx->pci_device, sizeof(struct pcl),
 			    lynx->rcv_start_pcl, lynx->rcv_start_pcl_bus);
 	pci_free_consistent(lynx->pci_device, sizeof(struct pcl),
@@ -498,11 +550,7 @@ remove_card(struct pci_dev *dev)
 
 	iounmap(lynx->registers);
 	pci_disable_device(dev);
-
-	minors[lynx->misc.minor] = NULL;
-	misc_deregister(&lynx->misc);
-
-	kfree(lynx);
+	lynx_put(lynx);
 }
 
 #define RCV_BUFFER_SIZE (16 * 1024)
@@ -536,6 +584,7 @@ add_card(struct pci_dev *dev, const struct pci_device_id *unused)
 
 	spin_lock_init(&lynx->client_list_lock);
 	INIT_LIST_HEAD(&lynx->client_list);
+	kref_init(&lynx->kref);
 
 	lynx->registers = ioremap_nocache(pci_resource_start(dev, 0),
 					  PCILYNX_MAX_REGISTER);
@@ -619,12 +668,16 @@ add_card(struct pci_dev *dev, const struct pci_device_id *unused)
 	lynx->misc.minor = MISC_DYNAMIC_MINOR;
 	lynx->misc.name = "nosy";
 	lynx->misc.fops = &nosy_ops;
+
+	mutex_lock(&card_mutex);
 	ret = misc_register(&lynx->misc);
 	if (ret) {
 		error("Failed to register misc char device\n");
+		mutex_unlock(&card_mutex);
 		goto fail_free_irq;
 	}
-	minors[lynx->misc.minor] = lynx;
+	list_add_tail(&lynx->link, &card_list);
+	mutex_unlock(&card_mutex);
 
 	notify("Initialized PCILynx IEEE1394 card, irq=%d\n", dev->irq);