* [PATCH] drm/syncobj: Fix syncobj leak in drm_syncobj_eventfd_ioctl
@ 2024-09-09 20:53 T.J. Mercier
2024-09-10 7:26 ` Tvrtko Ursulin
0 siblings, 1 reply; 5+ messages in thread
From: T.J. Mercier @ 2024-09-09 20:53 UTC (permalink / raw)
To: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
David Airlie, Daniel Vetter, Christian König, Simon Ser,
Pekka Paalanen
Cc: T.J. Mercier, Xingyu Jin, dri-devel, linux-kernel
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@google.com>
Fixes: c7a472297169 ("drm/syncobj: add IOCTL to register an eventfd")
Signed-off-by: T.J. Mercier <tjmercier@google.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
--
2.46.0.598.g6f2099f65c-goog
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] drm/syncobj: Fix syncobj leak in drm_syncobj_eventfd_ioctl
2024-09-09 20:53 [PATCH] drm/syncobj: Fix syncobj leak in drm_syncobj_eventfd_ioctl T.J. Mercier
@ 2024-09-10 7:26 ` Tvrtko Ursulin
2024-09-10 7:30 ` Christian König
0 siblings, 1 reply; 5+ messages in thread
From: Tvrtko Ursulin @ 2024-09-10 7:26 UTC (permalink / raw)
To: T.J. Mercier, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Daniel Vetter,
Christian König, Simon Ser, Pekka Paalanen
Cc: Xingyu Jin, dri-devel, linux-kernel
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@google.com>
> Fixes: c7a472297169 ("drm/syncobj: add IOCTL to register an eventfd")
> Signed-off-by: T.J. Mercier <tjmercier@google.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@igalia.com>
Regards,
Tvrtko
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] drm/syncobj: Fix syncobj leak in drm_syncobj_eventfd_ioctl
2024-09-10 7:26 ` Tvrtko Ursulin
@ 2024-09-10 7:30 ` Christian König
2024-09-10 14:47 ` T.J. Mercier
0 siblings, 1 reply; 5+ messages in thread
From: Christian König @ 2024-09-10 7:30 UTC (permalink / raw)
To: Tvrtko Ursulin, T.J. Mercier, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Daniel Vetter, Simon Ser,
Pekka Paalanen
Cc: Xingyu Jin, dri-devel, linux-kernel
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@google.com>
>> Fixes: c7a472297169 ("drm/syncobj: add IOCTL to register an eventfd")
>> Signed-off-by: T.J. Mercier <tjmercier@google.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@igalia.com>
Looks reasonable to me as well.
Reviewed-by. Christian König <christian.koenig@amd.com>
CC: stable?
Let me know when you need someone to push it to drm-misc-fixes.
Regards,
Christian.
>
> Regards,
>
> Tvrtko
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] drm/syncobj: Fix syncobj leak in drm_syncobj_eventfd_ioctl
2024-09-10 7:30 ` Christian König
@ 2024-09-10 14:47 ` T.J. Mercier
2024-09-10 15:16 ` Christian König
0 siblings, 1 reply; 5+ messages in thread
From: T.J. Mercier @ 2024-09-10 14:47 UTC (permalink / raw)
To: Christian König
Cc: Tvrtko Ursulin, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Daniel Vetter, Simon Ser,
Pekka Paalanen, Xingyu Jin, dri-devel, linux-kernel
On Tue, Sep 10, 2024 at 12:30 AM Christian König
<christian.koenig@amd.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@google.com>
> >> Fixes: c7a472297169 ("drm/syncobj: add IOCTL to register an eventfd")
> >> Signed-off-by: T.J. Mercier <tjmercier@google.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@igalia.com>
>
> Looks reasonable to me as well.
>
> Reviewed-by. Christian König <christian.koenig@amd.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
>
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] drm/syncobj: Fix syncobj leak in drm_syncobj_eventfd_ioctl
2024-09-10 14:47 ` T.J. Mercier
@ 2024-09-10 15:16 ` Christian König
0 siblings, 0 replies; 5+ messages in thread
From: Christian König @ 2024-09-10 15:16 UTC (permalink / raw)
To: T.J. Mercier
Cc: Tvrtko Ursulin, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Daniel Vetter, Simon Ser,
Pekka Paalanen, Xingyu Jin, dri-devel, linux-kernel
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@amd.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@google.com>
>>>> Fixes: c7a472297169 ("drm/syncobj: add IOCTL to register an eventfd")
>>>> Signed-off-by: T.J. Mercier <tjmercier@google.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@igalia.com>
>> Looks reasonable to me as well.
>>
>> Reviewed-by. Christian König <christian.koenig@amd.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
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2024-09-10 15:17 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-09-09 20:53 [PATCH] drm/syncobj: Fix syncobj leak in drm_syncobj_eventfd_ioctl T.J. Mercier
2024-09-10 7:26 ` Tvrtko Ursulin
2024-09-10 7:30 ` Christian König
2024-09-10 14:47 ` T.J. Mercier
2024-09-10 15:16 ` Christian König
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®