[PATCH -nect RFC v2 2/2] block: fix uaf for bd_holder_dir

From: Yu Kuai
Date: Thu Oct 20 2022 - 08:59:17 EST


Lifecycle of bd_holder_dir is problematic:

t1: t2:

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

Fix the problem by protecting bd_holder_dir by open_mutex.

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

diff --git a/block/genhd.c b/block/genhd.c
index 53f9c8b2690a..b3d04e79e854 100644
--- a/block/genhd.c
+++ b/block/genhd.c
@@ -499,6 +499,8 @@ int __must_check device_add_disk(struct device *parent, struct gendisk *disk,
ret = -ENOMEM;
goto out_del_integrity;
}
+ disk->part0->holder_dir_ref = 1;
+
disk->slave_dir = kobject_create_and_add("slaves", &ddev->kobj);
if (!disk->slave_dir) {
ret = -ENOMEM;
@@ -557,6 +559,8 @@ int __must_check device_add_disk(struct device *parent, struct gendisk *disk,
kobject_put(disk->slave_dir);
out_put_holder_dir:
kobject_put(disk->part0->bd_holder_dir);
+ disk->part0->bd_holder_dir = NULL;
+ disk->part0->holder_dir_ref = 0;
out_del_integrity:
blk_integrity_del(disk);
out_del_block_link:
@@ -649,7 +653,7 @@ void del_gendisk(struct gendisk *disk)

blk_unregister_queue(disk);

- kobject_put(disk->part0->bd_holder_dir);
+ bd_put_holder_dir(disk->part0);
kobject_put(disk->slave_dir);

part_stat_set_all(disk->part0, 0);
diff --git a/block/holder.c b/block/holder.c
index 5283bc804cc1..2e28b472ba1a 100644
--- a/block/holder.c
+++ b/block/holder.c
@@ -1,6 +1,7 @@
// SPDX-License-Identifier: GPL-2.0-only
#include <linux/blkdev.h>
#include <linux/slab.h>
+#include "blk.h"

struct bd_holder_disk {
struct list_head list;
@@ -85,9 +86,19 @@ int bd_link_disk_holder(struct block_device *bdev, struct gendisk *disk)
goto out_unlock;
}

+ /*
+ * del_gendisk drops the initial reference to bd_holder_dir, so we need
+ * to keep our own here to allow for cleanup past that point.
+ */
+ if (!bd_try_get_holder_dir(bdev)) {
+ ret = -ENODEV;
+ goto out_unlock;
+ }
+
holder = kzalloc(sizeof(*holder), GFP_KERNEL);
if (!holder) {
ret = -ENOMEM;
+ bd_put_holder_dir(bdev);
goto out_unlock;
}

@@ -98,16 +109,12 @@ int bd_link_disk_holder(struct block_device *bdev, struct gendisk *disk)
ret = __link_disk_holder(bdev, disk);
if (ret) {
kfree(holder);
+ bd_put_holder_dir(bdev);
goto out_unlock;
}
}

list_add(&holder->list, &disk->slave_bdevs);
- /*
- * del_gendisk drops the initial reference to bd_holder_dir, so we need
- * to keep our own here to allow for cleanup past that point.
- */
- kobject_get(bdev->bd_holder_dir);

out_unlock:
mutex_unlock(&disk->open_mutex);
@@ -141,7 +148,7 @@ void bd_unlink_disk_holder(struct block_device *bdev, struct gendisk *disk)
if (!WARN_ON_ONCE(holder == NULL) && !--holder->refcnt) {
if (disk->slave_dir)
__unlink_disk_holder(bdev, disk);
- kobject_put(bdev->bd_holder_dir);
+ bd_put_holder_dir(bdev);
list_del_init(&holder->list);
kfree(holder);
}
diff --git a/block/partitions/core.c b/block/partitions/core.c
index b8112f52d388..39a14f7c308e 100644
--- a/block/partitions/core.c
+++ b/block/partitions/core.c
@@ -279,7 +279,7 @@ 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);
+ bd_put_holder_dir(part);
device_del(&part->bd_device);

/*
@@ -390,6 +390,7 @@ static struct block_device *add_partition(struct gendisk *disk, int partno,
bdev->bd_holder_dir = kobject_create_and_add("holders", &pdev->kobj);
if (!bdev->bd_holder_dir)
goto out_del;
+ bdev->holder_dir_ref = 1;

dev_set_uevent_suppress(pdev, 0);
if (flags & ADDPART_FLAG_WHOLEDISK) {
@@ -411,6 +412,8 @@ static struct block_device *add_partition(struct gendisk *disk, int partno,

out_del:
kobject_put(bdev->bd_holder_dir);
+ bdev->bd_holder_dir = NULL;
+ bdev->holder_dir_ref = 0;
device_del(pdev);
out_put:
put_device(pdev);
--
2.31.1