From 424d66cedae8bebb00fdb917fc8430f7b8a655cf Mon Sep 17 00:00:00 2001 From: Stefan Richter Date: Thu, 22 Jul 2010 11:56:38 +0200 Subject: [PATCH] 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 --- drivers/firewire/nosy.c | 111 +++++++++++++++++++++++++++++++++++------------- 1 file 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 #include #include +#include #include #include +#include #include #include #include /* 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); -- 2.7.4