mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH] nvme-tcp: fix NULL queue->ctrl dereference on reconnect
@ 2026-10-28  0:26 Christian Aurich Zanettini Martins
  2026-10-28  1:48 ` Keith Busch
  0 siblings, 1 reply; 2+ messages in thread
From: Christian Aurich Zanettini Martins @ 2026-10-28  0:26 UTC (permalink / raw)
  To: linux-nvme
  Cc: kbusch, axboe, hch, sagi, hare, sgogte, wagi, linux-kernel,
	Christian Aurich Zanettini Martins

Since commit 61b0ed3be81b ("nvme-tcp: parallelize I/O queue allocation
and startup"), a reconnect that gets more I/O queues than before grows
the tag set before the new queues are allocated. queue->ctrl is only
set by nvme_tcp_alloc_queue(), so I/O submitted in between reaches
nvme_tcp_queue_rq() on a queue that was never allocated and
dereferences a NULL queue->ctrl:

  BUG: kernel NULL pointer dereference, address: 00000000000002ac
  RIP: 0010:nvme_tcp_queue_rq+0x33/0x3b0

Add nvme_tcp_init_queue() for the queue fields that do not depend on
the connection (ctrl, pf_cache_lock and both digests) and call it for
every queue when allocating the controller. Such requests are then
requeued like on any other queue that is not live.

Fixes: 61b0ed3be81b ("nvme-tcp: parallelize I/O queue allocation and startup")
Suggested-by: Keith Busch <kbusch@kernel.org>
Signed-off-by: Christian Aurich Zanettini Martins <christian.aurichzm@gmail.com>
---
Keith suggested the helper when applying "nvme-tcp: fix request PDU
size when header digest is enabled". Based on nvme-7.4 (9afc1b71d963)
with that patch on top.

Reproduced in QEMU with nvmet-tcp over loopback and
CONFIG_NVME_MULTIPATH=n: connect with attr_qid_max=2, keep direct reads
running on the CPUs that map to the new hw queues, then raise
attr_qid_max to 4. Before the patch the host oopsed as above within two
tries in each of three boots; after it, 8 of 8 tries went live. Digest
I/O, the 2 to 4 queue grow and 30 resets under I/O also pass, and the
grow and reset runs are clean with lockdep enabled.

 drivers/nvme/host/tcp.c | 31 ++++++++++++++++++-------------
 1 file changed, 18 insertions(+), 13 deletions(-)

diff --git a/drivers/nvme/host/tcp.c b/drivers/nvme/host/tcp.c
index 191e5811d25e..46816a9fb2bf 100644
--- a/drivers/nvme/host/tcp.c
+++ b/drivers/nvme/host/tcp.c
@@ -1977,6 +1977,22 @@ static int nvme_tcp_start_tls(struct nvme_ctrl *nctrl,
 	return ret;
 }
 
+/*
+ * The I/O tag set is allocated, and grown on reconnect, before the queues are
+ * connected, so blk-mq can initialize requests on a queue, and dispatch to it,
+ * before nvme_tcp_alloc_queue() has run for it. Set up the fields that do not
+ * depend on the connection once, when allocating the controller.
+ */
+static void nvme_tcp_init_queue(struct nvme_tcp_ctrl *ctrl, int qid)
+{
+	struct nvme_tcp_queue *queue = &ctrl->queues[qid];
+
+	queue->ctrl = ctrl;
+	mutex_init(&queue->pf_cache_lock);
+	queue->hdr_digest = ctrl->ctrl.opts->hdr_digest;
+	queue->data_digest = ctrl->ctrl.opts->data_digest;
+}
+
 static int nvme_tcp_alloc_queue(struct nvme_ctrl *nctrl, int qid,
 				key_serial_t pskid)
 {
@@ -1986,7 +2002,6 @@ static int nvme_tcp_alloc_queue(struct nvme_ctrl *nctrl, int qid,
 	struct file *sock_file;
 
 	mutex_init(&queue->queue_lock);
-	queue->ctrl = ctrl;
 	init_llist_head(&queue->req_list);
 	INIT_LIST_HEAD(&queue->send_list);
 	mutex_init(&queue->send_mutex);
@@ -2079,8 +2094,6 @@ static int nvme_tcp_alloc_queue(struct nvme_ctrl *nctrl, int qid,
 		}
 	}
 
-	queue->data_digest = nctrl->opts->data_digest;
-
 	rcv_pdu_size = sizeof(struct nvme_tcp_rsp_pdu) +
 			nvme_tcp_hdgst_len(queue);
 	queue->pdu = kmalloc(rcv_pdu_size, GFP_KERNEL);
@@ -3225,16 +3238,8 @@ static struct nvme_tcp_ctrl *nvme_tcp_alloc_ctrl(struct device *dev,
 		goto out_free_ctrl;
 	}
 
-	/*
-	 * The I/O tag set is allocated before the I/O queues, and initializing
-	 * its requests already allocates the PDUs from the page frag caches of
-	 * the queues, sized by their header digest setting, so set up the
-	 * locks and hdr_digest here.
-	 */
-	for (i = 0; i < ctrl->ctrl.queue_count; i++) {
-		mutex_init(&ctrl->queues[i].pf_cache_lock);
-		ctrl->queues[i].hdr_digest = opts->hdr_digest;
-	}
+	for (i = 0; i < ctrl->ctrl.queue_count; i++)
+		nvme_tcp_init_queue(ctrl, i);
 
 	ctrl->net = get_net(current->nsproxy->net_ns);
 
-- 
2.43.0


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

* Re: [PATCH] nvme-tcp: fix NULL queue->ctrl dereference on reconnect
  2026-10-28  0:26 [PATCH] nvme-tcp: fix NULL queue->ctrl dereference on reconnect Christian Aurich Zanettini Martins
@ 2026-10-28  1:48 ` Keith Busch
  0 siblings, 0 replies; 2+ messages in thread
From: Keith Busch @ 2026-10-28  1:48 UTC (permalink / raw)
  To: Christian Aurich Zanettini Martins
  Cc: linux-nvme, axboe, hch, sagi, hare, sgogte, wagi, linux-kernel

On Tue, Oct 27, 2026 at 09:26:54PM -0300, Christian Aurich Zanettini Martins wrote:
> +static void nvme_tcp_init_queue(struct nvme_tcp_ctrl *ctrl, int qid)
> +{
> +	struct nvme_tcp_queue *queue = &ctrl->queues[qid];
> +
> +	queue->ctrl = ctrl;
> +	mutex_init(&queue->pf_cache_lock);
> +	queue->hdr_digest = ctrl->ctrl.opts->hdr_digest;
> +	queue->data_digest = ctrl->ctrl.opts->data_digest;

It's a good start, but if we're introducing an init helper, let's take
it all the way! The queue_lock at least is something Guixin has in a
conflicting patch here:

https://lore.kernel.org/linux-nvme/20261027123950.3122316-1-kanie@linux.alibaba.com/

Other fields including io_work and send_mutex look safe to move here too.

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

end of thread, other threads:[~2026-10-28  1:48 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-10-28  0:26 [PATCH] nvme-tcp: fix NULL queue->ctrl dereference on reconnect Christian Aurich Zanettini Martins
2026-10-28  1:48 ` Keith Busch

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®