2022-01-22 00:38:07

by Christian König

[permalink] [raw]
Subject: Re: [PATCH] drm/amd/amdgpu/amdgpu_cs: fix refcount leak of a dma_fence obj

Am 21.01.22 um 06:28 schrieb Xin Xiong:
> This issue takes place in an error path in
> amdgpu_cs_fence_to_handle_ioctl(). When `info->in.what` falls into
> default case, the function simply returns -EINVAL, forgetting to
> decrement the reference count of a dma_fence obj, which is bumped
> earlier by amdgpu_cs_get_fence(). This may result in reference count
> leaks.
>
> Fix it by decreasing the refcount of specific object before returning
> the error code.
>
> Signed-off-by: Xin Xiong <[email protected]>
> Signed-off-by: Xin Tan <[email protected]>

Good catch. Reviewed-by: Christian König <[email protected]>

> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> index 0311d799a..894869789 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> @@ -1510,6 +1510,7 @@ int amdgpu_cs_fence_to_handle_ioctl(struct drm_device *dev, void *data,
> return 0;
>
> default:
> + dma_fence_put(fence);
> return -EINVAL;
> }
> }


2022-01-22 08:08:19

by Alex Deucher

[permalink] [raw]
Subject: Re: [PATCH] drm/amd/amdgpu/amdgpu_cs: fix refcount leak of a dma_fence obj

On Fri, Jan 21, 2022 at 2:45 AM Christian König
<[email protected]> wrote:
>
> Am 21.01.22 um 06:28 schrieb Xin Xiong:
> > This issue takes place in an error path in
> > amdgpu_cs_fence_to_handle_ioctl(). When `info->in.what` falls into
> > default case, the function simply returns -EINVAL, forgetting to
> > decrement the reference count of a dma_fence obj, which is bumped
> > earlier by amdgpu_cs_get_fence(). This may result in reference count
> > leaks.
> >
> > Fix it by decreasing the refcount of specific object before returning
> > the error code.
> >
> > Signed-off-by: Xin Xiong <[email protected]>
> > Signed-off-by: Xin Tan <[email protected]>
>
> Good catch. Reviewed-by: Christian König <[email protected]>

Applied manually. Strangely I never got this on any of my emails, and
I don't see it in the archives.

Alex

>
> > ---
> > drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 1 +
> > 1 file changed, 1 insertion(+)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> > index 0311d799a..894869789 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> > @@ -1510,6 +1510,7 @@ int amdgpu_cs_fence_to_handle_ioctl(struct drm_device *dev, void *data,
> > return 0;
> >
> > default:
> > + dma_fence_put(fence);
> > return -EINVAL;
> > }
> > }
>