[PATCH RFC 2/2] block: protect slave_dir/bd_holder_dir by open_mutex

From: Yu Kuai
Date: Tue Oct 18 2022 - 08:52:50 EST


Lifecycle of slave_dir/bd_holder_dir is problematic currently:

t1: t2:

// get bdev of lower disk
blkdev_get_by_dev
// remove lower disk
del_gendisk
// initial reference is released, and
// slave_dir/bd_holder_dir can be freed
kobject_put
// uaf is triggered
bd_link_disk_holder

Fix the problem by protecting them by open_mutex.

Signed-off-by: Yu Kuai <yukuai3@xxxxxxxxxx>
---
block/genhd.c | 8 ++++++--
block/holder.c | 13 ++++++++++++-
block/partitions/core.c | 5 ++++-
3 files changed, 22 insertions(+), 4 deletions(-)

diff --git a/block/genhd.c b/block/genhd.c
index 17b33c62423d..d9ad889d011a 100644
--- a/block/genhd.c
+++ b/block/genhd.c
@@ -622,8 +622,12 @@ void del_gendisk(struct gendisk *disk)

blk_unregister_queue(disk);

- kobject_put(disk->part0->bd_holder_dir);
- kobject_put(disk->slave_dir);
+ mutex_lock(&disk->open_mutex);
+ if (kobject_put(disk->part0->bd_holder_dir))
+ disk->part0->bd_holder_dir = NULL;
+ if (kobject_put(disk->slave_dir))
+ disk->slave_dir = NULL;
+ mutex_unlock(&disk->open_mutex);

part_stat_set_all(disk->part0, 0);
disk->part0->bd_stamp = 0;
diff --git a/block/holder.c b/block/holder.c
index 5283bc804cc1..fdfbe82e31e3 100644
--- a/block/holder.c
+++ b/block/holder.c
@@ -75,6 +75,13 @@ int bd_link_disk_holder(struct block_device *bdev, struct gendisk *disk)
struct bd_holder_disk *holder;
int ret = 0;

+ mutex_lock(&bdev->bd_disk->open_mutex);
+ /* Failed if bd_holder_dir is freed by del_gendisk() */
+ if (!bdev->bd_holder_dir) {
+ mutex_unlock(&bdev->bd_disk->open_mutex);
+ return -ENODEV;
+ }
+
mutex_lock(&disk->open_mutex);

WARN_ON_ONCE(!bdev->bd_holder);
@@ -111,6 +118,7 @@ int bd_link_disk_holder(struct block_device *bdev, struct gendisk *disk)

out_unlock:
mutex_unlock(&disk->open_mutex);
+ mutex_unlock(&bdev->bd_disk->open_mutex);
return ret;
}
EXPORT_SYMBOL_GPL(bd_link_disk_holder);
@@ -136,16 +144,19 @@ void bd_unlink_disk_holder(struct block_device *bdev, struct gendisk *disk)
{
struct bd_holder_disk *holder;

+ mutex_lock(&bdev->bd_disk->open_mutex);
mutex_lock(&disk->open_mutex);
holder = bd_find_holder_disk(bdev, disk);
if (!WARN_ON_ONCE(holder == NULL) && !--holder->refcnt) {
if (disk->slave_dir)
__unlink_disk_holder(bdev, disk);
- kobject_put(bdev->bd_holder_dir);
+ if (kobject_put(bdev->bd_holder_dir))
+ bdev->bd_holder_dir = NULL;
list_del_init(&holder->list);
kfree(holder);
}
mutex_unlock(&disk->open_mutex);
+ mutex_unlock(&bdev->bd_disk->open_mutex);
}
EXPORT_SYMBOL_GPL(bd_unlink_disk_holder);

diff --git a/block/partitions/core.c b/block/partitions/core.c
index b8112f52d388..eef7b8615419 100644
--- a/block/partitions/core.c
+++ b/block/partitions/core.c
@@ -279,7 +279,10 @@ static void delete_partition(struct block_device *part)
__invalidate_device(part, true);

xa_erase(&part->bd_disk->part_tbl, part->bd_partno);
- kobject_put(part->bd_holder_dir);
+ mutex_lock(&part->bd_disk->open_mutex);
+ if (kobject_put(part->bd_holder_dir))
+ part->bd_holder_dir = NULL;
+ mutex_unlock(&part->bd_disk->open_mutex);
device_del(&part->bd_device);

/*
--
2.31.1