usb_disk: Unpublish devices in the driver removed hook.
We publish devices in the driver register hook, and if we try to unpublish them from the device removed hook, not the driver removed hook, we'll confuse the device_manager and cause it to do use-after-frees on list iteration. The two are called in quick succession of each other, so this shouldn't cause problems. Fixes a KDL reported by OscarL when unplugging a USB card reader (which I reproduced with another USB card reader.) It seems the problem would happen with any USB disk device that published more than one LUN. (cherry picked from commit a688b173e7c1e903845b6cac78baa7bd2ea23c31) Change-Id: Ia9fae3428a45e199cb2c903acfe9c689dfa9bed4 Reviewed-on: https://review.haiku-os.org/c/haiku/+/11305 Reviewed-by: waddlesplash <[email protected]>
This commit is contained in:
committed by
waddlesplash
parent
42371830dc
commit
0c29b31205
@@ -132,6 +132,7 @@ void
|
||||
usb_disk_free_device_and_luns(disk_device *device)
|
||||
{
|
||||
ASSERT_LOCKED_MUTEX(&device->lock);
|
||||
dprintf("usb_disk_free_device_and_luns\n");
|
||||
|
||||
for (uint8 i = 0; i < device->lun_count; i++) {
|
||||
delete device->luns[i]->io_scheduler;
|
||||
@@ -1256,12 +1257,8 @@ usb_disk_device_removed(void *cookie)
|
||||
disk_device *device = (disk_device *)cookie;
|
||||
mutex_lock(&device->lock);
|
||||
|
||||
for (uint8 i = 0; i < device->lun_count; i++) {
|
||||
// unpublish_device() can call close().
|
||||
mutex_unlock(&device->lock);
|
||||
gDeviceManager->unpublish_device(device->node, device->luns[i]->name);
|
||||
mutex_lock(&device->lock);
|
||||
}
|
||||
for (uint8 i = 0; i < device->lun_count; i++)
|
||||
device->luns[i]->media_present = false;
|
||||
|
||||
device->removed = true;
|
||||
gUSBModule->cancel_queued_transfers(device->bulk_in);
|
||||
@@ -1269,11 +1266,7 @@ usb_disk_device_removed(void *cookie)
|
||||
if (device->is_ufi)
|
||||
gUSBModule->cancel_queued_transfers(device->interrupt);
|
||||
|
||||
// At this point, open_count should always be 0 anyway.
|
||||
if (device->open_count == 0)
|
||||
usb_disk_free_device_and_luns(device);
|
||||
else
|
||||
mutex_unlock(&device->lock);
|
||||
mutex_unlock(&device->lock);
|
||||
}
|
||||
|
||||
|
||||
@@ -1503,16 +1496,10 @@ usb_disk_free(void *cookie)
|
||||
device_lun *lun = (device_lun *)cookie;
|
||||
disk_device *device = lun->device;
|
||||
mutex_lock(&device->lock);
|
||||
|
||||
device->open_count--;
|
||||
if (device->open_count == 0 && device->removed) {
|
||||
// we can simply free the device here as it has been removed from
|
||||
// the device list in the device removed notification hook
|
||||
usb_disk_free_device_and_luns(device);
|
||||
} else {
|
||||
mutex_unlock(&device->lock);
|
||||
}
|
||||
mutex_unlock(&device->lock);
|
||||
|
||||
// The device will actually be freed by uninit_driver.
|
||||
return B_OK;
|
||||
}
|
||||
|
||||
@@ -1956,7 +1943,11 @@ static void
|
||||
usb_disk_uninit_driver(void *_cookie)
|
||||
{
|
||||
CALLED();
|
||||
// Nothing to do.
|
||||
disk_device *device = (disk_device *)_cookie;
|
||||
mutex_lock(&device->lock);
|
||||
|
||||
ASSERT(device->open_count == 0 && device->removed);
|
||||
usb_disk_free_device_and_luns(device);
|
||||
}
|
||||
|
||||
|
||||
@@ -1981,6 +1972,24 @@ usb_disk_register_child_devices(void* _cookie)
|
||||
}
|
||||
|
||||
|
||||
static void
|
||||
usb_disk_removed(void* _cookie)
|
||||
{
|
||||
CALLED();
|
||||
disk_device *device = (disk_device *)_cookie;
|
||||
mutex_lock(&device->lock);
|
||||
|
||||
for (uint8 i = 0; i < device->lun_count; i++) {
|
||||
// unpublish_device() can call close().
|
||||
mutex_unlock(&device->lock);
|
||||
gDeviceManager->unpublish_device(device->node, device->luns[i]->name);
|
||||
mutex_lock(&device->lock);
|
||||
}
|
||||
|
||||
mutex_unlock(&device->lock);
|
||||
}
|
||||
|
||||
|
||||
// #pragma mark -
|
||||
|
||||
|
||||
@@ -2026,7 +2035,7 @@ struct driver_module_info sUsbDiskDriver = {
|
||||
usb_disk_uninit_driver,
|
||||
usb_disk_register_child_devices,
|
||||
NULL, // rescan
|
||||
NULL, // removed
|
||||
usb_disk_removed,
|
||||
};
|
||||
|
||||
module_info* modules[] = {
|
||||
|
||||
Reference in New Issue
Block a user