Re: [PATCH v3] nvme-tcp: fix NULL queue->ctrl dereference on reconnect

[Hannes Reinecke <hare@suse.de>]

On 10/28/26 3:33 PM, Christian Aurich Zanettini Martins wrote:
> 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 and call it for every queue when allocating the
> controller. Such requests are then requeued like on any other queue
> that is not live. send_mutex and io_work move there as well, and
> nvme_tcp_exit_queue() destroys the helper's mutexes when the controller
> is freed.
> 
> Fixes: 61b0ed3be81b ("nvme-tcp: parallelize I/O queue allocation and startup")
> Suggested-by: Keith Busch <kbusch@kernel.org>
> Reviewed-by: Nilay Shroff <nilay@linux.ibm.com>
> Signed-off-by: Christian Aurich Zanettini Martins <christian.aurichzm@gmail.com>
> ---
> v3:
> - Add nvme_tcp_exit_queue() for the mutex_destroy() calls in
>    nvme_tcp_free_ctrl() (Hannes)
> - Keep Nilay's Reviewed-by, as the change is mechanical
> 
> v2:
> - Rebase on Guixin's v4 "nvme-tcp: Validate the queue state under the
>    queue lock in the address attribute" and move its queue_lock init
>    into the helper
> - Also move send_mutex and io_work into the helper, and destroy
>    send_mutex and pf_cache_lock next to queue_lock in
>    nvme_tcp_free_ctrl() (Keith)
> - Left in nvme_tcp_alloc_queue(): req_list and send_list, as resetting
>    them drops entries left over from the previous connection, and the
>    lockdep keys, which are per socket instance since commit 19bdb70c77d3
>    ("nvme-tcp: lockdep: use dynamic lockdep keys per socket instance")
> 
> Based on nvme-7.4 (9afc1b71d963) with "nvme-tcp: fix request PDU size
> when header digest is enabled" and Guixin's v4 on top. The last hunk
> assumes the conflict between those two in nvme_tcp_alloc_ctrl() is
> resolved by keeping both loop bodies and both comments. I can resend on
> top of nvme-7.4 once both are there.
> 
> 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. Without the patch the host oopses as above by the
> second try; with v3, 8 of 8 tries go live, with and without lockdep.
> 30 resets under digest I/O, with a reader looping on the address
> attribute, also pass and are clean with lockdep.
> 
>   drivers/nvme/host/tcp.c | 64 ++++++++++++++++++++++-------------------
>   1 file changed, 35 insertions(+), 29 deletions(-)
> 
Reviewed-by: Hannes Reinecke <hare@kernel.org>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke                  Kernel Storage Architect
hare@suse.de                                +49 911 74053 688
SUSE Software Solutions GmbH, Frankenstr. 146, 90461 Nürnberg
HRB 36809 (AG Nürnberg), GF: I. Totev, A. McDonald, W. Knoblich