[PATCH] block: stop the timeout timer when releasing a never added disk

[PATCH] block: stop the timeout timer when releasing a never added disk

[Chao Shi]

disk_release() undoes blk_mq_init_allocated_queue() for a disk whose
probe failed before add_disk(), but it only calls blk_mq_exit_queue().
Nothing there stops q->timeout, and that timer rolls forward: it stays
pending until it next expires, not until the last request completes.
So if the driver issued any I/O before adding the disk, the
request_queue is freed while still linked into a timer wheel bucket.

Commit 6f8191fdf41d ("block: simplify disk shutdown") dropped the
blk_cleanup_queue() call that used to stop it.  __del_gendisk() and
blk_mq_destroy_queue() still do; only the probe failure path lost it.

nvme gets there because nvme_update_ns_info() submits Report Zones or
FDP io-mgmt-recv on ns->queue before the disk is added, so a later
failure - a concurrent reset setting NVME_CTRL_FROZEN, or
device_add_disk() failing - lands in put_disk() with the timer armed:

  BUG: KASAN: slab-use-after-free in detach_if_pending+0x30c/0x340
  Write of size 8 at addr ffff888004d71310 by task kworker/u8:2/37
   __timer_delete_sync+0x156/0x240 kernel/time/timer.c:1621
   blk_sync_queue+0x22/0x40 block/blk-core.c:222
   nvme_sync_queues+0x100/0x150 drivers/nvme/host/core.c:5362
   nvme_reset_work+0x138/0x930 drivers/nvme/host/pci.c:3264

  Allocated by task 34:
   __blk_mq_alloc_disk+0x33/0x100 block/blk-mq.c:4462
   nvme_alloc_ns+0x290/0x3870 drivers/nvme/host/core.c:4146

  Freed by task 0:
   blk_free_queue_rcu+0x3a/0x50 block/blk-core.c:254
   rcu_core+0xc10/0x1730 kernel/rcu/tree.c:2857

The queue being synced there is ctrl->admin_q, only a victim sharing a
timer wheel bucket with the freed queue's dangling entry; other runs
tripped in enqueue_timer(), __run_timers() or blk_mq_timeout_work().
Failing nvme_alloc_ns() with a debug patch makes it deterministic: one
leaked timer trips KASAN within seconds, while 1987 patched releases
produced no splat.

Stop the timer and the queue work items before blk_mq_exit_queue(), like
blk_mq_destroy_queue() does.

Found by FuzzNvme.

Fixes: 6f8191fdf41d ("block: simplify disk shutdown")
Acked-by: Weidong Zhu <weizhu@fiu.edu>
Signed-off-by: Chao Shi <coshi036@gmail.com>
---
 block/genhd.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/block/genhd.c b/block/genhd.c
index df2c3c69b467..e8ce0cabf392 100644
--- a/block/genhd.c
+++ b/block/genhd.c
@@ -1281,14 +1281,18 @@ static void disk_release(struct device *dev)
 	/*
 	 * To undo the all initialization from blk_mq_init_allocated_queue in
 	 * case of a probe failure where add_disk is never called we have to
-	 * call blk_mq_exit_queue here. We can't do this for the more common
-	 * teardown case (yet) as the tagset can be gone by the time the disk
-	 * is released once it was added.
+	 * call blk_mq_exit_queue here, after stopping the timer and work items
+	 * that I/O issued before add_disk may have left pending.  We can't do
+	 * this for the more common teardown case (yet) as the tagset can be
+	 * gone by the time the disk is released once it was added.
 	 */
 	if (queue_is_mq(disk->queue) &&
 	    test_bit(GD_OWNS_QUEUE, &disk->state) &&
-	    !test_bit(GD_ADDED, &disk->state))
+	    !test_bit(GD_ADDED, &disk->state)) {
+		blk_sync_queue(disk->queue);
+		blk_mq_cancel_work_sync(disk->queue);
 		blk_mq_exit_queue(disk->queue);
+	}
 
 	blkcg_exit_disk(disk);
 
-- 
2.43.0

Re: [PATCH] block: stop the timeout timer when releasing a never added disk

[Christoph Hellwig]

Looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>

Re: [PATCH] block: stop the timeout timer when releasing a never added disk

[Chris S]

Thank you!

Best,
Chao

On Tue, Jul 28, 2026 at 12:23 AM Christoph Hellwig <hch@lst.de> wrote:
>
> Looks good:
>
> Reviewed-by: Christoph Hellwig <hch@lst.de>
>

Re: [PATCH] block: stop the timeout timer when releasing a never added disk

[Jens Axboe]

On Mon, 27 Jul 2026 16:12:57 -0400, Chao Shi wrote:
> disk_release() undoes blk_mq_init_allocated_queue() for a disk whose
> probe failed before add_disk(), but it only calls blk_mq_exit_queue().
> Nothing there stops q->timeout, and that timer rolls forward: it stays
> pending until it next expires, not until the last request completes.
> So if the driver issued any I/O before adding the disk, the
> request_queue is freed while still linked into a timer wheel bucket.
> 
> [...]

Applied, thanks!

[1/1] block: stop the timeout timer when releasing a never added disk
      commit: 26cb8ebbfaf713c82e142d08828d4d765057633b

Best regards,
-- 
Jens Axboe

Re: [PATCH] block: stop the timeout timer when releasing a never added disk

[Chris S]

Thank you so much!

Best,
Chao

On Wed, Jul 29, 2026 at 7:23 AM Jens Axboe <axboe@kernel.dk> wrote:
>
>
> On Mon, 27 Jul 2026 16:12:57 -0400, Chao Shi wrote:
> > disk_release() undoes blk_mq_init_allocated_queue() for a disk whose
> > probe failed before add_disk(), but it only calls blk_mq_exit_queue().
> > Nothing there stops q->timeout, and that timer rolls forward: it stays
> > pending until it next expires, not until the last request completes.
> > So if the driver issued any I/O before adding the disk, the
> > request_queue is freed while still linked into a timer wheel bucket.
> >
> > [...]
>
> Applied, thanks!
>
> [1/1] block: stop the timeout timer when releasing a never added disk
>       commit: 26cb8ebbfaf713c82e142d08828d4d765057633b
>
> Best regards,
> --
> Jens Axboe
>
>
>

Re: [PATCH] block: stop the timeout timer when releasing a never added disk

[Chris S]

Hi Jan, I'm currently working on the changes we discussed.  Based on
vfs.all now.  Four things came out of writing it that I would rather
resolve before posting.

1. mark_buffer_write_io_error() is not safe for jbd2's buffers yet.  It
dereferences bh->b_folio->mapping directly (fs/buffer.c:1057), and
jbd2_journal_write_metadata_buffer() points the temporary bh at
virt_to_folio(jh->b_frozen_data), which is slab-backed.
buffer_set_crypto_ctx() right next to it already uses folio_mapping() for
exactly this reason.  So I would send a prerequisite converting
mark_buffer_write_io_error() to folio_mapping() before the jbd2 change.
Also worth noting the consequence of A2: in ordered mode the temp bh
inherits the source folio, so the error lands on the bdev mapping and
mapping_set_error() errseq_set()s s_wb_err - a later syncfs() on an
unrelated fd of the same fs can then return EIO.

2. gfs2 already has what we are building: gfs2_end_log_write_bh()
(fs/gfs2/lops.c:171) calls mark_buffer_write_io_error() and does not clear
uptodate.  Which means fs/gfs2/log.c:110 and :324 are blind to log write
errors today, and converting them is not behaviour-preserving - it makes
them start catching those.  I think that is right, but say if you would
rather it were separate.

3. On the sticky BH_Write_EIO: I am adding it to BUFFER_FLAGS_DISCARD as
you suggested.  That covers discard_buffer(), but there are four other
clear_buffer_req() sites (fs/buffer.c:1687, fs/gfs2/aops.c:600,
fs/jbd2/commit.c:1035, fs/jbd2/transaction.c:2462).  Three of them also
clear_buffer_mapped() and NULL b_bdev, so __bh_submit()'s
BUG_ON(!buffer_mapped(bh)) makes them unreachable.  Only
clean_bdev_aliases() leaves the buffer writable, and I could not construct
a workload that reaches it.  So I am not sending the ungating patch.  The
one place it would matter is ocfs2: fs/ocfs2/journal.c:691 is nested
inside an if (!buffer_uptodate(bh)) at :675 that goes dead once the clear
is gone, so :691 has to be hoisted out, and it then becomes a pre-use
check on a sticky flag rather than a post-write one. I will Cc ocfs2-devel
on that patch.

4. fs/jbd2/transaction.c:923, J_EXPECT_JH(jh, buffer_uptodate(bh)) in
jbd2_freeze_jh_data(): after the series a failed write leaves the buffer
uptodate, so this stops firing for write errors.  Arguably correct - the
in-memory copy being frozen is still valid - but it is your assert.  Leave
it, or convert it?  I instrumented it and ran ext4 with data=journal under
injected write errors, forcing copy-out; it never saw a non-uptodate
buffer, so I have no evidence either way.

One thing I did settle by testing.  fs/ext4/ext4_jbd2.c:416 open-codes
buffer_req(bh) && !buffer_uptodate(bh) after sync_dirty_buffer(), which
already returns -EIO.  On an instrumented kernel (ext4 without a journal,
-o sync, injected write errors) the two agreed on all 40 occurrences and
never diverged, with BH_Write_EIO set every time.  So that site just
consumes the return value and the buffer_req() question goes away.

Best,
Chao

On Wed, Jul 29, 2026 at 7:23 AM Jens Axboe <axboe@kernel.dk> wrote:
>
>
> On Mon, 27 Jul 2026 16:12:57 -0400, Chao Shi wrote:
> > disk_release() undoes blk_mq_init_allocated_queue() for a disk whose
> > probe failed before add_disk(), but it only calls blk_mq_exit_queue().
> > Nothing there stops q->timeout, and that timer rolls forward: it stays
> > pending until it next expires, not until the last request completes.
> > So if the driver issued any I/O before adding the disk, the
> > request_queue is freed while still linked into a timer wheel bucket.
> >
> > [...]
>
> Applied, thanks!
>
> [1/1] block: stop the timeout timer when releasing a never added disk
>       commit: 26cb8ebbfaf713c82e142d08828d4d765057633b
>
> Best regards,
> --
> Jens Axboe
>
>
>

Re: [PATCH] block: stop the timeout timer when releasing a never added disk

[Chris S]

Sorry for the mistake, I think I replied to the wrong emails.

Best,
Chao

On Wed, Jul 29, 2026 at 7:23 AM Jens Axboe <axboe@kernel.dk> wrote:
>
>
> On Mon, 27 Jul 2026 16:12:57 -0400, Chao Shi wrote:
> > disk_release() undoes blk_mq_init_allocated_queue() for a disk whose
> > probe failed before add_disk(), but it only calls blk_mq_exit_queue().
> > Nothing there stops q->timeout, and that timer rolls forward: it stays
> > pending until it next expires, not until the last request completes.
> > So if the driver issued any I/O before adding the disk, the
> > request_queue is freed while still linked into a timer wheel bucket.
> >
> > [...]
>
> Applied, thanks!
>
> [1/1] block: stop the timeout timer when releasing a never added disk
>       commit: 26cb8ebbfaf713c82e142d08828d4d765057633b
>
> Best regards,
> --
> Jens Axboe
>
>
>