[PATCH 5/6] nvmet: fix controller reference leak in nvmet_install_queue() error paths

[Hannes Reinecke]

On 11/6/26 2:58 AM, Guixin Liu wrote:
> nvmet_install_queue() takes a controller reference for req->cq->ctrl
> and installs the controller in both req->sq->ctrl and req->cq->ctrl,
> but neither error path undoes its work: the busy return after the
> failed cq->ctrl cmpxchg leaves sq->ctrl set and the reference taken,
> and the err path for a failed transport ->install_queue() clears
> sq->ctrl but leaves cq->ctrl set with the reference still held.
> 
> The queue is then permanently wedged: a connect retry finds the stale
> cq->ctrl, takes another reference and fails again, and the leaked
> references keep the controller alive forever. Clearing cq->ctrl also
> matters because the caller drops its own reference, which can be the
> last one on the admin connect path, leaving cq->ctrl dangling.
> 
> Roll back the sq->ctrl assignment and drop the reference on both
> error paths, and also clear ctrl->cqs[qid] on the err path to mirror
> what nvmet_cq_destroy() would do.
> 
> Fixes: bb78836b3a7ca ("nvmet: fabrics: add CQ init and destroy")
> Cc: stable at vger.kernel.org
> Signed-off-by: Guixin Liu <kanie at linux.alibaba.com>
> ---
>   drivers/nvme/target/fabrics-cmd.c | 5 +++++
>   1 file changed, 5 insertions(+)
> 
> diff --git a/drivers/nvme/target/fabrics-cmd.c b/drivers/nvme/target/fabrics-cmd.c
> index fc626c2fd2c9..773dad857a35 100644
> --- a/drivers/nvme/target/fabrics-cmd.c
> +++ b/drivers/nvme/target/fabrics-cmd.c
> @@ -216,6 +216,8 @@ static u16 nvmet_install_queue(struct nvmet_ctrl *ctrl, struct nvmet_req *req)
>   	if (old) {
>   		pr_warn("queue already connected!\n");
>   		req->error_loc = offsetof(struct nvmf_connect_command, opcode);
> +		req->sq->ctrl = NULL;
> +		nvmet_ctrl_put(ctrl);
>   		return NVME_SC_CONNECT_CTRL_BUSY | NVME_STATUS_DNR;
>   	}
>   
> @@ -234,6 +236,7 @@ static u16 nvmet_install_queue(struct nvmet_ctrl *ctrl, struct nvmet_req *req)
>   			pr_err("failed to install queue %d cntlid %d ret %x\n",
>   				qid, ctrl->cntlid, ret);
>   			ctrl->sqs[qid] = NULL;
> +			ctrl->cqs[qid] = NULL;
>   			goto err;
>   		}
>   	}
> @@ -242,6 +245,8 @@ static u16 nvmet_install_queue(struct nvmet_ctrl *ctrl, struct nvmet_req *req)
>   
>   err:
>   	req->sq->ctrl = NULL;
> +	req->cq->ctrl = NULL;
> +	nvmet_ctrl_put(ctrl);
>   	return ret;
>   }
>   

That looks wrong.
We already do a call to 'nvmet_ctrl_put()' in 
'nvmet_execute_admin_connect()', right after nvmet_install_queue()
failed.

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