Module: Mesa Branch: staging/23.3 Commit: 52255e201e1859ef695d2df555d36c5602a85d66 URL: http://cgit.freedesktop.org/mesa/mesa/commit/?id=52255e201e1859ef695d2df555d36c5602a85d66
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> (cherry picked from commit b06f6e00fba6e33c28a198a1bb14b89e9dfbb4ae) --- .pick_status.json | 2 +- 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 ++++ 5 files changed, 51 insertions(+), 3 deletions(-) diff --git a/.pick_status.json b/.pick_status.json index b65c46e2e2c..d03f93bb351 100644 --- a/.pick_status.json +++ b/.pick_status.json @@ -1054,7 +1054,7 @@ "description": "zink: fix heap-use-after-free on batch_state with sub-allocated pipe_resources", "nominated": true, "nomination_type": 0, - "resolution": 0, + "resolution": 1, "main_sha": null, "because_sha": null, "notes": null diff --git a/src/gallium/drivers/zink/zink_batch.c b/src/gallium/drivers/zink/zink_batch.c index 7e3b268fc0c..533d3e0abc4 100644 --- a/src/gallium/drivers/zink/zink_batch.c +++ b/src/gallium/drivers/zink/zink_batch.c @@ -414,6 +414,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 db60dd5e1d1..f4716527883 100644 --- a/src/gallium/drivers/zink/zink_context.c +++ b/src/gallium/drivers/zink/zink_context.c @@ -161,16 +161,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 97af50f68c0..bfb4fe59d74 100644 --- a/src/gallium/drivers/zink/zink_screen.c +++ b/src/gallium/drivers/zink/zink_screen.c @@ -1461,6 +1461,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)) @@ -3496,6 +3502,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 b1aff033b76..b6477e3244a 100644 --- a/src/gallium/drivers/zink/zink_types.h +++ b/src/gallium/drivers/zink/zink_types.h @@ -1401,6 +1401,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;
