Hi,
在 2022/11/01 19:21, Christoph Hellwig 写道:
On Tue, Nov 01, 2022 at 07:12:51PM +0800, Yu Kuai wrote:
But how could the reference be 0 here? The driver that calls
bd_link_disk_holder must have the block device open and thus hold a
reference to it.
Like I said before, the caller of bd_link_disk_holder() get bdev by
blkdev_get_by_dev(), which do not grab reference of holder_dir, and
grab disk reference can only prevent disk_release() to be called, not
del_gendisk() while holder_dir reference is dropped in del_gendisk()
and can be decreased to 0.
Oh, the bd_holder_dir reference, not the block_device one. So yes,
I agree.
If you agree with above explanation, I tried to fix this:
1) move kobject_put(bd_holder_dir) from del_gendisk to disk_release,
there seems to be a lot of other dependencies.
2) protect bd_holder_dir reference by open_mutex.
I think simply switching the kobject_get in bd_link_disk_holder
into a kobject_get_unless_zero and unwinding if there is no reference
should be enough:
diff --git a/block/holder.c b/block/holder.c
index a8c355b9d0806..cd18064f6ff80 100644
--- a/block/holder.c
+++ b/block/holder.c
@@ -83,7 +83,11 @@ int bd_link_disk_holder(struct block_device *bdev, struct gendisk *disk)
INIT_LIST_HEAD(&holder->list);
holder->refcnt = 1;
- holder->holder_dir = kobject_get(bdev->bd_holder_dir);
+ if (!kobject_get_unless_zero(bdev->bd_holder_dir)) {
+ ret = -EBUSY;
+ goto out_free_holder;
+ }
+ holder->holder_dir = bdev->bd_holder_dir;
What if bd_holder_dir is already freed here, then uaf can be triggered.
Thus bd_holder_dir need to be resed in del_gendisk() if it's reference
is dropped to 0, however, kobject apis can't do that...
Thanks,
Kuai
ret = add_symlink(disk->slave_dir, bdev_kobj(bdev));
if (ret)
@@ -100,6 +104,7 @@ int bd_link_disk_holder(struct block_device *bdev, struct gendisk *disk)
del_symlink(disk->slave_dir, bdev_kobj(bdev));
out_put_holder_dir:
kobject_put(holder->holder_dir);
+out_free_holder:
kfree(holder);
out_unlock:
mutex_unlock(&disk->open_mutex);
.
