* [PATCH 6.1.y] nvmet-tcp: fix race between ICReq handling and queue teardown
@ 2026-07-02 12:36 Philo Lu
2026-07-03 4:06 ` Sasha Levin
0 siblings, 1 reply; 2+ messages in thread
From: Philo Lu @ 2026-07-02 12:36 UTC (permalink / raw)
To: stable
Cc: hch, sagi, kch, gregkh, skumar47, kumar.shivam43666, kbusch,
dust.li, linux-nvme, linux-kernel
From: Chaitanya Kulkarni <kch@nvidia.com>
commit 5293a8882c549fab4a878bc76b0b6c951f980a61 upstream.
nvmet_tcp_handle_icreq() updates queue->state after sending an
Initialization Connection Response (ICResp), but it does so without
serializing against target-side queue teardown.
If an NVMe/TCP host sends an Initialization Connection Request
(ICReq) and immediately closes the connection, target-side teardown
may start in softirq context before io_work drains the already
buffered ICReq. In that case, nvmet_tcp_schedule_release_queue()
sets queue->state to NVMET_TCP_Q_DISCONNECTING and drops the queue
reference under state_lock.
If io_work later processes that ICReq, nvmet_tcp_handle_icreq() can
still overwrite the state back to NVMET_TCP_Q_LIVE. That defeats the
DISCONNECTING-state guard in nvmet_tcp_schedule_release_queue() and
allows a later socket state change to re-enter teardown and issue a
second kref_put() on an already released queue.
The ICResp send failure path has the same problem. If teardown has
already moved the queue to DISCONNECTING, a send error can still
overwrite the state with NVMET_TCP_Q_FAILED, again reopening the
window for a second teardown path to drop the queue reference.
Fix this by serializing both post-send state transitions with
state_lock and bailing out if teardown has already started.
Use -ESHUTDOWN as an internal sentinel for that bail-out path rather
than propagating it as a transport error like -ECONNRESET. Keep
nvmet_tcp_socket_error() setting rcv_state to NVMET_TCP_RECV_ERR before
honoring that sentinel so receive-side parsing stays quiesced until the
existing release path completes.
Fixes: c46a6465bac2 ("nvmet-tcp: add NVMe over TCP target driver")
Cc: stable@vger.kernel.org
Reported-by: Shivam Kumar <skumar47@syr.edu>
Tested-by: Shivam Kumar <kumar.shivam43666@gmail.com>
Signed-off-by: Chaitanya Kulkarni <kch@nvidia.com>
Signed-off-by: Keith Busch <kbusch@kernel.org>
[ context diff adaptation: drop `queue->state = NVMET_TCP_Q_FAILED` since
the enum introduced in 6.7, 675b453e0241 ("nvmet-tcp: enable TLS handshake
upcall" ]
Signed-off-by: Philo Lu <lulie@linux.alibaba.com>
---
drivers/nvme/target/tcp.c | 29 ++++++++++++++++++++++++++++-
1 file changed, 28 insertions(+), 1 deletion(-)
diff --git a/drivers/nvme/target/tcp.c b/drivers/nvme/target/tcp.c
index 01d685499b97..a0751ca359f5 100644
--- a/drivers/nvme/target/tcp.c
+++ b/drivers/nvme/target/tcp.c
@@ -353,6 +353,19 @@ static int nvmet_tcp_build_pdu_iovec(struct nvmet_tcp_cmd *cmd)
static void nvmet_tcp_fatal_error(struct nvmet_tcp_queue *queue)
{
+ /*
+ * Keep rcv_state at RECV_ERR even for the internal -ESHUTDOWN path.
+ * nvmet_tcp_handle_icreq() can return -ESHUTDOWN after the ICReq has
+ * already been consumed and queue teardown has started.
+ *
+ * If nvmet_tcp_data_ready() or nvmet_tcp_write_space() queues
+ * nvmet_tcp_io_work() again before nvmet_tcp_release_queue_work()
+ * cancels it, the queue must not keep that old receive state.
+ * Otherwise the next nvmet_tcp_io_work() run can reach
+ * nvmet_tcp_done_recv_pdu() and try to handle the same ICReq again.
+ *
+ * That is why queue->rcv_state needs to be updated before we return.
+ */
queue->rcv_state = NVMET_TCP_RECV_ERR;
if (queue->nvme_sq.ctrl)
nvmet_ctrl_fatal_error(queue->nvme_sq.ctrl);
@@ -896,10 +909,24 @@ static int nvmet_tcp_handle_icreq(struct nvmet_tcp_queue *queue)
iov.iov_base = icresp;
iov.iov_len = sizeof(*icresp);
ret = kernel_sendmsg(queue->sock, &msg, &iov, 1, iov.iov_len);
- if (ret < 0)
+ if (ret < 0) {
+ spin_lock_bh(&queue->state_lock);
+ if (queue->state == NVMET_TCP_Q_DISCONNECTING) {
+ spin_unlock_bh(&queue->state_lock);
+ return -ESHUTDOWN;
+ }
+ spin_unlock_bh(&queue->state_lock);
return ret; /* queue removal will cleanup */
+ }
+ spin_lock_bh(&queue->state_lock);
+ if (queue->state == NVMET_TCP_Q_DISCONNECTING) {
+ spin_unlock_bh(&queue->state_lock);
+ /* Tell nvmet_tcp_socket_error() teardown is in progress. */
+ return -ESHUTDOWN;
+ }
queue->state = NVMET_TCP_Q_LIVE;
+ spin_unlock_bh(&queue->state_lock);
nvmet_prepare_receive_pdu(queue);
return 0;
}
--
2.47.3
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH 6.1.y] nvmet-tcp: fix race between ICReq handling and queue teardown
2026-07-02 12:36 [PATCH 6.1.y] nvmet-tcp: fix race between ICReq handling and queue teardown Philo Lu
@ 2026-07-03 4:06 ` Sasha Levin
0 siblings, 0 replies; 2+ messages in thread
From: Sasha Levin @ 2026-07-03 4:06 UTC (permalink / raw)
To: stable
Cc: Sasha Levin, hch, sagi, kch, gregkh, skumar47, kumar.shivam43666,
kbusch, dust.li, linux-nvme, linux-kernel, Philo Lu
On Thu, Jul 02, 2026 at 08:36:44PM +0800, Philo Lu wrote:
> From: Chaitanya Kulkarni <kch@nvidia.com>
>
> commit 5293a8882c549fab4a878bc76b0b6c951f980a61 upstream.
>
> nvmet_tcp_handle_icreq() updates queue->state after sending an
> Initialization Connection Response (ICResp), but it does so without
> serializing against target-side queue teardown.
Queued up for 6.1.y, thanks!
--
Thanks,
Sasha
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2026-07-03 4:07 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-02 12:36 [PATCH 6.1.y] nvmet-tcp: fix race between ICReq handling and queue teardown Philo Lu
2026-07-03 4:06 ` Sasha Levin
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox