From e4359cc49ccf89de9171e2a6b429a2be9de875b3 Mon Sep 17 00:00:00 2001 From: Mark Collins Date: Mon, 31 Mar 2025 19:56:37 +0000 Subject: [PATCH] tu/kgsl: Fix KGSL syncobj lifetime in no CB submit The temporary syncobj created in the fast path of kgsl_queue_submit was not being destroyed, and potentially being assigned to multiple syncobjs without being properly duplicated. This could lead to a use-after-free or double-free since multiple syncobjs could be assigned the same FD. Signed-off-by: Mark Collins Part-of: --- src/freedreno/vulkan/tu_knl_kgsl.cc | 36 +++++++++++++++++++++++++---- 1 file changed, 31 insertions(+), 5 deletions(-) diff --git a/src/freedreno/vulkan/tu_knl_kgsl.cc b/src/freedreno/vulkan/tu_knl_kgsl.cc index c5bf255935c..ead4ae420e1 100644 --- a/src/freedreno/vulkan/tu_knl_kgsl.cc +++ b/src/freedreno/vulkan/tu_knl_kgsl.cc @@ -463,12 +463,11 @@ kgsl_syncobj_reset(struct kgsl_syncobj *s) if (s->state == KGSL_SYNCOBJ_STATE_FD && s->fd >= 0) { ASSERTED int ret = close(s->fd); assert(ret == 0); - s->fd = -1; - } else if (s->state == KGSL_SYNCOBJ_STATE_TS) { - s->timestamp = UINT32_MAX; } s->state = KGSL_SYNCOBJ_STATE_UNSIGNALED; + s->timestamp = UINT32_MAX; + s->fd = -1; } static void @@ -477,6 +476,17 @@ kgsl_syncobj_destroy(struct kgsl_syncobj *s) kgsl_syncobj_reset(s); } +static struct kgsl_syncobj +kgsl_syncobj_dup(struct kgsl_syncobj *s) +{ + struct kgsl_syncobj dups = *s; + if (s->state == KGSL_SYNCOBJ_STATE_FD && s->fd >= 0) { + dups.fd = dup(s->fd); + assert(dups.fd >= 0); + } + return dups; +} + static int timestamp_to_fd(struct tu_queue *queue, uint32_t timestamp) { @@ -1122,15 +1132,31 @@ kgsl_queue_submit(struct tu_queue *queue, void *_submit, assert(wait_sync.state != KGSL_SYNCOBJ_STATE_UNSIGNALED); // Would wait forever - for (uint32_t i = 0; i < signal_count; i++) { + if (signal_count == 1) { + /* Move instead of duplicating the syncobj, as we don't need to + * keep the original one around. + */ struct kgsl_syncobj *signal_sync = - &container_of(signals[i].sync, struct vk_kgsl_syncobj, vk) + &container_of(signals[0].sync, struct vk_kgsl_syncobj, vk) ->syncobj; kgsl_syncobj_reset(signal_sync); *signal_sync = wait_sync; + } else { + for (uint32_t i = 0; i < signal_count; i++) { + struct kgsl_syncobj *signal_sync = + &container_of(signals[i].sync, struct vk_kgsl_syncobj, vk) + ->syncobj; + + kgsl_syncobj_reset(signal_sync); + *signal_sync = kgsl_syncobj_dup(&wait_sync); + } + + kgsl_syncobj_destroy(&wait_sync); } + kgsl_syncobj_destroy(&last_submit_sync); + return VK_SUCCESS; }