[dm-devel] [PATCH 1/6] block: clear the holder releated fields when deleting the kobjects

[Yu Kuai]

Hi, Christoph

在 2022/10/21 0:46, Christoph Hellwig 写道:
> Zero out the pointers to the holder related kobjects so that the holder
> code doesn't incorrectly when called by dm for the delayed holder
> registration.
> 
> Fixes: 89f871af1b26 ("dm: delay registering the gendisk")
> Reported-by: Yu Kuai <yukuai1 at huaweicloud.com>
> Signed-off-by: Christoph Hellwig <hch at lst.de>
> ---
>   block/genhd.c | 4 ++++
>   1 file changed, 4 insertions(+)
> 
> diff --git a/block/genhd.c b/block/genhd.c
> index 17b33c62423df..cd90df6c775c2 100644
> --- a/block/genhd.c
> +++ b/block/genhd.c
> @@ -528,8 +528,10 @@ int __must_check device_add_disk(struct device *parent, struct gendisk *disk,
>   	blk_unregister_queue(disk);
>   out_put_slave_dir:
>   	kobject_put(disk->slave_dir);
> +	disk->slave_dir = NULL;
>   out_put_holder_dir:
>   	kobject_put(disk->part0->bd_holder_dir);
> +	disk->part0->bd_holder_dir = NULL;
>   out_del_integrity:
>   	blk_integrity_del(disk);
>   out_del_block_link:
> @@ -623,7 +625,9 @@ void del_gendisk(struct gendisk *disk)
>   	blk_unregister_queue(disk);
>   
>   	kobject_put(disk->part0->bd_holder_dir);
> +	disk->part0->bd_holder_dir = NULL;

I don't think this is enough. There is still no guarantee that
bd_link_disk_holder() won't access freed bd_holder_dir. It's still
possible that bd_link_disk_holer() read bd_holder_dir first, and then
del_gendisk() free and reset it.

By the way, I still think that the problem for the bd_holder_dir uaf is
not just related to dm.

Thanks,
Kuai

>   	kobject_put(disk->slave_dir);
> +	disk->slave_dir = NULL;
>   
>   	part_stat_set_all(disk->part0, 0);
>   	disk->part0->bd_stamp = 0;
>