Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 424d66ce authored by Stefan Richter's avatar Stefan Richter
Browse files

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: default avatarStefan Richter <stefanr@s5r6.in-berlin.de>
parent b6d9c125
Loading
Loading
Loading
Loading
+82 −29
Original line number Diff line number Diff line
@@ -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;
	struct pcilynx *tmp, *lynx = NULL;

	if (minor > MAX_MINORS || minors[minor] == 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);