lkml.org 
[lkml]   [2022]   [Nov]   [3]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 2/2] block: fix use after free for bd_holder_dir
On Thu, Nov 03, 2022 at 10:55:41AM +0800, Yu Kuai wrote:
> From: Yu Kuai <yukuai3@huawei.com>
>
> Currently, the caller of bd_link_disk_holer() get 'bdev' by
> blkdev_get_by_dev(), which will look up 'bdev' by inode number 'dev'.
> Howerver, it's possible that del_gendisk() can be called currently, and
> 'bd_holder_dir' can be freed before bd_link_disk_holer() access it, thus
> use after free is triggered.
>
> t1: t2:
> bdev = blkdev_get_by_dev
> del_gendisk
> kobject_put(bd_holder_dir)
> kobject_free()
> bd_link_disk_holder
>
> Fix the problem by checking disk is still live and grabbing a reference
> to 'bd_holder_dir' first in bd_link_disk_holder().

Looks good with some minor stilistic nipicks:

> + if (!disk_live(bdev->bd_disk)) {
> + mutex_unlock(&bdev->bd_disk->open_mutex);
> + return -ENODEV;
> + }

This can use a goto out_unlock;

> holder->refcnt++;
> + kobject_put(bdev->bd_holder_dir);
> goto out_unlock;
> }
>
> holder = kzalloc(sizeof(*holder), GFP_KERNEL);
> if (!holder) {
> ret = -ENOMEM;
> + kobject_put(bdev->bd_holder_dir);
> goto out_unlock;
> }
>
> @@ -101,16 +114,12 @@ int bd_link_disk_holder(struct block_device *bdev, struct gendisk *disk)
> ret = __link_disk_holder(bdev, disk);
> if (ret) {
> kfree(holder);
> + kobject_put(bdev->bd_holder_dir);

And I think a goto out_put_holder and out_free_holder would clean this up
nicely.

> 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);

.. with this then jumping straight to out_unlock.


We should repost a series with my first 7 patches and your two. I can do
that, but it might take some time as I just got through (minor) knee
surgery and am still at the hospital, so if you have spare cycles feel
free to do it.

\
 
 \ /
  Last update: 2022-11-03 09:12    [W:0.055 / U:1.832 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site