linux-kernel - [PATCH RFC 2/2] block: protect slave_dir/bd_holder_dir by open

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 18 Oct 2022 21:14:32 +0800
From:   Yu Kuai <yukuai3@...wei.com>
To:     <hch@....de>, <axboe@...nel.dk>, <gregkh@...uxfoundation.org>,
        <willy@...radead.org>, <martin.petersen@...cle.com>,
        <kch@...dia.com>
CC:     <linux-block@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
        <yukuai3@...wei.com>, <yukuai1@...weicloud.com>,
        <yi.zhang@...wei.com>
Subject: [PATCH RFC 2/2] block: protect slave_dir/bd_holder_dir by open_mutex

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@...wei.com>
---
 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