Re: [PATCH] nvmet: Check the admin queue before reading its TLS key in debugfs

[Guixin Liu <kanie@linux.alibaba.com>]

在 2026/10/28 15:42, Hannes Reinecke 写道:
> On 10/28/26 7:44 AM, Guixin Liu wrote:
>> nvmet_ctrl_tls_key_show() dereferences ctrl->sqs[0] unconditionally,
>> but the admin queue is not installed yet while the controller is
>> being created, and sqs[0] is set to NULL again when it is destroyed.
>> Reading the tls_key debugfs entry in either window crashes the kernel.
>>
>> Only query the queue's TLS key when the admin queue is present and
>> report 0 otherwise.
>>
>> Fixes: 316dabe6089fe ("nvmet: add tls_concat and tls_key debugfs entries")
>> Cc: stable@vger.kernel.org
>> Signed-off-by: Guixin Liu <kanie@linux.alibaba.com>
>> ---
>>   drivers/nvme/target/debugfs.c | 4 +++-
>>   1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/nvme/target/debugfs.c b/drivers/nvme/target/debugfs.c
>> index e85fe1d4c9f8..a27a501fe61a 100644
>> --- a/drivers/nvme/target/debugfs.c
>> +++ b/drivers/nvme/target/debugfs.c
>> @@ -136,8 +136,10 @@ NVMET_DEBUGFS_ATTR(nvmet_ctrl_host_traddr);
>>   static int nvmet_ctrl_tls_key_show(struct seq_file *m, void *p)
>>   {
>>       struct nvmet_ctrl *ctrl = m->private;
>> -    key_serial_t keyid = nvmet_queue_tls_keyid(ctrl->sqs[0]);
>> +    key_serial_t keyid = 0;
>>   +    if (ctrl->sqs[0])
>> +        keyid = nvmet_queue_tls_keyid(ctrl->sqs[0]);
>>       seq_printf(m, "%08x\n", keyid);
>>       return 0;
>>   }
>
> I think it might be easier if 'nvmet_queue_tls_keyid()' would accept
> a NULL argument.
This is simpler, however Sashiko's review points out that after the
NULL check, sq->tls_key clould be freed by nvmet_sq_put_tls_key(),
or sq itself could be destroyed. 

My idea is store admin queue's tls_key to nvmet_ctrl to prevent access
sq and sq->tls_key in debugfs interface, what do you think?

Best Regards,
Guixin Liu
>
> Cheers,
>
> Hannes