[PATCH] nvme-core: Fixing ctrl de-referencing in nvme_init_ctrl

[irvin cote]

The thing is that after device_initialize is called the reference
count for the device is equal to 1. Now the function also calls
nvme_get_ctrl which increases the ref-count to 2.
However the teardown path only accounts for 1 decrement. This means
that if an error were to occur during nvme_init_ctrl, we would return
from nvme_probe without having freed the resources of the controller.

On Mon, 3 Apr 2023 at 07:22, Sagi Grimberg <sagi at grimberg.me> wrote:
>
>
>
> On 4/3/23 03:41, Irvin Cote wrote:
> > Correcting the de-reference count for
> > the controller in the core layer to avoid
> > a memory leak.
>
> Is this a regression?
>
> > Also cleaning a bit the teardown logic of nvme_init_ctrl.
>
> This is a good cleanup.
>
> >
> > Signed-off-by: Irvin Cote <irvincoteg at gmail.com>
> > ---
> >   drivers/nvme/host/core.c | 14 ++++++--------
> >   1 file changed, 6 insertions(+), 8 deletions(-)
> >
> > diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
> > index 8698410aeb84..3d7aca7d0f2a 100644
> > --- a/drivers/nvme/host/core.c
> > +++ b/drivers/nvme/host/core.c
> > @@ -5137,14 +5137,12 @@ int nvme_init_ctrl(struct nvme_ctrl *ctrl, struct device *dev,
> >       BUILD_BUG_ON(NVME_DSM_MAX_RANGES * sizeof(struct nvme_dsm_range) >
> >                       PAGE_SIZE);
> >       ctrl->discard_page = alloc_page(GFP_KERNEL);
> > -     if (!ctrl->discard_page) {
> > -             ret = -ENOMEM;
> > -             goto out;
> > -     }
> > +     if (!ctrl->discard_page)
> > +             return -ENOMEM;
> >
> >       ret = ida_alloc(&nvme_instance_ida, GFP_KERNEL);
> >       if (ret < 0)
> > -             goto out;
> > +             goto out_free_page;
> >       ctrl->instance = ret;
> >
> >       device_initialize(&ctrl->ctrl_device);
> > @@ -5191,10 +5189,10 @@ int nvme_init_ctrl(struct nvme_ctrl *ctrl, struct device *dev,
> >       nvme_put_ctrl(ctrl);
> >       kfree_const(ctrl->device->kobj.name);
> >   out_release_instance:
> > +     nvme_put_ctrl(ctrl);
> >       ida_free(&nvme_instance_ida, ctrl->instance);
> > -out:
> > -     if (ctrl->discard_page)
> > -             __free_page(ctrl->discard_page);
> > +out_free_page:
> > +     __free_page(ctrl->discard_page);
> >       return ret;
> >   }
> >   EXPORT_SYMBOL_GPL(nvme_init_ctrl);