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 11:33:22AM +0800, Ming Lei wrote:
> 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.

oops, the rcu read lock only protects `q->queue_hw_ctx`, so this
way is fine, sorry for the noise!


Thanks. 
Ming