Module: Mesa Branch: main Commit: b06f6e00fba6e33c28a198a1bb14b89e9dfbb4ae URL: http://cgit.freedesktop.org/mesa/mesa/commit/?id=b06f6e00fba6e33c28a198a1bb14b89e9dfbb4ae
Author: Karol Herbst <kher...@redhat.com> Date: Thu Jan 4 02:42:16 2024 +0100 zink: fix heap-use-after-free on batch_state with sub-allocated pipe_resources zink_bo_create can run into a heap-use-after-free when the bo is still referencing an batch_state from an older destroyed context. In order to fix this, every context gives back their batch_states to the zink, where they can be reused from for new contexts. Cc: mesa-stable Suggested-by: Mike Blumenkrantz <michael.blumenkra...@gmail.com> Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/26889> --- src/gallium/drivers/zink/zink_batch.c | 12 ++++++++++++ src/gallium/drivers/zink/zink_context.c | 29 +++++++++++++++++++++++++++-- src/gallium/drivers/zink/zink_screen.c | 7 +++++++ src/gallium/drivers/zink/zink_types.h | 4 ++++ 4 files changed, 50 insertions(+), 2 deletions(-) diff --git a/src/gallium/drivers/zink/zink_batch.c b/src/gallium/drivers/zink/zink_batch.c index ec6b730ab7e..769ba4f212c 100644 --- a/src/gallium/drivers/zink/zink_batch.c +++ b/src/gallium/drivers/zink/zink_batch.c @@ -446,6 +446,18 @@ get_batch_state(struct zink_context *ctx, struct zink_batch *batch) if (bs == ctx->last_free_batch_state) ctx->last_free_batch_state = NULL; } + /* try from the ones that are given back to the screen next */ + if (!bs) { + simple_mtx_lock(&screen->free_batch_states_lock); + if (screen->free_batch_states) { + bs = screen->free_batch_states; + bs->ctx = ctx; + screen->free_batch_states = bs->next; + if (bs == screen->last_free_batch_state) + screen->last_free_batch_state = NULL; + } + simple_mtx_unlock(&screen->free_batch_states_lock); + } if (!bs && ctx->batch_states) { /* states are stored sequentially, so if the first one doesn't work, none of them will */ if (zink_screen_check_last_finished(screen, ctx->batch_states->fence.batch_id) || diff --git a/src/gallium/drivers/zink/zink_context.c b/src/gallium/drivers/zink/zink_context.c index e1569aefde4..f483613ee79 100644 --- a/src/gallium/drivers/zink/zink_context.c +++ b/src/gallium/drivers/zink/zink_context.c @@ -162,16 +162,41 @@ zink_context_destroy(struct pipe_context *pctx) while (bs) { struct zink_batch_state *bs_next = bs->next; zink_clear_batch_state(ctx, bs); - zink_batch_state_destroy(screen, bs); + /* restore link as we insert them into the screens free_batch_states + * list below + */ + bs->next = bs_next; bs = bs_next; } bs = ctx->free_batch_states; while (bs) { struct zink_batch_state *bs_next = bs->next; zink_clear_batch_state(ctx, bs); - zink_batch_state_destroy(screen, bs); + /* restore link as we insert them into the screens free_batch_states + * list below + */ + bs->next = bs_next; bs = bs_next; } + simple_mtx_lock(&screen->free_batch_states_lock); + if (ctx->batch_states) { + if (screen->free_batch_states) + screen->last_free_batch_state->next = ctx->batch_states; + else { + screen->free_batch_states = ctx->batch_states; + screen->last_free_batch_state = screen->free_batch_states; + } + while (screen->last_free_batch_state->next) + screen->last_free_batch_state = screen->last_free_batch_state->next; + } + if (ctx->free_batch_states) { + if (screen->free_batch_states) + screen->last_free_batch_state->next = ctx->free_batch_states; + else + screen->free_batch_states = ctx->free_batch_states; + screen->last_free_batch_state = ctx->last_free_batch_state; + } + simple_mtx_unlock(&screen->free_batch_states_lock); if (ctx->batch.state) { zink_clear_batch_state(ctx, ctx->batch.state); zink_batch_state_destroy(screen, ctx->batch.state); diff --git a/src/gallium/drivers/zink/zink_screen.c b/src/gallium/drivers/zink/zink_screen.c index 66d1361f1de..aa92c3d2be8 100644 --- a/src/gallium/drivers/zink/zink_screen.c +++ b/src/gallium/drivers/zink/zink_screen.c @@ -1463,6 +1463,12 @@ static void zink_destroy_screen(struct pipe_screen *pscreen) { struct zink_screen *screen = zink_screen(pscreen); + struct zink_batch_state *bs = screen->free_batch_states; + while (bs) { + struct zink_batch_state *bs_next = bs->next; + zink_batch_state_destroy(screen, bs); + bs = bs_next; + } #ifdef HAVE_RENDERDOC_APP_H if (screen->renderdoc_capture_all && p_atomic_dec_zero(&num_screens)) @@ -3498,6 +3504,7 @@ zink_internal_create_screen(const struct pipe_screen_config *config, int64_t dev screen->base_descriptor_size = MAX4(screen->db_size[0], screen->db_size[1], screen->db_size[2], screen->db_size[3]); } + simple_mtx_init(&screen->free_batch_states_lock, mtx_plain); simple_mtx_init(&screen->dt_lock, mtx_plain); util_idalloc_mt_init_tc(&screen->buffer_ids); diff --git a/src/gallium/drivers/zink/zink_types.h b/src/gallium/drivers/zink/zink_types.h index c0e9f9cd3a8..9ed132ddebc 100644 --- a/src/gallium/drivers/zink/zink_types.h +++ b/src/gallium/drivers/zink/zink_types.h @@ -1416,6 +1416,10 @@ struct zink_screen { simple_mtx_t copy_context_lock; struct zink_context *copy_context; + struct zink_batch_state *free_batch_states; //unused batch states + struct zink_batch_state *last_free_batch_state; //for appending + simple_mtx_t free_batch_states_lock; + simple_mtx_t semaphores_lock; struct util_dynarray semaphores; struct util_dynarray fd_semaphores;
