From 96b19062e741b715cf399312c30e0672d8889569 Mon Sep 17 00:00:00 2001 From: Stefan Richter Date: Sat, 2 Feb 2008 15:01:09 +0100 Subject: firewire: fix "kobject_add failed for fw* with -EEXIST" There is a race between shutdown and creation of devices: fw-core may attempt to add a device with the same name of an already existing device. http://bugzilla.kernel.org/show_bug.cgi?id=9828 Impact of the bug: Happens rarely (when shutdown of a device coincides with creation of another), forces the user to unplug and replug the new device to get it working. The fix is obvious: Free the minor number *after* instead of *before* device_unregister(). This requires to take an additional reference of the fw_device as long as the IDR tree points to it. And while we are at it, we fix an additional race condition: fw_device_op_open() took its reference of the fw_device a little bit too late, hence was in danger to access an already invalid fw_device. Signed-off-by: Stefan Richter --- drivers/firewire/fw-cdev.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) (limited to 'drivers/firewire/fw-cdev.c') diff --git a/drivers/firewire/fw-cdev.c b/drivers/firewire/fw-cdev.c index 7e73cbaa4121..44ccee26c368 100644 --- a/drivers/firewire/fw-cdev.c +++ b/drivers/firewire/fw-cdev.c @@ -109,15 +109,17 @@ static int fw_device_op_open(struct inode *inode, struct file *file) struct client *client; unsigned long flags; - device = fw_device_from_devt(inode->i_rdev); + device = fw_device_get_by_devt(inode->i_rdev); if (device == NULL) return -ENODEV; client = kzalloc(sizeof(*client), GFP_KERNEL); - if (client == NULL) + if (client == NULL) { + fw_device_put(device); return -ENOMEM; + } - client->device = fw_device_get(device); + client->device = device; INIT_LIST_HEAD(&client->event_list); INIT_LIST_HEAD(&client->resource_list); spin_lock_init(&client->lock); -- cgit v1.2.2 From fae603121428ba83b7343c88e68a7144525ab3eb Mon Sep 17 00:00:00 2001 From: Stefan Richter Date: Wed, 20 Feb 2008 21:10:06 +0100 Subject: firewire: fix NULL pointer deref. and resource leak By supplying ioctl()s in the wrong order, a userspace client was able to trigger NULL pointer dereferences. Furthermore, by calling ioctl_create_iso_context more than once, new contexts could be created without ever freeing the previously created contexts. Thanks to Anders Blomdell for the report. Signed-off-by: Stefan Richter --- drivers/firewire/fw-cdev.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) (limited to 'drivers/firewire/fw-cdev.c') diff --git a/drivers/firewire/fw-cdev.c b/drivers/firewire/fw-cdev.c index 44ccee26c368..46bc197a047f 100644 --- a/drivers/firewire/fw-cdev.c +++ b/drivers/firewire/fw-cdev.c @@ -646,6 +646,10 @@ static int ioctl_create_iso_context(struct client *client, void *buffer) struct fw_cdev_create_iso_context *request = buffer; struct fw_iso_context *context; + /* We only support one context at this time. */ + if (client->iso_context != NULL) + return -EBUSY; + if (request->channel > 63) return -EINVAL; @@ -792,8 +796,9 @@ static int ioctl_start_iso(struct client *client, void *buffer) { struct fw_cdev_start_iso *request = buffer; - if (request->handle != 0) + if (client->iso_context == NULL || request->handle != 0) return -EINVAL; + if (client->iso_context->type == FW_ISO_CONTEXT_RECEIVE) { if (request->tags == 0 || request->tags > 15) return -EINVAL; @@ -810,7 +815,7 @@ static int ioctl_stop_iso(struct client *client, void *buffer) { struct fw_cdev_stop_iso *request = buffer; - if (request->handle != 0) + if (client->iso_context == NULL || request->handle != 0) return -EINVAL; return fw_iso_context_stop(client->iso_context); -- cgit v1.2.2