[bug report] kmemleak observed during blktests nvme-tcp

[Yi Zhang]

Hi Chaitanya

The kmemleak in [1] is fixed by your patch, but there still has
one[2], would you mind checking it, thanks.

[1]
nvme_ctrl_dhchap_secret_store
cdev_device_add

[2]
unreferenced object 0xffff888288a53b80 (size 96):
  comm "nvme", pid 1934, jiffies 4294932237 (age 237.359s)
  hex dump (first 32 bytes):
    00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
    00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
  backtrace:
    [<ffffffff86564437>] kmalloc_trace+0x27/0xe0
    [<ffffffff87395fa0>] dev_pm_qos_update_user_latency_tolerance+0xe0/0x200
    [<ffffffffc08c783c>] nvme_init_ctrl+0xc5c/0x1140 [nvme_core]
    [<ffffffffc1ab0e0c>] 0xffffffffc1ab0e0c
    [<ffffffffc0d38177>] 0xffffffffc0d38177
    [<ffffffffc0d38613>] 0xffffffffc0d38613
    [<ffffffff867b5056>] vfs_write+0x216/0xc60
    [<ffffffff867b62e9>] ksys_write+0xf9/0x1d0
    [<ffffffff881adc4c>] do_syscall_64+0x5c/0x90
    [<ffffffff882000aa>] entry_SYSCALL_64_after_hwframe+0x72/0xdc

On Wed, Apr 26, 2023 at 4:34 PM Chaitanya Kulkarni
<chaitanyak at nvidia.com> wrote:
>
> On 4/26/23 01:23, Chaitanya Kulkarni wrote:
> >
> >>>> [<ffffffff86f646ab>] __kmalloc+0x4b/0x190
> >>>>       [<ffffffffc09fb710>]
> >>>> nvme_ctrl_dhchap_secret_store+0x110/0x350 [nvme_core]
> >>>>       [<ffffffff873cc848>] kernfs_fop_write_iter+0x358/0x530
> >>>>       [<ffffffff871b47d2>] vfs_write+0x802/0xc60
> >>>>       [<ffffffff871b5479>] ksys_write+0xf9/0x1d0
> >>>>       [<ffffffff88ba8f9c>] do_syscall_64+0x5c/0x90
> >>>>       [<ffffffff88c000aa>] entry_SYSCALL_64_after_hwframe+0x72/0xdc
> >
> > can you check if following fixes your problem for dhchap ?
> >
> >
> > linux-block (for-next) # git diff
> > diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
> > index 1bfd52eae2ee..0e22d048de3c 100644
> > --- a/drivers/nvme/host/core.c
> > +++ b/drivers/nvme/host/core.c
> > @@ -3825,8 +3825,10 @@ static ssize_t
> > nvme_ctrl_dhchap_secret_store(struct device *dev,
> >                 int ret;
> >
> >                 ret = nvme_auth_generate_key(dhchap_secret, &key);
> > -               if (ret)
> > +               if (ret) {
> > +                       kfree(dhchap_secret);
> >                         return ret;
> > +               }
> >                 kfree(opts->dhchap_secret);
> >                 opts->dhchap_secret = dhchap_secret;
> >                 host_key = ctrl->host_key;
> > @@ -3879,8 +3881,10 @@ static ssize_t
> > nvme_ctrl_dhchap_ctrl_secret_store(struct device *dev,
> >                 int ret;
> >
> >                 ret = nvme_auth_generate_key(dhchap_secret, &key);
> > -               if (ret)
> > +               if (ret) {
> > +                       kfree(dhchap_secret);
> >                         return ret;
> > +               }
> >                 kfree(opts->dhchap_ctrl_secret);
> >                 opts->dhchap_ctrl_secret = dhchap_secret;
> >                 ctrl_key = ctrl->ctrl_key;
> >
> > -ck
> >
> >
>
> sorry my forget to add ida changes, plz ignore earlier and try this :-
>
> linux-block (for-next) # git diff
> diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
> index 1bfd52eae2ee..bb376cc6a5a3 100644
> --- a/drivers/nvme/host/core.c
> +++ b/drivers/nvme/host/core.c
> @@ -3825,8 +3825,10 @@ static ssize_t
> nvme_ctrl_dhchap_secret_store(struct device *dev,
>                  int ret;
>
>                  ret = nvme_auth_generate_key(dhchap_secret, &key);
> -               if (ret)
> +               if (ret) {
> +                       kfree(dhchap_secret);
>                          return ret;
> +               }
>                  kfree(opts->dhchap_secret);
>                  opts->dhchap_secret = dhchap_secret;
>                  host_key = ctrl->host_key;
> @@ -3879,8 +3881,10 @@ static ssize_t
> nvme_ctrl_dhchap_ctrl_secret_store(struct device *dev,
>                  int ret;
>
>                  ret = nvme_auth_generate_key(dhchap_secret, &key);
> -               if (ret)
> +               if (ret) {
> +                       kfree(dhchap_secret);
>                          return ret;
> +               }
>                  kfree(opts->dhchap_ctrl_secret);
>                  opts->dhchap_ctrl_secret = dhchap_secret;
>                  ctrl_key = ctrl->ctrl_key;
> @@ -4042,8 +4046,10 @@ int nvme_cdev_add(struct cdev *cdev, struct
> device *cdev_device,
>          cdev_init(cdev, fops);
>          cdev->owner = owner;
>          ret = cdev_device_add(cdev, cdev_device);
> -       if (ret)
> +       if (ret) {
>                  put_device(cdev_device);
> +               ida_free(&nvme_ns_chr_minor_ida, MINOR(cdev_device->devt));
> +       }
>
>          return ret;
>   }
>
>
> with above patch I was able to get this :-
>
> blktests (master) # ./check nvme/044
> nvme/044 (Test bi-directional authentication) [passed]
>      runtime  1.729s  ...  1.892s
> blktests (master) # ./check nvme/045
> nvme/045 (Test re-authentication) [passed]
>      runtime  4.798s  ...  6.303s
>
> -ck
>
>


-- 
Best Regards,
  Yi Zhang