diff options
author | Stefan Richter <stefanr@s5r6.in-berlin.de> | 2008-02-02 09:01:09 -0500 |
---|---|---|
committer | Stefan Richter <stefanr@s5r6.in-berlin.de> | 2008-02-16 09:40:33 -0500 |
commit | 96b19062e741b715cf399312c30e0672d8889569 (patch) | |
tree | b0e2197ab7380590bdff00e02dde19e81952a33f /drivers | |
parent | 1b9c12ba2fdf802a23630f70eddb0e821296634e (diff) |
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 <stefanr@s5r6.in-berlin.de>
Diffstat (limited to 'drivers')
-rw-r--r-- | drivers/firewire/fw-cdev.c | 8 | ||||
-rw-r--r-- | drivers/firewire/fw-device.c | 20 | ||||
-rw-r--r-- | drivers/firewire/fw-device.h | 2 |
3 files changed, 20 insertions, 10 deletions
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) | |||
109 | struct client *client; | 109 | struct client *client; |
110 | unsigned long flags; | 110 | unsigned long flags; |
111 | 111 | ||
112 | device = fw_device_from_devt(inode->i_rdev); | 112 | device = fw_device_get_by_devt(inode->i_rdev); |
113 | if (device == NULL) | 113 | if (device == NULL) |
114 | return -ENODEV; | 114 | return -ENODEV; |
115 | 115 | ||
116 | client = kzalloc(sizeof(*client), GFP_KERNEL); | 116 | client = kzalloc(sizeof(*client), GFP_KERNEL); |
117 | if (client == NULL) | 117 | if (client == NULL) { |
118 | fw_device_put(device); | ||
118 | return -ENOMEM; | 119 | return -ENOMEM; |
120 | } | ||
119 | 121 | ||
120 | client->device = fw_device_get(device); | 122 | client->device = device; |
121 | INIT_LIST_HEAD(&client->event_list); | 123 | INIT_LIST_HEAD(&client->event_list); |
122 | INIT_LIST_HEAD(&client->resource_list); | 124 | INIT_LIST_HEAD(&client->resource_list); |
123 | spin_lock_init(&client->lock); | 125 | spin_lock_init(&client->lock); |
diff --git a/drivers/firewire/fw-device.c b/drivers/firewire/fw-device.c index de9066e69adf..c04c28800f1d 100644 --- a/drivers/firewire/fw-device.c +++ b/drivers/firewire/fw-device.c | |||
@@ -610,12 +610,14 @@ static DECLARE_RWSEM(idr_rwsem); | |||
610 | static DEFINE_IDR(fw_device_idr); | 610 | static DEFINE_IDR(fw_device_idr); |
611 | int fw_cdev_major; | 611 | int fw_cdev_major; |
612 | 612 | ||
613 | struct fw_device *fw_device_from_devt(dev_t devt) | 613 | struct fw_device *fw_device_get_by_devt(dev_t devt) |
614 | { | 614 | { |
615 | struct fw_device *device; | 615 | struct fw_device *device; |
616 | 616 | ||
617 | down_read(&idr_rwsem); | 617 | down_read(&idr_rwsem); |
618 | device = idr_find(&fw_device_idr, MINOR(devt)); | 618 | device = idr_find(&fw_device_idr, MINOR(devt)); |
619 | if (device) | ||
620 | fw_device_get(device); | ||
619 | up_read(&idr_rwsem); | 621 | up_read(&idr_rwsem); |
620 | 622 | ||
621 | return device; | 623 | return device; |
@@ -627,13 +629,14 @@ static void fw_device_shutdown(struct work_struct *work) | |||
627 | container_of(work, struct fw_device, work.work); | 629 | container_of(work, struct fw_device, work.work); |
628 | int minor = MINOR(device->device.devt); | 630 | int minor = MINOR(device->device.devt); |
629 | 631 | ||
630 | down_write(&idr_rwsem); | ||
631 | idr_remove(&fw_device_idr, minor); | ||
632 | up_write(&idr_rwsem); | ||
633 | |||
634 | fw_device_cdev_remove(device); | 632 | fw_device_cdev_remove(device); |
635 | device_for_each_child(&device->device, NULL, shutdown_unit); | 633 | device_for_each_child(&device->device, NULL, shutdown_unit); |
636 | device_unregister(&device->device); | 634 | device_unregister(&device->device); |
635 | |||
636 | down_write(&idr_rwsem); | ||
637 | idr_remove(&fw_device_idr, minor); | ||
638 | up_write(&idr_rwsem); | ||
639 | fw_device_put(device); | ||
637 | } | 640 | } |
638 | 641 | ||
639 | static struct device_type fw_device_type = { | 642 | static struct device_type fw_device_type = { |
@@ -682,10 +685,13 @@ static void fw_device_init(struct work_struct *work) | |||
682 | } | 685 | } |
683 | 686 | ||
684 | err = -ENOMEM; | 687 | err = -ENOMEM; |
688 | |||
689 | fw_device_get(device); | ||
685 | down_write(&idr_rwsem); | 690 | down_write(&idr_rwsem); |
686 | if (idr_pre_get(&fw_device_idr, GFP_KERNEL)) | 691 | if (idr_pre_get(&fw_device_idr, GFP_KERNEL)) |
687 | err = idr_get_new(&fw_device_idr, device, &minor); | 692 | err = idr_get_new(&fw_device_idr, device, &minor); |
688 | up_write(&idr_rwsem); | 693 | up_write(&idr_rwsem); |
694 | |||
689 | if (err < 0) | 695 | if (err < 0) |
690 | goto error; | 696 | goto error; |
691 | 697 | ||
@@ -741,7 +747,9 @@ static void fw_device_init(struct work_struct *work) | |||
741 | idr_remove(&fw_device_idr, minor); | 747 | idr_remove(&fw_device_idr, minor); |
742 | up_write(&idr_rwsem); | 748 | up_write(&idr_rwsem); |
743 | error: | 749 | error: |
744 | put_device(&device->device); | 750 | fw_device_put(device); /* fw_device_idr's reference */ |
751 | |||
752 | put_device(&device->device); /* our reference */ | ||
745 | } | 753 | } |
746 | 754 | ||
747 | static int update_unit(struct device *dev, void *data) | 755 | static int update_unit(struct device *dev, void *data) |
diff --git a/drivers/firewire/fw-device.h b/drivers/firewire/fw-device.h index 0854fe2bc110..43808c02793e 100644 --- a/drivers/firewire/fw-device.h +++ b/drivers/firewire/fw-device.h | |||
@@ -77,13 +77,13 @@ fw_device_is_shutdown(struct fw_device *device) | |||
77 | } | 77 | } |
78 | 78 | ||
79 | struct fw_device *fw_device_get(struct fw_device *device); | 79 | struct fw_device *fw_device_get(struct fw_device *device); |
80 | struct fw_device *fw_device_get_by_devt(dev_t devt); | ||
80 | void fw_device_put(struct fw_device *device); | 81 | void fw_device_put(struct fw_device *device); |
81 | int fw_device_enable_phys_dma(struct fw_device *device); | 82 | int fw_device_enable_phys_dma(struct fw_device *device); |
82 | 83 | ||
83 | void fw_device_cdev_update(struct fw_device *device); | 84 | void fw_device_cdev_update(struct fw_device *device); |
84 | void fw_device_cdev_remove(struct fw_device *device); | 85 | void fw_device_cdev_remove(struct fw_device *device); |
85 | 86 | ||
86 | struct fw_device *fw_device_from_devt(dev_t devt); | ||
87 | extern int fw_cdev_major; | 87 | extern int fw_cdev_major; |
88 | 88 | ||
89 | struct fw_unit { | 89 | struct fw_unit { |