Re: [PATCH RFC] block: fix use after free for bd_holder_dir/slave_dir

From: Christoph Hellwig
Date: Mon Oct 17 2022 - 03:59:27 EST


AFAICS the problem is the that pre-registered holders don't get
unregistered for a late add_disk failure.

Something like this should fix your error:

diff --git a/block/genhd.c b/block/genhd.c
index 17b33c62423df..6123005154b2a 100644
--- a/block/genhd.c
+++ b/block/genhd.c
@@ -484,7 +484,7 @@ int __must_check device_add_disk(struct device *parent, struct gendisk *disk,

ret = blk_register_queue(disk);
if (ret)
- goto out_put_slave_dir;
+ goto out_unregister_holders;

if (!(disk->flags & GENHD_FL_HIDDEN)) {
ret = bdi_register(disk->bdi, "%u:%u",
@@ -526,6 +526,8 @@ int __must_check device_add_disk(struct device *parent, struct gendisk *disk,
bdi_unregister(disk->bdi);
out_unregister_queue:
blk_unregister_queue(disk);
+out_unregister_holders:
+ bd_unregister_all_holders(disk);
out_put_slave_dir:
kobject_put(disk->slave_dir);
out_put_holder_dir:
diff --git a/block/holder.c b/block/holder.c
index 5283bc804cc14..12c09d5c21280 100644
--- a/block/holder.c
+++ b/block/holder.c
@@ -169,3 +169,13 @@ int bd_register_pending_holders(struct gendisk *disk)
mutex_unlock(&disk->open_mutex);
return ret;
}
+
+void bd_unregister_all_holders(struct gendisk *disk)
+{
+ struct bd_holder_disk *holder;
+
+ mutex_lock(&disk->open_mutex);
+ list_for_each_entry(holder, &disk->slave_bdevs, list)
+ __unlink_disk_holder(holder->bdev, disk);
+ mutex_unlock(&disk->open_mutex);
+}
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index 50e358a19d986..ccab9a2dae4bd 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -840,6 +840,7 @@ void set_capacity(struct gendisk *disk, sector_t size);
int bd_link_disk_holder(struct block_device *bdev, struct gendisk *disk);
void bd_unlink_disk_holder(struct block_device *bdev, struct gendisk *disk);
int bd_register_pending_holders(struct gendisk *disk);
+void bd_unregister_all_holders(struct gendisk *disk);
#else
static inline int bd_link_disk_holder(struct block_device *bdev,
struct gendisk *disk)
@@ -854,6 +855,9 @@ static inline int bd_register_pending_holders(struct gendisk *disk)
{
return 0;
}
+static inline void bd_unregister_all_holders(struct gendisk *disk)
+{
+}
#endif /* CONFIG_BLOCK_HOLDER_DEPRECATED */

dev_t part_devt(struct gendisk *disk, u8 partno);