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

[Guixin Liu]

在 2026/11/6 15:31, 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
Right, I miss that, sorry, remove this in v2.

Best Regards,
Guixin Liu