[PATCH] nvme: bound the freeze drain in passthrough commands

[PATCH] nvme: bound the freeze drain in passthrough commands

[Chao Shi]

nvme_passthru_start() drains in-flight I/O via the unbounded
nvme_wait_freeze() before submitting a command with command-set
effects (Format NVM, Sanitize, Namespace Management, vendor unique).
If a completion is silently dropped or the device hangs, the calling
task wedges with ctrl->scan_lock and ctrl->subsys->lock held, fanning
out into hung-task reports on any concurrent open/close/passthru on
the same controller:

  INFO: task syz-executor:NNNN blocked for more than 123 seconds.
   nvme_wait_freeze+0x82/0x100
   nvme_passthru_start drivers/nvme/host/core.c:1249 [inline]
   nvme_submit_user_cmd+0x1ee/0x3d0 drivers/nvme/host/ioctl.c:189

The other freeze-drain sites (pci shutdown, tcp/rdma reset) already
bound the wait with nvme_wait_freeze_timeout(NVME_IO_TIMEOUT).  Apply
it here too; on timeout, unwind the freeze and return -EBUSY (or
NVME_SC_INTERNAL on the nvmet path) instead of submitting the command.

Found by FuzzNvme(Syzkaller with FEMU fuzzing framework).

Acked-by: Sungwoo Kim <iam@sung-woo.kim>
Acked-by: Dave Tian <daveti@purdue.edu>
Acked-by: Weidong Zhu <weizhu@fiu.edu>
Signed-off-by: Chao Shi <coshi036@gmail.com>
---
 drivers/nvme/host/core.c       | 26 ++++++++++++++++++++------
 drivers/nvme/host/ioctl.c      |  7 ++++++-
 drivers/nvme/host/nvme.h       |  3 ++-
 drivers/nvme/target/passthru.c |  7 ++++++-
 4 files changed, 34 insertions(+), 9 deletions(-)

diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index 7bf228df6001..575f98b9a6cc 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -1232,23 +1232,37 @@ u32 nvme_command_effects(struct nvme_ctrl *ctrl, struct nvme_ns *ns, u8 opcode)
 }
 EXPORT_SYMBOL_NS_GPL(nvme_command_effects, "NVME_TARGET_PASSTHRU");
 
-u32 nvme_passthru_start(struct nvme_ctrl *ctrl, struct nvme_ns *ns, u8 opcode)
+int nvme_passthru_start(struct nvme_ctrl *ctrl, struct nvme_ns *ns, u8 opcode,
+			u32 *effects)
 {
-	u32 effects = nvme_command_effects(ctrl, ns, opcode);
+	*effects = nvme_command_effects(ctrl, ns, opcode);
 
 	/*
 	 * For simplicity, IO to all namespaces is quiesced even if the command
-	 * effects say only one namespace is affected.
+	 * effects say only one namespace is affected.  Bound the drain wait so
+	 * a stuck I/O cannot wedge the passthrough caller (and any task on the
+	 * scan_lock or subsys lock) indefinitely; the other in-tree callers of
+	 * the freeze drain (pci shutdown, tcp/rdma reset) already use this same
+	 * NVME_IO_TIMEOUT bound.
 	 */
-	if (effects & NVME_CMD_EFFECTS_CSE_MASK) {
+	if (*effects & NVME_CMD_EFFECTS_CSE_MASK) {
 		mutex_lock(&ctrl->scan_lock);
 		mutex_lock(&ctrl->subsys->lock);
 		nvme_mpath_start_freeze(ctrl->subsys);
 		nvme_mpath_wait_freeze(ctrl->subsys);
 		nvme_start_freeze(ctrl);
-		nvme_wait_freeze(ctrl);
+		if (!nvme_wait_freeze_timeout(ctrl, NVME_IO_TIMEOUT)) {
+			dev_warn(ctrl->device,
+				 "I/O did not drain in %u seconds; aborting passthrough\n",
+				 nvme_io_timeout);
+			nvme_unfreeze(ctrl);
+			nvme_mpath_unfreeze(ctrl->subsys);
+			mutex_unlock(&ctrl->subsys->lock);
+			mutex_unlock(&ctrl->scan_lock);
+			return -EBUSY;
+		}
 	}
-	return effects;
+	return 0;
 }
 EXPORT_SYMBOL_NS_GPL(nvme_passthru_start, "NVME_TARGET_PASSTHRU");
 
diff --git a/drivers/nvme/host/ioctl.c b/drivers/nvme/host/ioctl.c
index a9c097dacad6..762458a23b38 100644
--- a/drivers/nvme/host/ioctl.c
+++ b/drivers/nvme/host/ioctl.c
@@ -186,7 +186,12 @@ static int nvme_submit_user_cmd(struct request_queue *q,
 	bio = req->bio;
 	ctrl = nvme_req(req)->ctrl;
 
-	effects = nvme_passthru_start(ctrl, ns, cmd->common.opcode);
+	ret = nvme_passthru_start(ctrl, ns, cmd->common.opcode, &effects);
+	if (ret) {
+		if (bio)
+			blk_rq_unmap_user(bio);
+		goto out_free_req;
+	}
 	ret = nvme_execute_rq(req, false);
 	if (result)
 		*result = le64_to_cpu(nvme_req(req)->result.u64);
diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h
index 9a5f28c5103c..665d75de044e 100644
--- a/drivers/nvme/host/nvme.h
+++ b/drivers/nvme/host/nvme.h
@@ -1211,7 +1211,8 @@ static inline void nvme_auth_revoke_tls_key(struct nvme_ctrl *ctrl) {};
 
 u32 nvme_command_effects(struct nvme_ctrl *ctrl, struct nvme_ns *ns,
 			 u8 opcode);
-u32 nvme_passthru_start(struct nvme_ctrl *ctrl, struct nvme_ns *ns, u8 opcode);
+int nvme_passthru_start(struct nvme_ctrl *ctrl, struct nvme_ns *ns, u8 opcode,
+			u32 *effects);
 int nvme_execute_rq(struct request *rq, bool at_head);
 void nvme_passthru_end(struct nvme_ctrl *ctrl, struct nvme_ns *ns, u32 effects,
 		       struct nvme_command *cmd, int status);
diff --git a/drivers/nvme/target/passthru.c b/drivers/nvme/target/passthru.c
index 67c423a8b052..7b97bfc1ace6 100644
--- a/drivers/nvme/target/passthru.c
+++ b/drivers/nvme/target/passthru.c
@@ -220,7 +220,12 @@ static void nvmet_passthru_execute_cmd_work(struct work_struct *w)
 	u32 effects;
 	int status;
 
-	effects = nvme_passthru_start(ctrl, ns, req->cmd->common.opcode);
+	status = nvme_passthru_start(ctrl, ns, req->cmd->common.opcode, &effects);
+	if (status) {
+		nvmet_req_complete(req, NVME_SC_INTERNAL);
+		blk_mq_free_request(rq);
+		return;
+	}
 	status = nvme_execute_rq(rq, false);
 	if (status == NVME_SC_SUCCESS &&
 	    req->cmd->common.opcode == nvme_admin_identify) {
-- 
2.43.0

Re: [PATCH] nvme: bound the freeze drain in passthrough commands

[Christoph Hellwig]

On Wed, May 27, 2026 at 01:59:23AM -0400, Chao Shi wrote:
> nvme_passthru_start() drains in-flight I/O via the unbounded
> nvme_wait_freeze() before submitting a command with command-set
> effects (Format NVM, Sanitize, Namespace Management, vendor unique).
> If a completion is silently dropped or the device hangs, the calling
> task wedges with ctrl->scan_lock and ctrl->subsys->lock held, fanning
> out into hung-task reports on any concurrent open/close/passthru on
> the same controller:
> 
>   INFO: task syz-executor:NNNN blocked for more than 123 seconds.
>    nvme_wait_freeze+0x82/0x100
>    nvme_passthru_start drivers/nvme/host/core.c:1249 [inline]
>    nvme_submit_user_cmd+0x1ee/0x3d0 drivers/nvme/host/ioctl.c:189
> 
> The other freeze-drain sites (pci shutdown, tcp/rdma reset) already
> bound the wait with nvme_wait_freeze_timeout(NVME_IO_TIMEOUT).  Apply
> it here too; on timeout, unwind the freeze and return -EBUSY (or
> NVME_SC_INTERNAL on the nvmet path) instead of submitting the command.
> 
> Found by FuzzNvme(Syzkaller with FEMU fuzzing framework).

So not blocking forever sounds useful, but this might break existing
uses.  I guess we could do it based on the O_NONBLOCK flag if people
really cared.

Note that the blocked message itself is not a problem, but around
this time we should have done a controller reset and fixed up the
issue.  Does that not happen for your test case?

Re: [PATCH] nvme: bound the freeze drain in passthrough commands

[Keith Busch]

On Wed, May 27, 2026 at 01:59:23AM -0400, Chao Shi wrote:
> If a completion is silently dropped or the device hangs, the calling
> task wedges with ctrl->scan_lock and ctrl->subsys->lock held, fanning
> out into hung-task reports on any concurrent open/close/passthru on
> the same controller:

The IO timeout callbacks that nvme drivers provide are supposed to
forcefully reclaim any IO no matter what state the device is in. Is that
not happening for some reason?

Re: [PATCH] nvme: bound the freeze drain in passthrough commands

[Chao S]

Hi Keith, Christoph,

Both questions land on the same point, so one answer below.

Keith wrote:
> The IO timeout callbacks that nvme drivers provide are supposed to
> forcefully reclaim any IO no matter what state the device is in. Is
> that not happening for some reason?

Christoph wrote:
> Note that the blocked message itself is not a problem, but around
> this time we should have done a controller reset and fixed up the
> issue.  Does that not happen for your test case?

It did happen.  nvme_io_timeout was the default 30, and the relevant
dmesg from the campaign that produced the hung-task report:

  [ 44.220408] nvme nvme0: I/O tag 451 opcode 0x2 (Read) QID 1 timeout, aborting
  [ 44.220798] nvme nvme0: I/O tag 819 opcode 0x1 (Write) QID 1
timeout, aborting
  [ 44.220820] nvme nvme0: Abort status: 0x0
  [ 44.221286] nvme nvme0: Abort status: 0x0
  [ 45.591197] nvme nvme0: resetting controller
  [ 46.307151] nvme nvme0: IO queues lost
  [...100s of silence...]
  [144.561596] INFO: task systemd-udevd:134 blocked for more than 123 seconds.

Timeout fires, abort is accepted, reset starts, reset reaches the
"IO queues lost" branch (drivers/nvme/host/pci.c).  Then nvme_reset_work
itself blocks at

  nvme_mark_namespaces_dead -> blk_mark_disk_dead -> blk_report_disk_dead
    -> bdev_mark_dead(bdev, true) -> sync_blockdev -> folio_wait_writeback

i.e. the unconditional sync_blockdev in bdev_mark_dead's bare-bdev
else-branch (block/bdev.c) is itself waiting on the writeback that the
reset was supposed to drain.  The reset_work kworker doesn't show in
this dmesg because it crosses the 123s threshold ~25s after the
snapshot console was cut.

So in this report, the IO timeout did its job, but the reset that the
timeout kicks off cannot complete, and nvme_passthru_start (which is
already in nvme_wait_freeze at this point) has no way to back out.

Two reasons I still think the bound in passthru_start is worth applying
on its own merit:

1. The reset path has several ways to fail to drain in
   nvme_io_timeout: abort can be rejected, the admin tag for abort
   can be unavailable, the controller can be wedged before abort
   lands, an in-progress reset can outlast nvme_io_timeout, or (as
   here) reset itself can block.  Each leaves nvme_passthru_start
   waiting forever, holding ctrl->scan_lock + subsys->lock + every
   namespace's freeze ref, which then fans out on bd_disk->open_mutex
   via any concurrent bdev_open/release or BLKRRPART.

2. The same pattern is already established in the tree.  pci shutdown
   (drivers/nvme/host/pci.c), nvme-tcp reset, nvme-rdma reset,
   nvme-apple, and Daniel Wagner's 2021 nvme-fc series
   (20210818120530.130501-1-dwagner@suse.de) all use
   nvme_wait_freeze_timeout(NVME_IO_TIMEOUT) for exactly this reason.
   nvme_passthru_start is the only userspace-reachable caller still
   on the unbounded variant.

Christoph also wrote:
> So not blocking forever sounds useful, but this might break existing
> uses.  I guess we could do it based on the O_NONBLOCK flag if people
> really cared.

NVME_IO_TIMEOUT is already the bound any submitted I/O can be
synchronously waited on; a freeze drain that legitimately exceeds it
implies the controller isn't doing useful I/O anyway, and
nvme_core.io_timeout=N scales both knobs coherently.  Drain on a
healthy system is sub-second.

Gating on O_NONBLOCK is a reasonable fallback if you'd rather keep
the old default, but the nvme ioctl path doesn't currently consult
fd flags, so it would be a new userspace contract.  My concern with
the current default is that any of the scenarios above lets a
userspace ioctl wedge a kthread with two ctrl-wide mutexes held; the
+9-line bound prevents that without changing the success path.

If the maintainer preference is the O_NONBLOCK gate instead, I'll
respin.

Thanks,
Chao

On Wed, May 27, 2026 at 11:46 AM Keith Busch <kbusch@kernel.org> wrote:
>
> On Wed, May 27, 2026 at 01:59:23AM -0400, Chao Shi wrote:
> > If a completion is silently dropped or the device hangs, the calling
> > task wedges with ctrl->scan_lock and ctrl->subsys->lock held, fanning
> > out into hung-task reports on any concurrent open/close/passthru on
> > the same controller:
>
> The IO timeout callbacks that nvme drivers provide are supposed to
> forcefully reclaim any IO no matter what state the device is in. Is that
> not happening for some reason?

Re: [PATCH] nvme: bound the freeze drain in passthrough commands

[Chao S]

Gentle ping.

On Tue, Jun 23, 2026 at 6:28 PM Chao S <coshi036@gmail.com> wrote:
>
> Hi Keith, Christoph,
>
> Both questions land on the same point, so one answer below.
>
> Keith wrote:
> > The IO timeout callbacks that nvme drivers provide are supposed to
> > forcefully reclaim any IO no matter what state the device is in. Is
> > that not happening for some reason?
>
> Christoph wrote:
> > Note that the blocked message itself is not a problem, but around
> > this time we should have done a controller reset and fixed up the
> > issue.  Does that not happen for your test case?
>
> It did happen.  nvme_io_timeout was the default 30, and the relevant
> dmesg from the campaign that produced the hung-task report:
>
>   [ 44.220408] nvme nvme0: I/O tag 451 opcode 0x2 (Read) QID 1 timeout, aborting
>   [ 44.220798] nvme nvme0: I/O tag 819 opcode 0x1 (Write) QID 1
> timeout, aborting
>   [ 44.220820] nvme nvme0: Abort status: 0x0
>   [ 44.221286] nvme nvme0: Abort status: 0x0
>   [ 45.591197] nvme nvme0: resetting controller
>   [ 46.307151] nvme nvme0: IO queues lost
>   [...100s of silence...]
>   [144.561596] INFO: task systemd-udevd:134 blocked for more than 123 seconds.
>
> Timeout fires, abort is accepted, reset starts, reset reaches the
> "IO queues lost" branch (drivers/nvme/host/pci.c).  Then nvme_reset_work
> itself blocks at
>
>   nvme_mark_namespaces_dead -> blk_mark_disk_dead -> blk_report_disk_dead
>     -> bdev_mark_dead(bdev, true) -> sync_blockdev -> folio_wait_writeback
>
> i.e. the unconditional sync_blockdev in bdev_mark_dead's bare-bdev
> else-branch (block/bdev.c) is itself waiting on the writeback that the
> reset was supposed to drain.  The reset_work kworker doesn't show in
> this dmesg because it crosses the 123s threshold ~25s after the
> snapshot console was cut.
>
> So in this report, the IO timeout did its job, but the reset that the
> timeout kicks off cannot complete, and nvme_passthru_start (which is
> already in nvme_wait_freeze at this point) has no way to back out.
>
> Two reasons I still think the bound in passthru_start is worth applying
> on its own merit:
>
> 1. The reset path has several ways to fail to drain in
>    nvme_io_timeout: abort can be rejected, the admin tag for abort
>    can be unavailable, the controller can be wedged before abort
>    lands, an in-progress reset can outlast nvme_io_timeout, or (as
>    here) reset itself can block.  Each leaves nvme_passthru_start
>    waiting forever, holding ctrl->scan_lock + subsys->lock + every
>    namespace's freeze ref, which then fans out on bd_disk->open_mutex
>    via any concurrent bdev_open/release or BLKRRPART.
>
> 2. The same pattern is already established in the tree.  pci shutdown
>    (drivers/nvme/host/pci.c), nvme-tcp reset, nvme-rdma reset,
>    nvme-apple, and Daniel Wagner's 2021 nvme-fc series
>    (20210818120530.130501-1-dwagner@suse.de) all use
>    nvme_wait_freeze_timeout(NVME_IO_TIMEOUT) for exactly this reason.
>    nvme_passthru_start is the only userspace-reachable caller still
>    on the unbounded variant.
>
> Christoph also wrote:
> > So not blocking forever sounds useful, but this might break existing
> > uses.  I guess we could do it based on the O_NONBLOCK flag if people
> > really cared.
>
> NVME_IO_TIMEOUT is already the bound any submitted I/O can be
> synchronously waited on; a freeze drain that legitimately exceeds it
> implies the controller isn't doing useful I/O anyway, and
> nvme_core.io_timeout=N scales both knobs coherently.  Drain on a
> healthy system is sub-second.
>
> Gating on O_NONBLOCK is a reasonable fallback if you'd rather keep
> the old default, but the nvme ioctl path doesn't currently consult
> fd flags, so it would be a new userspace contract.  My concern with
> the current default is that any of the scenarios above lets a
> userspace ioctl wedge a kthread with two ctrl-wide mutexes held; the
> +9-line bound prevents that without changing the success path.
>
> If the maintainer preference is the O_NONBLOCK gate instead, I'll
> respin.
>
> Thanks,
> Chao
>
> On Wed, May 27, 2026 at 11:46 AM Keith Busch <kbusch@kernel.org> wrote:
> >
> > On Wed, May 27, 2026 at 01:59:23AM -0400, Chao Shi wrote:
> > > If a completion is silently dropped or the device hangs, the calling
> > > task wedges with ctrl->scan_lock and ctrl->subsys->lock held, fanning
> > > out into hung-task reports on any concurrent open/close/passthru on
> > > the same controller:
> >
> > The IO timeout callbacks that nvme drivers provide are supposed to
> > forcefully reclaim any IO no matter what state the device is in. Is that
> > not happening for some reason?

Re: [PATCH] nvme: bound the freeze drain in passthrough commands

[Keith Busch]

On Tue, Jun 23, 2026 at 06:28:49PM -0400, Chao S wrote:
> Timeout fires, abort is accepted, reset starts, reset reaches the
> "IO queues lost" branch (drivers/nvme/host/pci.c).  Then nvme_reset_work
> itself blocks at
> 
>   nvme_mark_namespaces_dead -> blk_mark_disk_dead -> blk_report_disk_dead
>     -> bdev_mark_dead(bdev, true) -> sync_blockdev -> folio_wait_writeback
>
> i.e. the unconditional sync_blockdev in bdev_mark_dead's bare-bdev
> else-branch (block/bdev.c) is itself waiting on the writeback that the
> reset was supposed to drain.

But sync_blockdev is *not* unconditional. It's conditional on this not
being a surprise removal, and blk_mark_disk_dead() is considered a
surprise, so I am not sure you've identified the right sequence.
 
> So in this report, the IO timeout did its job, but the reset that the
> timeout kicks off cannot complete, and nvme_passthru_start (which is
> already in nvme_wait_freeze at this point) has no way to back out.

I guess what may have happened is that the timeout handler requeued the
IO  on a quiesced queue so it does reach frozen state. Just move the
unquiesce to before the call to nvme_mark_namespaces_dead(). But I still
don't think it should be needed because nvme_mark_namespaces_dead()
shouldn't block.

> 1. The reset path has several ways to fail to drain in
>    nvme_io_timeout: abort can be rejected, the admin tag for abort
>    can be unavailable, the controller can be wedged before abort
>    lands, an in-progress reset can outlast nvme_io_timeout, or (as
>    here) reset itself can block.  Each leaves nvme_passthru_start
>    waiting forever, holding ctrl->scan_lock + subsys->lock + every
>    namespace's freeze ref, which then fans out on bd_disk->open_mutex
>    via any concurrent bdev_open/release or BLKRRPART.

No, that's not what happens.

If abort fails, then the command times out a 2nd time, then we escalate
to reset.

If we can't get an abort tag, then that means an abort is in progress.
If it doesn't completely timely, then we escalate to reset.

> 2. The same pattern is already established in the tree.  pci shutdown
>    (drivers/nvme/host/pci.c), nvme-tcp reset, nvme-rdma reset,
>    nvme-apple, and Daniel Wagner's 2021 nvme-fc series
>    (20210818120530.130501-1-dwagner@suse.de) all use
>    nvme_wait_freeze_timeout(NVME_IO_TIMEOUT) for exactly this reason.
>    nvme_passthru_start is the only userspace-reachable caller still
>    on the unbounded variant.

Those are for entirely different cases where we're in the last line of
recovery and have to guarantee forward progress.

Re: [PATCH] nvme: bound the freeze drain in passthrough commands

[Chao S]

Get it now, thanks for the detailed suggestions.

On Thu, Jul 9, 2026 at 2:17 PM Keith Busch <kbusch@kernel.org> wrote:
>
> On Tue, Jun 23, 2026 at 06:28:49PM -0400, Chao S wrote:
> > Timeout fires, abort is accepted, reset starts, reset reaches the
> > "IO queues lost" branch (drivers/nvme/host/pci.c).  Then nvme_reset_work
> > itself blocks at
> >
> >   nvme_mark_namespaces_dead -> blk_mark_disk_dead -> blk_report_disk_dead
> >     -> bdev_mark_dead(bdev, true) -> sync_blockdev -> folio_wait_writeback
> >
> > i.e. the unconditional sync_blockdev in bdev_mark_dead's bare-bdev
> > else-branch (block/bdev.c) is itself waiting on the writeback that the
> > reset was supposed to drain.
>
> But sync_blockdev is *not* unconditional. It's conditional on this not
> being a surprise removal, and blk_mark_disk_dead() is considered a
> surprise, so I am not sure you've identified the right sequence.
>
> > So in this report, the IO timeout did its job, but the reset that the
> > timeout kicks off cannot complete, and nvme_passthru_start (which is
> > already in nvme_wait_freeze at this point) has no way to back out.
>
> I guess what may have happened is that the timeout handler requeued the
> IO  on a quiesced queue so it does reach frozen state. Just move the
> unquiesce to before the call to nvme_mark_namespaces_dead(). But I still
> don't think it should be needed because nvme_mark_namespaces_dead()
> shouldn't block.
>
> > 1. The reset path has several ways to fail to drain in
> >    nvme_io_timeout: abort can be rejected, the admin tag for abort
> >    can be unavailable, the controller can be wedged before abort
> >    lands, an in-progress reset can outlast nvme_io_timeout, or (as
> >    here) reset itself can block.  Each leaves nvme_passthru_start
> >    waiting forever, holding ctrl->scan_lock + subsys->lock + every
> >    namespace's freeze ref, which then fans out on bd_disk->open_mutex
> >    via any concurrent bdev_open/release or BLKRRPART.
>
> No, that's not what happens.
>
> If abort fails, then the command times out a 2nd time, then we escalate
> to reset.
>
> If we can't get an abort tag, then that means an abort is in progress.
> If it doesn't completely timely, then we escalate to reset.
>
> > 2. The same pattern is already established in the tree.  pci shutdown
> >    (drivers/nvme/host/pci.c), nvme-tcp reset, nvme-rdma reset,
> >    nvme-apple, and Daniel Wagner's 2021 nvme-fc series
> >    (20210818120530.130501-1-dwagner@suse.de) all use
> >    nvme_wait_freeze_timeout(NVME_IO_TIMEOUT) for exactly this reason.
> >    nvme_passthru_start is the only userspace-reachable caller still
> >    on the unbounded variant.
>
> Those are for entirely different cases where we're in the last line of
> recovery and have to guarantee forward progress.