From 0c29b3120507c139950c15caea44d1c9c616c955 Mon Sep 17 00:00:00 2001 From: Augustin Cavalier Date: Tue, 21 Jul 2026 18:20:15 -0400 Subject: [PATCH] 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 --- .../drivers/disk/usb/usb_disk/usb_disk.cpp | 51 +++++++++++-------- 1 file changed, 30 insertions(+), 21 deletions(-) diff --git a/src/add-ons/kernel/drivers/disk/usb/usb_disk/usb_disk.cpp b/src/add-ons/kernel/drivers/disk/usb/usb_disk/usb_disk.cpp index 01ec281efa..2db004af09 100644 --- a/src/add-ons/kernel/drivers/disk/usb/usb_disk/usb_disk.cpp +++ b/src/add-ons/kernel/drivers/disk/usb/usb_disk/usb_disk.cpp @@ -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[] = {