linux-kernel - Re: [PATCH] drm/syncobj: Fix syncobj leak in drm_syncobj_eventfd

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <88b7e403-5910-44ab-8f42-fc183beafc67@amd.com>
Date: Tue, 10 Sep 2024 17:16:56 +0200
From: Christian König <christian.koenig@....com>
To: "T.J. Mercier" <tjmercier@...gle.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

Am 10.09.24 um 16:47 schrieb T.J. Mercier:
> 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.

Done.

Christian.

>
>> Regards,
>> Christian.
>>
>>> Regards,
>>>
>>> Tvrtko