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 ccf9c461bd86..edd729aafeca 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);