Re: [PATCH] block: prevent deadlock in del_gendisk()

[Yu Kuai <yukuai1@huaweicloud.com>]

Hi,

在 2025/08/03 21:41, Ujwal Kundur 写道:
> A potential unsafe locking scenario presents itself when
> mutex_lock(&disk->open_mutex) is called with reader's lock held on
> update_nr_hwq_lock:
>         CPU0                    CPU1
>         ----                    ----
> rlock(&set->update_nr_hwq_lock)
>                                 lock(&nbd->config_lock);
>                                 lock(&set->update_nr_hwq_lock);
> lock(&disk->open_mutex)
> 
This problem is already fixed inside nbd by:
8b428f42f3ed ("nbd: fix lockdep deadlock warning")

Thanks,
Kuai
> When the gendisk is added back concurrently, a writer's lock is
> attempted to be held on update_nr_hwq_lock while holding other locks in
> the call-path, becoming a potential source of deadlock(s).
> 
> Scope read-critical section to blk_unregister_queue, which is the only
> function that interacts with switching elevator and requires
> synchronization with update_nr_hwq_lock.
> 
> Reported-by: syzbot+2e9e529ac0b319316453@syzkaller.appspotmail.com
> Signed-off-by: Ujwal Kundur <ujwal.kundur@gmail.com>
> ---
>   block/genhd.c | 8 +++-----
>   1 file changed, 3 insertions(+), 5 deletions(-)
>   mode change 100644 => 100755 block/genhd.c
> 
> diff --git a/block/genhd.c b/block/genhd.c
> old mode 100644
> new mode 100755
> index c26733f6324b..b56f09f5699b
> --- a/block/genhd.c
> +++ b/block/genhd.c
> @@ -696,6 +696,7 @@ static void __del_gendisk(struct gendisk *disk)
>   	struct block_device *part;
>   	unsigned long idx;
>   	bool start_drain;
> +	struct blk_mq_tag_set *set = q->tag_set;
>   
>   	might_sleep();
>   
> @@ -740,7 +741,9 @@ static void __del_gendisk(struct gendisk *disk)
>   		bdi_unregister(disk->bdi);
>   	}
>   
> +	down_read(&set->update_nr_hwq_lock);
>   	blk_unregister_queue(disk);
> +	up_read(&set->update_nr_hwq_lock);
>   
>   	kobject_put(disk->part0->bd_holder_dir);
>   	kobject_put(disk->slave_dir);
> @@ -808,20 +811,15 @@ static void disable_elv_switch(struct request_queue *q)
>    */
>   void del_gendisk(struct gendisk *disk)
>   {
> -	struct blk_mq_tag_set *set;
>   	unsigned int memflags;
>   
>   	if (!queue_is_mq(disk->queue)) {
>   		__del_gendisk(disk);
>   	} else {
> -		set = disk->queue->tag_set;
> -
>   		disable_elv_switch(disk->queue);
>   
>   		memflags = memalloc_noio_save();
> -		down_read(&set->update_nr_hwq_lock);
>   		__del_gendisk(disk);
> -		up_read(&set->update_nr_hwq_lock);
>   		memalloc_noio_restore(memflags);
>   	}
>   }
>