| Message ID | 20240125180321.41173-1-ekurzinger@nvidia.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/syncobj: handle NULL fence in syncobj_eventfd_entry_func | expand |
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);
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);
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(-)