2024-01-12 08:51:39

by Zhipeng Lu

Subject: [PATCH] drm/lima: fix a memleak in lima_heap_alloc

When lima_vm_map_bo fails, the resources need to be deallocated, or
there will be memleaks.

Fixes: 6aebc51d7aef ("drm/lima: support heap buffer creation")
Signed-off-by: Zhipeng Lu <[email protected]>
---
drivers/gpu/drm/lima/lima_gem.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/lima/lima_gem.c b/drivers/gpu/drm/lima/lima_gem.c
index 4f9736e5f929..824ed22141c7 100644
--- a/drivers/gpu/drm/lima/lima_gem.c
+++ b/drivers/gpu/drm/lima/lima_gem.c
@@ -92,8 +92,13 @@ int lima_heap_alloc(struct lima_bo *bo, struct lima_vm *vm)

if (vm) {
ret = lima_vm_map_bo(vm, bo, old_size >> PAGE_SHIFT);
- if (ret)
+ if (ret) {
+ dma_unmap_sgtable(dev, &sgt, DMA_BIDIRECTIONAL, 0);
+ sg_free_table(&sgt);
+ kfree(bo->base.sgt);
+ bo->base.sgt = NULL;
return ret;
+ }
}

bo->heap_size = new_size;
--
2.34.1

2024-01-16 01:54:14

by Qiang Yu

Subject: Re: [PATCH] drm/lima: fix a memleak in lima_heap_alloc

Thanks for the fix. As the error handling gets longer and duplicated,
could you rearrange them like the lima_gem_submit():
err_out2:
dma_unmap_sgtable(dev, &sgt, DMA_BIDIRECTIONAL, 0);
err_out1:
kfree(bo->base.sgt);
bo->base.sgt = NULL;
err_out0:
sg_free_table(&sgt);
return ret.

Regards,
Qiang

On Fri, Jan 12, 2024 at 4:49 PM Zhipeng Lu <[email protected]> wrote:
>
> When lima_vm_map_bo fails, the resources need to be deallocated, or
> there will be memleaks.
>
> Fixes: 6aebc51d7aef ("drm/lima: support heap buffer creation")
> Signed-off-by: Zhipeng Lu <[email protected]>
> ---
> drivers/gpu/drm/lima/lima_gem.c | 7 ++++++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/lima/lima_gem.c b/drivers/gpu/drm/lima/lima_gem.c
> index 4f9736e5f929..824ed22141c7 100644
> --- a/drivers/gpu/drm/lima/lima_gem.c
> +++ b/drivers/gpu/drm/lima/lima_gem.c
> @@ -92,8 +92,13 @@ int lima_heap_alloc(struct lima_bo *bo, struct lima_vm *vm)
>
> if (vm) {
> ret = lima_vm_map_bo(vm, bo, old_size >> PAGE_SHIFT);
> - if (ret)
> + if (ret) {
> + dma_unmap_sgtable(dev, &sgt, DMA_BIDIRECTIONAL, 0);
> + sg_free_table(&sgt);
> + kfree(bo->base.sgt);
> + bo->base.sgt = NULL;
> return ret;
> + }
> }
>
> bo->heap_size = new_size;
> --
> 2.34.1
>

2024-01-17 07:11:15

by Zhipeng Lu

Subject: Re: Re: [PATCH] drm/lima: fix a memleak in lima_heap_alloc

> Thanks for the fix. As the error handling gets longer and duplicated,
> could you rearrange them like the lima_gem_submit():
> err_out2:
> dma_unmap_sgtable(dev, &sgt, DMA_BIDIRECTIONAL, 0);
> err_out1:
> kfree(bo->base.sgt);
> bo->base.sgt = NULL;
> err_out0:
> sg_free_table(&sgt);
> return ret.
>
> Regards,
> Qiang
>

Sure, I'll send a v2 version of this patch later following your advise.

Regards,
Zhipeng

> On Fri, Jan 12, 2024 at 4:49 PM Zhipeng Lu <[email protected]> wrote:
> >
> > When lima_vm_map_bo fails, the resources need to be deallocated, or
> > there will be memleaks.
> >
> > Fixes: 6aebc51d7aef ("drm/lima: support heap buffer creation")
> > Signed-off-by: Zhipeng Lu <[email protected]>
> > ---
> > drivers/gpu/drm/lima/lima_gem.c | 7 ++++++-
> > 1 file changed, 6 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/lima/lima_gem.c b/drivers/gpu/drm/lima/lima_gem.c
> > index 4f9736e5f929..824ed22141c7 100644
> > --- a/drivers/gpu/drm/lima/lima_gem.c
> > +++ b/drivers/gpu/drm/lima/lima_gem.c
> > @@ -92,8 +92,13 @@ int lima_heap_alloc(struct lima_bo *bo, struct lima_vm *vm)
> >
> > if (vm) {
> > ret = lima_vm_map_bo(vm, bo, old_size >> PAGE_SHIFT);
> > - if (ret)
> > + if (ret) {
> > + dma_unmap_sgtable(dev, &sgt, DMA_BIDIRECTIONAL, 0);
> > + sg_free_table(&sgt);
> > + kfree(bo->base.sgt);
> > + bo->base.sgt = NULL;
> > return ret;
> > + }
> > }
> >
> > bo->heap_size = new_size;
> > --
> > 2.34.1
> >