[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <7aef07b2-9859-40a8-ba5b-22aba68c2d9c@amd.com>
Date: Tue, 10 Sep 2024 09:30:45 +0200
From: Christian König <christian.koenig@....com>
To: Tvrtko Ursulin <tursulin@...ulin.net>, "T.J. Mercier"
<tjmercier@...gle.com>, 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>
Cc: 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
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>
CC: stable?
Let me know when you need someone to push it to drm-misc-fixes.
Regards,
Christian.
>
> Regards,
>
> Tvrtko
Powered by blists - more mailing lists