[PATCH] drm/syncobj: handle NULL fence in syncobj_eventfd_entry_func

[PATCH] drm/syncobj: handle NULL fence in syncobj_eventfd_entry_func

[Erik Kurzinger]

During syncobj_eventfd_entry_func, dma_fence_chain_find_seqno may set
the fence to NULL if the given seqno is signaled and a later seqno has
already been submitted. In that case, the eventfd should be signaled
immediately which currently does not happen.

This is a similar issue to the one addressed by b19926d4f3a6
("drm/syncobj: Deal with signalled fences in drm_syncobj_find_fence")

As a fix, if the return value of dma_fence_chain_find_seqno indicates
success but it sets the fence to NULL, we should simply signal the
eventfd immediately.

Signed-off-by: Erik Kurzinger <ekurzinger@nvidia.com>
Fixes: c7a472297169 ("drm/syncobj: add IOCTL to register an eventfd")
---
 drivers/gpu/drm/drm_syncobj.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c
index e04965878a08..cc3af1084950 100644
--- a/drivers/gpu/drm/drm_syncobj.c
+++ b/drivers/gpu/drm/drm_syncobj.c
@@ -1441,10 +1441,21 @@ syncobj_eventfd_entry_func(struct drm_syncobj *syncobj,
 
 	/* This happens inside the syncobj lock */
 	fence = dma_fence_get(rcu_dereference_protected(syncobj->fence, 1));
+	if (!fence)
+		return;
+
 	ret = dma_fence_chain_find_seqno(&fence, entry->point);
-	if (ret != 0 || !fence) {
+	if (ret != 0) {
+		/* The given seqno has not been submitted yet. */
 		dma_fence_put(fence);
 		return;
+	} else if (!fence) {
+		/* If dma_fence_chain_find_seqno returns 0 but sets the fence
+		 * to NULL, it implies that the given seqno is signaled and a
+		 * later seqno has already been submitted. Signal the eventfd
+		 * immediately in that case. */
+		eventfd_signal(entry->ev_fd_ctx, 1);
+		syncobj_eventfd_entry_free(entry);
 	}
 
 	list_del_init(&entry->node);
-- 
2.43.0

Re: [PATCH] drm/syncobj: handle NULL fence in syncobj_eventfd_entry_func

[Erik Kurzinger]

Sorry, I realized there is a mistake in this patch after sending it out. It results in a use-after-free of "entry". I've sent out an updated version which should avoid the issue.

On 1/25/24 10:03, Erik Kurzinger wrote:
> During syncobj_eventfd_entry_func, dma_fence_chain_find_seqno may set
> the fence to NULL if the given seqno is signaled and a later seqno has
> already been submitted. In that case, the eventfd should be signaled
> immediately which currently does not happen.
> 
> This is a similar issue to the one addressed by b19926d4f3a6
> ("drm/syncobj: Deal with signalled fences in drm_syncobj_find_fence")
> 
> As a fix, if the return value of dma_fence_chain_find_seqno indicates
> success but it sets the fence to NULL, we should simply signal the
> eventfd immediately.
> 
> Signed-off-by: Erik Kurzinger <ekurzinger@nvidia.com>
> Fixes: c7a472297169 ("drm/syncobj: add IOCTL to register an eventfd")
> ---
>  drivers/gpu/drm/drm_syncobj.c | 13 ++++++++++++-
>  1 file changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c
> index e04965878a08..cc3af1084950 100644
> --- a/drivers/gpu/drm/drm_syncobj.c
> +++ b/drivers/gpu/drm/drm_syncobj.c
> @@ -1441,10 +1441,21 @@ syncobj_eventfd_entry_func(struct drm_syncobj *syncobj,
>  
>  	/* This happens inside the syncobj lock */
>  	fence = dma_fence_get(rcu_dereference_protected(syncobj->fence, 1));
> +	if (!fence)
> +		return;
> +
>  	ret = dma_fence_chain_find_seqno(&fence, entry->point);
> -	if (ret != 0 || !fence) {
> +	if (ret != 0) {
> +		/* The given seqno has not been submitted yet. */
>  		dma_fence_put(fence);
>  		return;
> +	} else if (!fence) {
> +		/* If dma_fence_chain_find_seqno returns 0 but sets the fence
> +		 * to NULL, it implies that the given seqno is signaled and a
> +		 * later seqno has already been submitted. Signal the eventfd
> +		 * immediately in that case. */
> +		eventfd_signal(entry->ev_fd_ctx, 1);
> +		syncobj_eventfd_entry_free(entry);
>  	}
>  
>  	list_del_init(&entry->node);