Re: [PATCH v2 2/2] blk-mq: fix potential uaf for 'queue_hw_ctx'

[Ming Lei <ming.lei@redhat.com>]

On Thu, Nov 27, 2025 at 09:39:08AM +0800, Fengnan Chang wrote:
> From: Fengnan Chang <changfengnan@bytedance.com>
> 
> This is just apply Kuai's patch in [1] with mirror changes.
> 
> blk_mq_realloc_hw_ctxs() will free the 'queue_hw_ctx'(e.g. undate
> submit_queues through configfs for null_blk), while it might still be
> used from other context(e.g. switch elevator to none):
> 
> t1					t2
> elevator_switch
>  blk_mq_unquiesce_queue
>   blk_mq_run_hw_queues
>    queue_for_each_hw_ctx
>     // assembly code for hctx = (q)->queue_hw_ctx[i]
>     mov    0x48(%rbp),%rdx -> read old queue_hw_ctx
> 
> 					__blk_mq_update_nr_hw_queues
> 					 blk_mq_realloc_hw_ctxs
> 					  hctxs = q->queue_hw_ctx
> 					  q->queue_hw_ctx = new_hctxs
> 					  kfree(hctxs)
>     movslq %ebx,%rax
>     mov    (%rdx,%rax,8),%rdi ->uaf
> 
> This problem was found by code review, and I comfirmed that the concurrent
> scenario do exist(specifically 'q->queue_hw_ctx' can be changed during
> blk_mq_run_hw_queues()), however, the uaf problem hasn't been repoduced yet
> without hacking the kernel.
> 
> Sicne the queue is freezed in __blk_mq_update_nr_hw_queues(), fix the
> problem by protecting 'queue_hw_ctx' through rcu where it can be accessed
> without grabbing 'q_usage_counter'.
> 
> [1] https://lore.kernel.org/all/20220225072053.2472431-1-yukuai3@huawei.com/
> 
> Signed-off-by: Yu Kuai <yukuai3@huawei.com>
> Signed-off-by: Fengnan Chang <changfengnan@bytedance.com>
> ---
>  block/blk-mq.c         |  7 ++++++-
>  block/blk-mq.h         | 11 +++++++++++
>  include/linux/blk-mq.h |  2 +-
>  include/linux/blkdev.h |  2 +-
>  4 files changed, 19 insertions(+), 3 deletions(-)
> 
> diff --git a/block/blk-mq.c b/block/blk-mq.c
> index eed12fab3484..0b8b72194003 100644
> --- a/block/blk-mq.c
> +++ b/block/blk-mq.c
> @@ -4524,7 +4524,12 @@ static void __blk_mq_realloc_hw_ctxs(struct blk_mq_tag_set *set,
>  		if (hctxs)
>  			memcpy(new_hctxs, hctxs, q->nr_hw_queues *
>  			       sizeof(*hctxs));
> -		q->queue_hw_ctx = new_hctxs;
> +		rcu_assign_pointer(q->queue_hw_ctx, new_hctxs);
> +		/*
> +		 * Make sure reading the old queue_hw_ctx from other
> +		 * context concurrently won't trigger uaf.
> +		 */
> +		synchronize_rcu_expedited();
>  		kfree(hctxs);
>  		hctxs = new_hctxs;
>  	}
> diff --git a/block/blk-mq.h b/block/blk-mq.h
> index 80a3f0c2bce7..52852fab78f0 100644
> --- a/block/blk-mq.h
> +++ b/block/blk-mq.h
> @@ -87,6 +87,17 @@ static inline struct blk_mq_hw_ctx *blk_mq_map_queue_type(struct request_queue *
>  	return q->queue_hw_ctx[q->tag_set->map[type].mq_map[cpu]];
>  }
>  
> +static inline struct blk_mq_hw_ctx *queue_hctx(struct request_queue *q, int id)
> +{
> +	struct blk_mq_hw_ctx *hctx;
> +
> +	rcu_read_lock();
> +	hctx = rcu_dereference(q->queue_hw_ctx)[id];
> +	rcu_read_unlock();
> +
> +	return hctx;
> +}


If `hctx` is retrieved from old table, uaf will be triggered on
`hctx` dereference after returning from queue_hctx().

So rcu read lock should be held anywhere for `hctx` deference.



Thanks, 
Ming