[PATCH] nvme: fix memory leak freeing command effects

[PATCH] nvme: fix memory leak freeing command effects

[Keith Busch]

xa_destroy() frees only internal data. The caller is responsible for
freeing the exteranl objects referenced by an xarray.

Fixes: 1cf7a12e09aa4 ("nvme: use an xarray to lookup the Commands Supported and Effects log")
Signed-off-by: Keith Busch <kbusch@kernel.org>
---
 drivers/nvme/host/core.c | 16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)

diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index 2c197f69c03a..a288d925f71b 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -4398,6 +4398,19 @@ void nvme_uninit_ctrl(struct nvme_ctrl *ctrl)
 }
 EXPORT_SYMBOL_GPL(nvme_uninit_ctrl);
 
+static void nvme_free_cels(struct nvme_ctrl *ctrl)
+{
+	struct nvme_effects_log	*cel;
+	unsigned long i;
+
+	xa_for_each (&ctrl->cels, i, cel) {
+		xa_erase(&ctrl->cels, i);
+		kfree(cel);
+	}
+
+	xa_destroy(&ctrl->cels);
+}
+
 static void nvme_free_ctrl(struct device *dev)
 {
 	struct nvme_ctrl *ctrl =
@@ -4407,8 +4420,7 @@ static void nvme_free_ctrl(struct device *dev)
 	if (!subsys || ctrl->instance != subsys->instance)
 		ida_simple_remove(&nvme_instance_ida, ctrl->instance);
 
-	xa_destroy(&ctrl->cels);
-
+	nvme_free_cels(ctrl);
 	nvme_mpath_uninit(ctrl);
 	__free_page(ctrl->discard_page);
 
-- 
2.24.1


_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

Re: [PATCH] nvme: fix memory leak freeing command effects

[Christoph Hellwig]

Thanks, applied to nvme-5.10.

_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

Re: [PATCH] nvme: fix memory leak freeing command effects

[Chaitanya Kulkarni]

On 11/13/20 10:45, Keith Busch wrote:
> xa_destroy() frees only internal data. The caller is responsible for
> freeing the exteranl objects referenced by an xarray.
>
> Fixes: 1cf7a12e09aa4 ("nvme: use an xarray to lookup the Commands Supported and Effects log")
> Signed-off-by: Keith Busch <kbusch@kernel.org>
> ---

Thanks for fixing this up, it'd nice to have a common helper

which also erases the itmes from XArray so we don't have to

repeat the code to erase item indivisually.


I'll see if I can send an RFC.


_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

Re: [PATCH] nvme: fix memory leak freeing command effects

[Keith Busch]

On Mon, Nov 16, 2020 at 11:23:30PM +0000, Chaitanya Kulkarni wrote:
> On 11/13/20 10:45, Keith Busch wrote:
> > xa_destroy() frees only internal data. The caller is responsible for
> > freeing the exteranl objects referenced by an xarray.
> >
> > Fixes: 1cf7a12e09aa4 ("nvme: use an xarray to lookup the Commands Supported and Effects log")
> > Signed-off-by: Keith Busch <kbusch@kernel.org>
> > ---
> 
> Thanks for fixing this up, it'd nice to have a common helper
> which also erases the itmes from XArray so we don't have to
> repeat the code to erase item indivisually.

That would be okay if there are multiple users that have a simple
kfree() on their elements when tearing down their xarray, but I'm not
finding others so far.

_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme