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

[Christian König]

Am 10.09.24 um 09:26 schrieb 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>

Looks reasonable to me as well.

Reviewed-by. Christian König <christian.koenig@xxxxxxx>
CC: stable?

Let me know when you need someone to push it to drm-misc-fixes.

Regards,
Christian.

> Regards,
>
> Tvrtko