Re: [PATCH] drm/syncobj: Fix syncobj leak in drm_syncobj_eventfd_ioctl

[Tvrtko Ursulin]

On 09/09/2024 21:53, T.J. Mercier wrote:
> A syncobj reference is taken in drm_syncobj_find, but not released if
> eventfd_ctx_fdget or kzalloc fails. Put the reference in these error
> paths.
>
> Reported-by: Xingyu Jin <xingyuj@xxxxxxxxxx>
> Fixes: c7a472297169 ("drm/syncobj: add IOCTL to register an eventfd")
> Signed-off-by: T.J. Mercier <tjmercier@xxxxxxxxxx>
> ---
>   drivers/gpu/drm/drm_syncobj.c | 17 +++++++++++++----
>   1 file changed, 13 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c
> index a0e94217b511..4fcfc0b9b386 100644
> --- a/drivers/gpu/drm/drm_syncobj.c
> +++ b/drivers/gpu/drm/drm_syncobj.c
> @@ -1464,6 +1464,7 @@ drm_syncobj_eventfd_ioctl(struct drm_device *dev, void *data,
>   	struct drm_syncobj *syncobj;
>   	struct eventfd_ctx *ev_fd_ctx;
>   	struct syncobj_eventfd_entry *entry;
> +	int ret;
>   
>   	if (!drm_core_check_feature(dev, DRIVER_SYNCOBJ_TIMELINE))
>   		return -EOPNOTSUPP;
> @@ -1479,13 +1480,15 @@ drm_syncobj_eventfd_ioctl(struct drm_device *dev, void *data,
>   		return -ENOENT;
>   
>   	ev_fd_ctx = eventfd_ctx_fdget(args->fd);
> -	if (IS_ERR(ev_fd_ctx))
> -		return PTR_ERR(ev_fd_ctx);
> +	if (IS_ERR(ev_fd_ctx)) {
> +		ret = PTR_ERR(ev_fd_ctx);
> +		goto err_fdget;
> +	}
>   
>   	entry = kzalloc(sizeof(*entry), GFP_KERNEL);
>   	if (!entry) {
> -		eventfd_ctx_put(ev_fd_ctx);
> -		return -ENOMEM;
> +		ret = -ENOMEM;
> +		goto err_kzalloc;
>   	}
>   	entry->syncobj = syncobj;
>   	entry->ev_fd_ctx = ev_fd_ctx;
> @@ -1496,6 +1499,12 @@ drm_syncobj_eventfd_ioctl(struct drm_device *dev, void *data,
>   	drm_syncobj_put(syncobj);
>   
>   	return 0;
> +
> +err_kzalloc:
> +	eventfd_ctx_put(ev_fd_ctx);
> +err_fdget:
> +	drm_syncobj_put(syncobj);
> +	return ret;
>   }
>   
>   int

Easy enough to review while browsing the list:

Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxxx>

Regards,

Tvrtko