[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CABdmKX2JRi-7x_pkSrkuwjzzjDnDQyMEcZmfWrn2AXLuOHQ6Qw@mail.gmail.com>
Date: Tue, 10 Sep 2024 07:47:54 -0700
From: "T.J. Mercier" <tjmercier@...gle.com>
To: Christian König <christian.koenig@....com>
Cc: Tvrtko Ursulin <tursulin@...ulin.net>,
Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>, Maxime Ripard <mripard@...nel.org>,
Thomas Zimmermann <tzimmermann@...e.de>, David Airlie <airlied@...il.com>, Daniel Vetter <daniel@...ll.ch>,
Simon Ser <contact@...rsion.fr>, Pekka Paalanen <pekka.paalanen@...labora.com>,
Xingyu Jin <xingyuj@...gle.com>, dri-devel@...ts.freedesktop.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH] drm/syncobj: Fix syncobj leak in drm_syncobj_eventfd_ioctl
On Tue, Sep 10, 2024 at 12:30 AM Christian König
<christian.koenig@....com> wrote:
>
> 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@...gle.com>
> >> Fixes: c7a472297169 ("drm/syncobj: add IOCTL to register an eventfd")
> >> Signed-off-by: T.J. Mercier <tjmercier@...gle.com>
> >> ---
> >> 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@...lia.com>
>
> Looks reasonable to me as well.
>
> Reviewed-by. Christian König <christian.koenig@....com>
Thanks!
> CC: stable?
Yes, I think we should. 6.6 and 6.10
> Let me know when you need someone to push it to drm-misc-fixes.
Anytime is good, no rush for this one.
>
> Regards,
> Christian.
>
> >
> > Regards,
> >
> > Tvrtko
>
Powered by blists - more mailing lists