[PATCH 2/2] nvme-tcp: remove tag set when second admin queue config fails

[Hannes Reinecke]

On 6/2/25 06:35, Shin'ichiro Kawasaki wrote:
> Commit 104d0e2f6222 ("nvme-fabrics: reset admin connection for secure
> concatenation") modified nvme_tcp_setup_ctrl() to call
> nvme_tcp_configure_admin_queue() twice. The first call prepares for
> DH-CHAP negotitation, and the second call is required for secure
> concatenation. However, this change triggered BUG KASAN slab-use-after-
> free in blk_mq_queue_tag_busy_iter(). This BUG can be recreated by
> repeating the blktests test case nvme/063 a few times [1].
> 
> When the BUG happens, nvme_tcp_create_ctrl() fails in the call chain
> below:
> 
> nvme_tcp_create_ctrl()
>   nvme_tcp_alloc_ctrl() new=true             ... Alloc nvme_tcp_ctrl and admin_tag_set
>   nvme_tcp_setup_ctrl() new=true
>    nvme_tcp_configure_admin_queue() new=true ... Succeed
>     nvme_alloc_admin_tag_set()               ... Alloc the tag set for admin_tag_set
>    nvme_stop_keep_alive()
>    nvme_tcp_teardown_admin_queue() remove=false
>    nvme_tcp_configure_admin_queue() new=false
>     nvme_tcp_alloc_admin_queue()             ... Fail, but do not call nvme_remove_admin_tag_set()
>   nvme_uninit_ctrl()
>   nvme_put_ctrl()                            ... Free up the nvme_tcp_ctrl and admin_tag_set
> 
> The first call of nvme_tcp_configure_admin_queue() succeeds with
> new=true argument. The second call fails with new=false argument. This
> second call does not call nvme_remove_admin_tag_set() on failure, due to
> the new=false argument. Then the admin tag set is not removed. However,
> nvme_tcp_create_ctrl() assumes that nvme_tcp_setup_ctrl() would call
> nvme_remove_admin_tag_set(). Then it frees up struct nvme_tcp_ctrl which
> has admin_tag_set field. Later on, the timeout handler accesses the
> admin_tag_set field and causes the BUG KASAN slab-use-after-free.
> 
> To not leave the admin tag set, call nvme_remove_admin_tag_set() when
> the second nvme_tcp_configure_admin_queue() call fails. Do not return
> from nvme_tcp_setup_ctrl() on failure. Instead, jump to "destroy_admin"
> go-to label to call nvme_tcp_teardown_admin_queue() which calls
> nvme_remove_admin_tag_set().
> 
> Fixes: 104d0e2f6222 ("nvme-fabrics: reset admin connection for secure concatenation")
> Cc: stable at vger.kernel.org
> Link: https://lore.kernel.org/linux-nvme/6mhxskdlbo6fk6hotsffvwriauurqky33dfb3s44mqtr5dsxmf@gywwmnyh3twm/ [1]
> Signed-off-by: Shin'ichiro Kawasaki <shinichiro.kawasaki at wdc.com>
> ---
>   drivers/nvme/host/tcp.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/nvme/host/tcp.c b/drivers/nvme/host/tcp.c
> index 14b9d67329b3..85acb4c9c977 100644
> --- a/drivers/nvme/host/tcp.c
> +++ b/drivers/nvme/host/tcp.c
> @@ -2350,7 +2350,7 @@ static int nvme_tcp_setup_ctrl(struct nvme_ctrl *ctrl, bool new)
>   		nvme_tcp_teardown_admin_queue(ctrl, false);
>   		ret = nvme_tcp_configure_admin_queue(ctrl, false);
>   		if (ret)
> -			return ret;
> +			goto destroy_admin;
>   	}
>   
>   	if (ctrl->icdoff) {

Reviewed-by: Hannes Reinecke <hare at suse.de>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke                  Kernel Storage Architect
hare at 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