Linux-NVME Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] nvmet-tcp: fix a hang on queue teardown with data digest
@ 2026-09-01  1:47 Shivam Kumar
  2026-09-07 23:11 ` Shivam Kumar
                   ` (2 more replies)
  0 siblings, 3 replies; 5+ messages in thread
From: Shivam Kumar @ 2026-09-01  1:47 UTC (permalink / raw)
  To: linux-nvme
  Cc: Greg KH, security, Christoph Hellwig, Sagi Grimberg,
	Chaitanya Kulkarni, Keith Busch, stable, Shivam Kumar

With data digest on, a command that has received all its data waits for
the digest in NVMET_TCP_RECV_DDGST. has_data_in() is already false there,
so need_data_in() is too, and nvmet_tcp_uninit_data_in_cmds() skips it on
teardown even though it still holds the nvmet_req_init() reference. The SQ
percpu_ref never drains, nvmet_sq_destroy() blocks forever in
wait_for_completion(), and the nvmet-wq release worker is stuck.

An unauthenticated host on an allow_any_host subsystem hits this by
negotiating data digest, sending a write's data but not the trailing
digest, and closing the connection. Each leaked command wedges a release
worker; a few stall queue teardown entirely, and with hung_task_panic the
box goes down.

Drop the reference for a command left in RECV_DDGST from
nvmet_tcp_release_queue_work(), before rcv_state is cleared. A command
that failed nvmet_req_init() never took one, so skip it.

Fixes: 872d26a391da ("nvmet-tcp: add NVMe over TCP target driver")
Cc: stable@vger.kernel.org
Signed-off-by: Shivam Kumar <kumar.shivam43666@gmail.com>
Assisted-by: Claude
---
 drivers/nvme/target/tcp.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

--- a/drivers/nvme/target/tcp.c
+++ b/drivers/nvme/target/tcp.c
@@ -1640,6 +1640,19 @@
 	nvmet_tcp_restore_socket_callbacks(queue);
 	cancel_delayed_work_sync(&queue->tls_handshake_tmo_work);
 	cancel_work_sync(&queue->io_work);
+
+	/*
+	 * A command that has received all of its data and is only waiting
+	 * for the data digest sits in RECV_DDGST: need_data_in() is already
+	 * false, so nvmet_tcp_uninit_data_in_cmds() below skips it, yet it
+	 * still holds the reference from nvmet_req_init(). Drop it here,
+	 * while rcv_state still reflects it, so the SQ percpu_ref can drain
+	 * and nvmet_sq_destroy() can complete. INIT_FAILED took no ref.
+	 */
+	if (queue->rcv_state == NVMET_TCP_RECV_DDGST && queue->cmd &&
+	    !(queue->cmd->flags & NVMET_TCP_F_INIT_FAILED))
+		nvmet_req_uninit(&queue->cmd->req);
+
 	/* stop accepting incoming data */
 	queue->rcv_state = NVMET_TCP_RECV_ERR;
 
-- 
2.34.1


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] nvmet-tcp: fix a hang on queue teardown with data digest
  2026-09-01  1:47 [PATCH] nvmet-tcp: fix a hang on queue teardown with data digest Shivam Kumar
@ 2026-09-07 23:11 ` Shivam Kumar
  2026-09-10 22:32 ` Keith Busch
  2026-09-11 21:15 ` Sagi Grimberg
  2 siblings, 0 replies; 5+ messages in thread
From: Shivam Kumar @ 2026-09-07 23:11 UTC (permalink / raw)
  To: linux-nvme
  Cc: Greg KH, security, Christoph Hellwig, Sagi Grimberg,
	Chaitanya Kulkarni, Keith Busch, stable

On Mon, Aug 31, 2026 at 9:47 PM Shivam Kumar
<kumar.shivam43666@gmail.com> wrote:
>
> With data digest on, a command that has received all its data waits for
> the digest in NVMET_TCP_RECV_DDGST. has_data_in() is already false there,
> so need_data_in() is too, and nvmet_tcp_uninit_data_in_cmds() skips it on
> teardown even though it still holds the nvmet_req_init() reference. The SQ
> percpu_ref never drains, nvmet_sq_destroy() blocks forever in
> wait_for_completion(), and the nvmet-wq release worker is stuck.
>
> An unauthenticated host on an allow_any_host subsystem hits this by
> negotiating data digest, sending a write's data but not the trailing
> digest, and closing the connection. Each leaked command wedges a release
> worker; a few stall queue teardown entirely, and with hung_task_panic the
> box goes down.
>
> Drop the reference for a command left in RECV_DDGST from
> nvmet_tcp_release_queue_work(), before rcv_state is cleared. A command
> that failed nvmet_req_init() never took one, so skip it.
>
> Fixes: 872d26a391da ("nvmet-tcp: add NVMe over TCP target driver")
> Cc: stable@vger.kernel.org
> Signed-off-by: Shivam Kumar <kumar.shivam43666@gmail.com>
> Assisted-by: Claude
> ---
>  drivers/nvme/target/tcp.c | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
>
> --- a/drivers/nvme/target/tcp.c
> +++ b/drivers/nvme/target/tcp.c
> @@ -1640,6 +1640,19 @@
>         nvmet_tcp_restore_socket_callbacks(queue);
>         cancel_delayed_work_sync(&queue->tls_handshake_tmo_work);
>         cancel_work_sync(&queue->io_work);
> +
> +       /*
> +        * A command that has received all of its data and is only waiting
> +        * for the data digest sits in RECV_DDGST: need_data_in() is already
> +        * false, so nvmet_tcp_uninit_data_in_cmds() below skips it, yet it
> +        * still holds the reference from nvmet_req_init(). Drop it here,
> +        * while rcv_state still reflects it, so the SQ percpu_ref can drain
> +        * and nvmet_sq_destroy() can complete. INIT_FAILED took no ref.
> +        */
> +       if (queue->rcv_state == NVMET_TCP_RECV_DDGST && queue->cmd &&
> +           !(queue->cmd->flags & NVMET_TCP_F_INIT_FAILED))
> +               nvmet_req_uninit(&queue->cmd->req);
> +
>         /* stop accepting incoming data */
>         queue->rcv_state = NVMET_TCP_RECV_ERR;
>
> --
> 2.34.1

Gentle ping on this one.


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] nvmet-tcp: fix a hang on queue teardown with data digest
  2026-09-01  1:47 [PATCH] nvmet-tcp: fix a hang on queue teardown with data digest Shivam Kumar
  2026-09-07 23:11 ` Shivam Kumar
@ 2026-09-10 22:32 ` Keith Busch
  2026-09-11 21:15 ` Sagi Grimberg
  2 siblings, 0 replies; 5+ messages in thread
From: Keith Busch @ 2026-09-10 22:32 UTC (permalink / raw)
  To: Shivam Kumar
  Cc: linux-nvme, Greg KH, security, Christoph Hellwig, Sagi Grimberg,
	Chaitanya Kulkarni, stable

On Mon, Aug 31, 2026 at 09:47:18PM -0400, Shivam Kumar wrote:
> With data digest on, a command that has received all its data waits for
> the digest in NVMET_TCP_RECV_DDGST. has_data_in() is already false there,
> so need_data_in() is too, and nvmet_tcp_uninit_data_in_cmds() skips it on
> teardown even though it still holds the nvmet_req_init() reference. The SQ
> percpu_ref never drains, nvmet_sq_destroy() blocks forever in
> wait_for_completion(), and the nvmet-wq release worker is stuck.
> 
> An unauthenticated host on an allow_any_host subsystem hits this by
> negotiating data digest, sending a write's data but not the trailing
> digest, and closing the connection. Each leaked command wedges a release
> worker; a few stall queue teardown entirely, and with hung_task_panic the
> box goes down.
> 
> Drop the reference for a command left in RECV_DDGST from
> nvmet_tcp_release_queue_work(), before rcv_state is cleared. A command
> that failed nvmet_req_init() never took one, so skip it.

"I have made this letter longer than usual because I lack the time to
make it shorter." - Blaise Pascal

Take some time to actually write your own (and hopefully more concise)
message to demonstrate you understand what you're changing. AI messages
are overly verbose.

> +	/*
> +	 * A command that has received all of its data and is only waiting
> +	 * for the data digest sits in RECV_DDGST: need_data_in() is already
> +	 * false, so nvmet_tcp_uninit_data_in_cmds() below skips it, yet it
> +	 * still holds the reference from nvmet_req_init(). Drop it here,
> +	 * while rcv_state still reflects it, so the SQ percpu_ref can drain
> +	 * and nvmet_sq_destroy() can complete. INIT_FAILED took no ref.
> +	 */

Same here.

> +	if (queue->rcv_state == NVMET_TCP_RECV_DDGST && queue->cmd &&
> +	    !(queue->cmd->flags & NVMET_TCP_F_INIT_FAILED))
> +		nvmet_req_uninit(&queue->cmd->req);

I don't think this criteria is even correct. RECV_DDGST doesn't mean the
command has all its data since that state is per-PDU.
nvmet_tcp_try_recv_data() enters that state at every H2C boundary, not
just the last one.


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] nvmet-tcp: fix a hang on queue teardown with data digest
  2026-09-01  1:47 [PATCH] nvmet-tcp: fix a hang on queue teardown with data digest Shivam Kumar
  2026-09-07 23:11 ` Shivam Kumar
  2026-09-10 22:32 ` Keith Busch
@ 2026-09-11 21:15 ` Sagi Grimberg
  2026-09-12  5:39   ` [PATCH v2] " Shivam Kumar
  2 siblings, 1 reply; 5+ messages in thread
From: Sagi Grimberg @ 2026-09-11 21:15 UTC (permalink / raw)
  To: Shivam Kumar, linux-nvme
  Cc: Greg KH, security, Christoph Hellwig, Chaitanya Kulkarni,
	Keith Busch, stable



On 01/09/2026 4:47, Shivam Kumar wrote:
> With data digest on, a command that has received all its data waits for
> the digest in NVMET_TCP_RECV_DDGST. has_data_in() is already false there,
> so need_data_in() is too, and nvmet_tcp_uninit_data_in_cmds() skips it on
> teardown even though it still holds the nvmet_req_init() reference.

So nvmet_tcp_uninit_data_in_cmds  is where this should be addressed.


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH v2] nvmet-tcp: fix a hang on queue teardown with data digest
  2026-09-11 21:15 ` Sagi Grimberg
@ 2026-09-12  5:39   ` Shivam Kumar
  0 siblings, 0 replies; 5+ messages in thread
From: Shivam Kumar @ 2026-09-12  5:39 UTC (permalink / raw)
  To: linux-nvme, Sagi Grimberg, Keith Busch, Christoph Hellwig,
	Chaitanya Kulkarni
  Cc: Greg KH, security, stable, Shivam Kumar

A command that has received all its data but whose data digest is still
outstanding is stalled in NVMET_TCP_RECV_DDGST, where
nvmet_tcp_need_data_in() is false. nvmet_tcp_uninit_data_in_cmds()
therefore skips it during teardown, leaking its submission queue
reference, and nvmet_sq_destroy() then blocks forever waiting on it.

Release such a command in nvmet_tcp_uninit_data_in_cmds(), and reset
rcv_state after that call.

Fixes: 872d26a391da ("nvmet-tcp: add NVMe over TCP target driver")
Cc: stable@vger.kernel.org
Signed-off-by: Shivam Kumar <kumar.shivam43666@gmail.com>
---
 drivers/nvme/target/tcp.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/nvme/target/tcp.c b/drivers/nvme/target/tcp.c
index e59810175262..3bf344b184e8 100644
--- a/drivers/nvme/target/tcp.c
+++ b/drivers/nvme/target/tcp.c
@@ -1608,6 +1608,11 @@ static void nvmet_tcp_uninit_data_in_cmds(struct nvmet_tcp_queue *queue)
 			nvmet_req_uninit(&cmd->req);
 	}
 
+	if (queue->rcv_state == NVMET_TCP_RECV_DDGST && queue->cmd &&
+	    !nvmet_tcp_need_data_in(queue->cmd) &&
+	    !(queue->cmd->flags & NVMET_TCP_F_INIT_FAILED))
+		nvmet_req_uninit(&queue->cmd->req);
+
 	if (!queue->nr_cmds && nvmet_tcp_need_data_in(&queue->connect)) {
 		/* failed in connect */
 		nvmet_req_uninit(&queue->connect.req);
@@ -1636,11 +1641,12 @@ static void nvmet_tcp_release_queue_work(struct work_struct *w)
 	nvmet_tcp_restore_socket_callbacks(queue);
 	cancel_delayed_work_sync(&queue->tls_handshake_tmo_work);
 	cancel_work_sync(&queue->io_work);
-	/* stop accepting incoming data */
-	queue->rcv_state = NVMET_TCP_RECV_ERR;
 
 	nvmet_sq_put_tls_key(&queue->nvme_sq);
 	nvmet_tcp_uninit_data_in_cmds(queue);
+	/* stop accepting incoming data */
+	queue->rcv_state = NVMET_TCP_RECV_ERR;
+
 	nvmet_sq_destroy(&queue->nvme_sq);
 	nvmet_cq_put(&queue->nvme_cq);
 	cancel_work_sync(&queue->io_work);
-- 
2.53.0



^ permalink raw reply related	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2026-09-12  5:39 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-09-01  1:47 [PATCH] nvmet-tcp: fix a hang on queue teardown with data digest Shivam Kumar
2026-09-07 23:11 ` Shivam Kumar
2026-09-10 22:32 ` Keith Busch
2026-09-11 21:15 ` Sagi Grimberg
2026-09-12  5:39   ` [PATCH v2] " Shivam Kumar

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox