* [PATCH RFC 1/2] kobject: add return value for kobject_put()
2022-10-18 13:14 [PATCH RFC 0/2] block: fix uaf in bd_link_disk_holder() Yu Kuai
@ 2022-10-18 13:14 ` Yu Kuai
2022-10-18 13:00 ` Greg KH
2022-10-18 13:14 ` [PATCH RFC 2/2] block: protect slave_dir/bd_holder_dir by open_mutex Yu Kuai
1 sibling, 1 reply; 6+ messages in thread
From: Yu Kuai @ 2022-10-18 13:14 UTC (permalink / raw)
To: hch, axboe, gregkh, willy, martin.petersen, kch
Cc: linux-block, linux-kernel, yukuai3, yukuai1, yi.zhang
The return value will be used in later patch to fix uaf for slave_dir
and bd_holder_dir in block layer.
Signed-off-by: Yu Kuai <yukuai3@huawei.com>
---
include/linux/kobject.h | 2 +-
lib/kobject.c | 7 +++++--
2 files changed, 6 insertions(+), 3 deletions(-)
diff --git a/include/linux/kobject.h b/include/linux/kobject.h
index 57fb972fea05..f12de6274c51 100644
--- a/include/linux/kobject.h
+++ b/include/linux/kobject.h
@@ -110,7 +110,7 @@ extern int __must_check kobject_move(struct kobject *, struct kobject *);
extern struct kobject *kobject_get(struct kobject *kobj);
extern struct kobject * __must_check kobject_get_unless_zero(
struct kobject *kobj);
-extern void kobject_put(struct kobject *kobj);
+extern bool kobject_put(struct kobject *kobj);
extern const void *kobject_namespace(struct kobject *kobj);
extern void kobject_get_ownership(struct kobject *kobj,
diff --git a/lib/kobject.c b/lib/kobject.c
index a0b2dbfcfa23..f86c55ae7376 100644
--- a/lib/kobject.c
+++ b/lib/kobject.c
@@ -711,15 +711,18 @@ static void kobject_release(struct kref *kref)
*
* Decrement the refcount, and if 0, call kobject_cleanup().
*/
-void kobject_put(struct kobject *kobj)
+bool kobject_put(struct kobject *kobj)
{
if (kobj) {
if (!kobj->state_initialized)
WARN(1, KERN_WARNING
"kobject: '%s' (%p): is not initialized, yet kobject_put() is being called.\n",
kobject_name(kobj), kobj);
- kref_put(&kobj->kref, kobject_release);
+ if (kref_put(&kobj->kref, kobject_release))
+ return true;
}
+
+ return false;
}
EXPORT_SYMBOL(kobject_put);
--
2.31.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH RFC 2/2] block: protect slave_dir/bd_holder_dir by open_mutex
2022-10-18 13:14 [PATCH RFC 0/2] block: fix uaf in bd_link_disk_holder() Yu Kuai
2022-10-18 13:14 ` [PATCH RFC 1/2] kobject: add return value for kobject_put() Yu Kuai
@ 2022-10-18 13:14 ` Yu Kuai
1 sibling, 0 replies; 6+ messages in thread
From: Yu Kuai @ 2022-10-18 13:14 UTC (permalink / raw)
To: hch, axboe, gregkh, willy, martin.petersen, kch
Cc: linux-block, linux-kernel, yukuai3, yukuai1, yi.zhang
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@huawei.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
^ permalink raw reply related [flat|nested] 6+ messages in thread