dri-devel.lists.freedesktop.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] drm/syncobj: Fix use-after-free
@ 2021-01-19 13:03 Daniel Vetter
  2021-01-19 13:08 ` Christian König
  0 siblings, 1 reply; 3+ messages in thread
From: Daniel Vetter @ 2021-01-19 13:03 UTC (permalink / raw)
  To: DRI Development
  Cc: David Airlie, Daniel Vetter, Intel Graphics Development,
	Thomas Zimmermann, Daniel Vetter, stable, Christian König

While reviewing Christian's annotation patch I noticed that we have a
user-after-free for the WAIT_FOR_SUBMIT case: We drop the syncobj
reference before we've completed the waiting.

Of course usually there's nothing bad happening here since userspace
keeps the reference, but we can't rely on userspace to play nice here!

Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
Fixes: bc9c80fe01a2 ("drm/syncobj: use the timeline point in drm_syncobj_find_fence v4")
Cc: Christian König <christian.koenig@amd.com>
Cc: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Maxime Ripard <mripard@kernel.org>
Cc: Thomas Zimmermann <tzimmermann@suse.de>
Cc: David Airlie <airlied@linux.ie>
Cc: Daniel Vetter <daniel@ffwll.ch>
Cc: dri-devel@lists.freedesktop.org
Cc: <stable@vger.kernel.org> # v5.2+
---
 drivers/gpu/drm/drm_syncobj.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c
index 6e74e6745eca..349146049849 100644
--- a/drivers/gpu/drm/drm_syncobj.c
+++ b/drivers/gpu/drm/drm_syncobj.c
@@ -388,19 +388,18 @@ int drm_syncobj_find_fence(struct drm_file *file_private,
 		return -ENOENT;
 
 	*fence = drm_syncobj_fence_get(syncobj);
-	drm_syncobj_put(syncobj);
 
 	if (*fence) {
 		ret = dma_fence_chain_find_seqno(fence, point);
 		if (!ret)
-			return 0;
+			goto out;
 		dma_fence_put(*fence);
 	} else {
 		ret = -EINVAL;
 	}
 
 	if (!(flags & DRM_SYNCOBJ_WAIT_FLAGS_WAIT_FOR_SUBMIT))
-		return ret;
+		goto out;
 
 	memset(&wait, 0, sizeof(wait));
 	wait.task = current;
@@ -432,6 +431,9 @@ int drm_syncobj_find_fence(struct drm_file *file_private,
 	if (wait.node.next)
 		drm_syncobj_remove_wait(syncobj, &wait);
 
+out:
+	drm_syncobj_put(syncobj);
+
 	return ret;
 }
 EXPORT_SYMBOL(drm_syncobj_find_fence);
-- 
2.30.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply related	[flat|nested] 3+ messages in thread
* Re: [PATCH] drm/syncobj: Fix use-after-free
  2021-01-19 13:03 [PATCH] drm/syncobj: Fix use-after-free Daniel Vetter
@ 2021-01-19 13:08 ` Christian König
  2021-01-20  9:28   ` Daniel Vetter
  0 siblings, 1 reply; 3+ messages in thread
From: Christian König @ 2021-01-19 13:08 UTC (permalink / raw)
  To: Daniel Vetter, DRI Development
  Cc: David Airlie, Intel Graphics Development, Thomas Zimmermann,
	Daniel Vetter, stable

Am 19.01.21 um 14:03 schrieb Daniel Vetter:
> While reviewing Christian's annotation patch I noticed that we have a
> user-after-free for the WAIT_FOR_SUBMIT case: We drop the syncobj
> reference before we've completed the waiting.
>
> Of course usually there's nothing bad happening here since userspace
> keeps the reference, but we can't rely on userspace to play nice here!
>
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> Fixes: bc9c80fe01a2 ("drm/syncobj: use the timeline point in drm_syncobj_find_fence v4")
> Cc: Christian König <christian.koenig@amd.com>
> Cc: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Cc: Maxime Ripard <mripard@kernel.org>
> Cc: Thomas Zimmermann <tzimmermann@suse.de>
> Cc: David Airlie <airlied@linux.ie>
> Cc: Daniel Vetter <daniel@ffwll.ch>
> Cc: dri-devel@lists.freedesktop.org
> Cc: <stable@vger.kernel.org> # v5.2+

Reviewed-by: Christian König <christian.koenig@amd.com>

> ---
>   drivers/gpu/drm/drm_syncobj.c | 8 +++++---
>   1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c
> index 6e74e6745eca..349146049849 100644
> --- a/drivers/gpu/drm/drm_syncobj.c
> +++ b/drivers/gpu/drm/drm_syncobj.c
> @@ -388,19 +388,18 @@ int drm_syncobj_find_fence(struct drm_file *file_private,
>   		return -ENOENT;
>   
>   	*fence = drm_syncobj_fence_get(syncobj);
> -	drm_syncobj_put(syncobj);
>   
>   	if (*fence) {
>   		ret = dma_fence_chain_find_seqno(fence, point);
>   		if (!ret)
> -			return 0;
> +			goto out;
>   		dma_fence_put(*fence);
>   	} else {
>   		ret = -EINVAL;
>   	}
>   
>   	if (!(flags & DRM_SYNCOBJ_WAIT_FLAGS_WAIT_FOR_SUBMIT))
> -		return ret;
> +		goto out;
>   
>   	memset(&wait, 0, sizeof(wait));
>   	wait.task = current;
> @@ -432,6 +431,9 @@ int drm_syncobj_find_fence(struct drm_file *file_private,
>   	if (wait.node.next)
>   		drm_syncobj_remove_wait(syncobj, &wait);
>   
> +out:
> +	drm_syncobj_put(syncobj);
> +
>   	return ret;
>   }
>   EXPORT_SYMBOL(drm_syncobj_find_fence);

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 3+ messages in thread
* Re: [PATCH] drm/syncobj: Fix use-after-free
  2021-01-19 13:08 ` Christian König
@ 2021-01-20  9:28   ` Daniel Vetter
  0 siblings, 0 replies; 3+ messages in thread
From: Daniel Vetter @ 2021-01-20  9:28 UTC (permalink / raw)
  To: Christian König
  Cc: David Airlie, Daniel Vetter, Intel Graphics Development,
	DRI Development, Thomas Zimmermann, Daniel Vetter, stable

On Tue, Jan 19, 2021 at 02:08:12PM +0100, Christian König wrote:
> Am 19.01.21 um 14:03 schrieb Daniel Vetter:
> > While reviewing Christian's annotation patch I noticed that we have a
> > user-after-free for the WAIT_FOR_SUBMIT case: We drop the syncobj
> > reference before we've completed the waiting.
> > 
> > Of course usually there's nothing bad happening here since userspace
> > keeps the reference, but we can't rely on userspace to play nice here!
> > 
> > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> > Fixes: bc9c80fe01a2 ("drm/syncobj: use the timeline point in drm_syncobj_find_fence v4")
> > Cc: Christian König <christian.koenig@amd.com>
> > Cc: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
> > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> > Cc: Maxime Ripard <mripard@kernel.org>
> > Cc: Thomas Zimmermann <tzimmermann@suse.de>
> > Cc: David Airlie <airlied@linux.ie>
> > Cc: Daniel Vetter <daniel@ffwll.ch>
> > Cc: dri-devel@lists.freedesktop.org
> > Cc: <stable@vger.kernel.org> # v5.2+
> 
> Reviewed-by: Christian König <christian.koenig@amd.com>

Pushed to drm-misc-fixes, thanks for reviewing.
-Daniel

> 
> > ---
> >   drivers/gpu/drm/drm_syncobj.c | 8 +++++---
> >   1 file changed, 5 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c
> > index 6e74e6745eca..349146049849 100644
> > --- a/drivers/gpu/drm/drm_syncobj.c
> > +++ b/drivers/gpu/drm/drm_syncobj.c
> > @@ -388,19 +388,18 @@ int drm_syncobj_find_fence(struct drm_file *file_private,
> >   		return -ENOENT;
> >   	*fence = drm_syncobj_fence_get(syncobj);
> > -	drm_syncobj_put(syncobj);
> >   	if (*fence) {
> >   		ret = dma_fence_chain_find_seqno(fence, point);
> >   		if (!ret)
> > -			return 0;
> > +			goto out;
> >   		dma_fence_put(*fence);
> >   	} else {
> >   		ret = -EINVAL;
> >   	}
> >   	if (!(flags & DRM_SYNCOBJ_WAIT_FLAGS_WAIT_FOR_SUBMIT))
> > -		return ret;
> > +		goto out;
> >   	memset(&wait, 0, sizeof(wait));
> >   	wait.task = current;
> > @@ -432,6 +431,9 @@ int drm_syncobj_find_fence(struct drm_file *file_private,
> >   	if (wait.node.next)
> >   		drm_syncobj_remove_wait(syncobj, &wait);
> > +out:
> > +	drm_syncobj_put(syncobj);
> > +
> >   	return ret;
> >   }
> >   EXPORT_SYMBOL(drm_syncobj_find_fence);
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 3+ messages in thread
end of thread, other threads:[~2021-01-20  9:29 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-01-19 13:03 [PATCH] drm/syncobj: Fix use-after-free Daniel Vetter
2021-01-19 13:08 ` Christian König
2021-01-20  9:28   ` Daniel Vetter

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).