mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Mohamed Khalfella <mkhalfella@purestorage.com>
To: mt4022@columbia.edu
Cc: Jens Axboe <axboe@kernel.dk>, Keith Busch <kbusch@kernel.org>,
	Ming Lei <tom.leiming@gmail.com>,
	Bart Van Assche <bvanassche@acm.org>,
	Christoph Hellwig <hch@lst.de>,
	Nilay Shroff <nilay@linux.ibm.com>,
	Josef Bacik <josef@toxicpanda.com>,
	linux-block@vger.kernel.org, linux-kernel@vger.kernel.org,
	tz2294@columbia.edu, stable@vger.kernel.org
Subject: Re: [PATCH] blk-mq: take quiesce_lock when removing a queue from the tag set
Date: Tue, 3 Nov 2026 11:51:48 -0800	[thread overview]
Message-ID: <20261103195148.GB5019-mkhalfella@purestorage.com> (raw)
In-Reply-To: <20261103-blk-mq-del-quiesce-lock-v1-1-04aa29a481f6@columbia.edu>

On Tue 2026-11-03 14:04:10 -0500, Mani Tofigh via B4 Relay wrote:
> From: Mani Tofigh <mt4022@columbia.edu>
> 
> Unquiescing a tag set can run the hw queues of a queue whose hctxs were
> already freed.  blk_mq_unquiesce_tagset() walks the tag set's queues
> under quiesce_lock and rcu_read_lock().  Commit 71ec3c7d6db8 ("blk-mq:
> quiesce a queue that joins a quiesced tag set") added quiesce_lock to
> the walks and to adding a queue, but blk_mq_del_queue_tag_set() still
> removes a queue without it.  The hctxs are then freed right away since
> RCU only keeps the request_queue itself alive:
> 
>   Task A (controller reset)            Task B (namespace removal)
>   blk_mq_unquiesce_tagset()
>     /* finds q */
>                                        blk_mq_del_queue_tag_set(q)
>                                        blk_mq_release(q)
>                                          /* frees the hctxs */
>     blk_mq_run_hw_queues(q)
>       /* reads freed hctxs */
> 
> The window is short.  On PREEMPT_RT quiesce_lock is a sleeping lock and
> the walk can be preempted inside it, which is how it's hit here:
> 
>   [  399.851277] BUG: KASAN: slab-use-after-free in blk_mq_run_hw_queues+0x4bd/0x550
>   [  399.851328] Read of size 8 at addr ff11000003014148 by task kworker/u9:2/116
>   [  399.851352] CPU: 1 UID: 0 PID: 116 Comm: kworker/u9:2 Not tainted 7.3.0-16299-g81cb9aa43246 #1 PREEMPT_RT
>   [  399.851425] Workqueue: nvme-reset-wq nvme_loop_reset_ctrl_work
>   [  399.851441] Call Trace:
>   [  399.851457]  <TASK>
>   [  399.851551]  blk_mq_run_hw_queues+0x4bd/0x550
>   [  399.851564]  blk_mq_unquiesce_tagset+0x128/0x210
>   [  399.851576]  nvme_loop_reset_ctrl_work+0x78/0x4c0
>   [  399.851600]  process_one_work+0x92b/0x16d0
>   [  399.851660]  worker_thread+0x4e6/0xd60
>   [  399.851707]  kthread+0x300/0x3e0
>   [  399.851727]  ret_from_fork+0x4f0/0x6f0
>   [  399.851776]  ret_from_fork_asm+0x1a/0x30
>   [  399.851793]  </TASK>
>   ...
>   [  399.851988] Freed by task 111:
>   [  399.852040]  kfree+0x1b3/0x4b0
>   [  399.852050]  blk_mq_release+0x400/0x550
>   [  399.852060]  blk_put_queue+0x94/0x120
>   [  399.852068]  disk_release+0x1bd/0x3f0
>   [  399.852102]  nvme_free_ns+0x39/0x270
>   [  399.852113]  nvme_remove_nsid_range+0x2d1/0x4a0
>   [  399.852123]  nvme_scan_ns_list+0x2cc/0x390
>   [  399.852134]  nvme_scan_work+0x421/0x9f0
>   ...
> 
> With BLK_CGROUP the disk's root blkg holds the queue until a grace
> period later, which hides this for NVMe namespaces.  It's still open
> without BLK_CGROUP, and from reading the code also for queues freed
> without a disk (e.g., SCSI device queues on a UFS host).
> 
> Take quiesce_lock around the list_del_rcu() as well so that the walk
> either finishes with a queue before it leaves the list or never sees
> it.  With the fix, KASAN doesn't report the use-after-free anymore.

Do we need to update the documentation of @quiesce_lock to mention that
it is now taken when a queue is deleted also?

> 
> This bug was identified and confirmed with the help of a deterministic
> hypervisor currently under development.  The C reproducer below however
> hits it about once every 20 minutes on a PREEMPT_RT kernel.
> 
> Fixes: 59e25ef2b413 ("block: Use RCU in blk_mq_[un]quiesce_tagset() instead of set->tag_list_lock")

Yep, my commit introduced the race by not taking set->tag_list_lock
which provided the needed serialization previously. Thank you for fixing
it.

> Cc: stable@vger.kernel.org
> Signed-off-by: Mani Tofigh <mt4022@columbia.edu>

Reviewed-by: Mohamed Khalfella <mkhalfella@purestorage.com>

      reply	other threads:[~2026-11-03 19:51 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-11-03 19:04 Mani Tofigh via B4 Relay
2026-11-03 19:51 ` Mohamed Khalfella [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20261103195148.GB5019-mkhalfella@purestorage.com \
    --to=mkhalfella@purestorage.com \
    --cc=axboe@kernel.dk \
    --cc=bvanassche@acm.org \
    --cc=hch@lst.de \
    --cc=josef@toxicpanda.com \
    --cc=kbusch@kernel.org \
    --cc=linux-block@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mt4022@columbia.edu \
    --cc=nilay@linux.ibm.com \
    --cc=stable@vger.kernel.org \
    --cc=tom.leiming@gmail.com \
    --cc=tz2294@columbia.edu \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®