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>

diff --git a/drivers/firewire/nosy.c b/drivers/firewire/nosy.c
index ccf9c461bd8636bcdfe3d4e4d1773b36a9ebe111..edd729aafecaf897c06615d8e9fab56ca77ca520 100644 (file)
--- 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);