[PATCH] nvme-core: fix oops in dhchap_ctrl_secret_show()

[Hannes Reinecke]

On 6/2/22 10:51, Sagi Grimberg wrote:
> 
>> In current implementation when we try to disconnect the subsystem
>> that doesn't exists nvme_ctrl_dhchap_ctrl_secrete_show() ends up in
>> OOPS:-
>>
>> RIP: 0010:nvme_ctrl_dhchap_ctrl_secret_show+0x16/0x40 [nvme_core]
>> Code: c6 78 a1 16 c0 e8 5a ad 2d c1 48 98 c3 0f 1f 80 00 00 00 00 0f 
>> 1f 44 00 00 49 89 f8 48 89 d7 49 8b 40 78 48 8b 80 98 0b 00 00 <48> 8b 
>> 50 68 48 85 d2 74 0f 48 c7 c6 46 a0 16 c0 e8 25 ad 2d c1 48
>> RSP: 0018:ffffc90000f3fdb8 EFLAGS: 00010282
>> RAX: 0000000000000000 RBX: ffffffffc01731e0 RCX: 0000000000000000
>> RDX: ffff888104b44000 RSI: ffffffffc01731e0 RDI: ffff888104b44000
>> RBP: ffffffff822f7380 R08: ffff88810274a2b8 R09: ffff8881782fbdb8
>> R10: 0000000000020000 R11: 0000000000000000 R12: ffffc90000f3fe80
>> R13: ffffc90000f3fe58 R14: ffff88817f0944d8 R15: 0000000000000001
>>   ? nvme_subsys_show_type+0x50/0x50 [nvme_core]
>>   dev_attr_show+0x19/0x40
>>   sysfs_kf_seq_show+0xa8/0xe0
>>   seq_read_iter+0x11c/0x4b0
>>   new_sync_read+0x106/0x180
>>   vfs_read+0x138/0x190
>>   ksys_read+0x5f/0xe0
>>   do_syscall_64+0x3b/0x90
>>   entry_SYSCALL_64_after_hwframe+0x44/0xae
>> RIP: 0033:0x7fdeccbdf702
>> Code: c0 e9 b2 fe ff ff 50 48 8d 3d ca 2a 0a 00 e8 b5 e8 01 00 0f 1f 
>> 44 00 00 f3 0f 1e fa 64 8b 04 25 18 00 00 00 85 c0 75 10 0f 05 <48> 3d 
>> 00 f0 ff ff 77 56 c3 0f 1f 44 00 00 48 83 ec 28 48 89 54 24
>> RSP: 002b:00007ffe0dfb4f28 EFLAGS: 00000246 ORIG_RAX: 0000000000000000
>> RAX: ffffffffffffffda RBX: 00000000017df780 RCX: 00007fdeccbdf702
>> RDX: 0000000000000fff RSI: 00007ffe0dfb4f30 RDI: 0000000000000003
>> RBP: 00007ffe0dfb4f30 R08: 00000000017df9f0 R09: 00007fdecccb1a60
>> R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000000003
>> R13: 00000000017e75a3 R14: 00000000017df760 R15: 00000000017e75a3
>>
>> Add guard to make sure opts is not NULL in the
>> nvme_ctrl_dhchap_ctrl_secret_show() to avoid the OOPs.
>>
>> Signed-off-by: Chaitanya Kulkarni <kch at nvidia.com>
>> ---
>>   drivers/nvme/host/core.c | 9 ++++++---
>>   1 file changed, 6 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
>> index 5bdeb7134ede..3df8e6bef589 100644
>> --- a/drivers/nvme/host/core.c
>> +++ b/drivers/nvme/host/core.c
>> @@ -3671,9 +3671,12 @@ static ssize_t 
>> nvme_ctrl_dhchap_ctrl_secret_show(struct device *dev,
>>       struct nvme_ctrl *ctrl = dev_get_drvdata(dev);
>>       struct nvmf_ctrl_options *opts = ctrl->opts;
>> -    if (!opts->dhchap_ctrl_secret)
>> -        return sysfs_emit(buf, "none\n");
>> -    return sysfs_emit(buf, "%s\n", opts->dhchap_ctrl_secret);
>> +    if (opts) {
>> +        if (!opts->dhchap_ctrl_secret)
>> +            return sysfs_emit(buf, "none\n");
>> +        return sysfs_emit(buf, "%s\n", opts->dhchap_ctrl_secret);
>> +    }
>> +    return 0;
> 
> Shouldn't this be filtered as non-visible in the first place?
> 
Indeed, the correct fix is

diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index 20a3565c3e87..fbd61419716d 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -3719,6 +3719,8 @@ static umode_t nvme_dev_attrs_are_visible(struct 
kobject *kobj,
  #ifdef CONFIG_NVME_AUTH
         if (a == &dev_attr_dhchap_secret.attr && !ctrl->opts)
                 return 0;
+       if (a == &dev_attr_dhchap_ctrl_secret.attr && !ctrl->opts)
+               return 0;
  #endif

         return a->mode;

Will be including it in the next round.

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		           Kernel Storage Architect
hare at suse.de			                  +49 911 74053 688
SUSE Software Solutions Germany GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), GF: Felix Imendörffer