From f150c50170461fec759b8818e71cb2a7bf667eae Mon Sep 17 00:00:00 2001 From: Timothy Arceri Date: Fri, 21 Mar 2025 12:06:41 +1100 Subject: [PATCH] mesa: fix potential race condition in with RenderBuffers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The calls look up a renderbuffer and create it if it doesn't already exist. However they weren't locking the hash between looking up the name and adding it to the hash so it could be possible another thread also generated the same name. Reviewed-by: Marek Olšák Reviewed-by: Pierre-Eric Pelloux-Prayer Fixes: 842c91300fd0 ("mesa: enable GL name reuse by default for all drivers except virgl") Part-of: (cherry picked from commit 0e61d31e9d9072482db64973d8a9154a7f4e6e45) --- .pick_status.json | 2 +- src/mesa/main/fbobject.c | 43 +++++++++++++++++++++++++++++----------- 2 files changed, 32 insertions(+), 13 deletions(-) diff --git a/.pick_status.json b/.pick_status.json index 55c63eb6700..de11c2aad1e 100644 --- a/.pick_status.json +++ b/.pick_status.json @@ -4064,7 +4064,7 @@ "description": "mesa: fix potential race condition in with RenderBuffers", "nominated": true, "nomination_type": 2, - "resolution": 0, + "resolution": 1, "main_sha": null, "because_sha": "842c91300fd041cf35398d05d2995f16d76870ae", "notes": null diff --git a/src/mesa/main/fbobject.c b/src/mesa/main/fbobject.c index 99f5922fe4d..d2343537eca 100644 --- a/src/mesa/main/fbobject.c +++ b/src/mesa/main/fbobject.c @@ -109,6 +109,17 @@ _mesa_get_incomplete_framebuffer(void) return &IncompleteFramebuffer; } +static struct gl_renderbuffer * +_mesa_lookup_renderbuffer_locked(struct gl_context *ctx, GLuint id) +{ + if (id == 0) + return NULL; + + struct gl_renderbuffer *rb = (struct gl_renderbuffer *) + _mesa_HashLookupLocked(&ctx->Shared->RenderBuffers, id); + return rb; +} + /** * Helper routine for getting a gl_renderbuffer. */ @@ -1799,7 +1810,8 @@ bind_renderbuffer(GLenum target, GLuint renderbuffer) */ if (renderbuffer) { - newRb = _mesa_lookup_renderbuffer(ctx, renderbuffer); + _mesa_HashLockMutex(&ctx->Shared->RenderBuffers); + newRb = _mesa_lookup_renderbuffer_locked(ctx, renderbuffer); if (newRb == &DummyRenderbuffer) { /* ID was reserved, but no real renderbuffer object made yet */ newRb = NULL; @@ -1808,15 +1820,15 @@ bind_renderbuffer(GLenum target, GLuint renderbuffer) /* All RB IDs must be Gen'd */ _mesa_error(ctx, GL_INVALID_OPERATION, "glBindRenderbuffer(non-gen name)"); + _mesa_HashUnlockMutex(&ctx->Shared->RenderBuffers); return; } if (!newRb) { - _mesa_HashLockMutex(&ctx->Shared->RenderBuffers); newRb = allocate_renderbuffer_locked(ctx, renderbuffer, "glBindRenderbufferEXT"); - _mesa_HashUnlockMutex(&ctx->Shared->RenderBuffers); } + _mesa_HashUnlockMutex(&ctx->Shared->RenderBuffers); } else { newRb = NULL; @@ -3046,13 +3058,16 @@ _mesa_NamedRenderbufferStorageEXT(GLuint renderbuffer, GLenum internalformat, GLsizei width, GLsizei height) { GET_CURRENT_CONTEXT(ctx); - struct gl_renderbuffer *rb = _mesa_lookup_renderbuffer(ctx, renderbuffer); + + _mesa_HashLockMutex(&ctx->Shared->RenderBuffers); + struct gl_renderbuffer *rb = + _mesa_lookup_renderbuffer_locked(ctx, renderbuffer); if (!rb || rb == &DummyRenderbuffer) { - _mesa_HashLockMutex(&ctx->Shared->RenderBuffers); rb = allocate_renderbuffer_locked(ctx, renderbuffer, "glNamedRenderbufferStorageEXT"); - _mesa_HashUnlockMutex(&ctx->Shared->RenderBuffers); } + _mesa_HashUnlockMutex(&ctx->Shared->RenderBuffers); + renderbuffer_storage(ctx, rb, internalformat, width, height, NO_SAMPLES, 0, "glNamedRenderbufferStorageEXT"); } @@ -3075,13 +3090,16 @@ _mesa_NamedRenderbufferStorageMultisampleEXT(GLuint renderbuffer, GLsizei sample GLsizei width, GLsizei height) { GET_CURRENT_CONTEXT(ctx); - struct gl_renderbuffer *rb = _mesa_lookup_renderbuffer(ctx, renderbuffer); + + _mesa_HashLockMutex(&ctx->Shared->RenderBuffers); + struct gl_renderbuffer *rb = + _mesa_lookup_renderbuffer_locked(ctx, renderbuffer); if (!rb || rb == &DummyRenderbuffer) { - _mesa_HashLockMutex(&ctx->Shared->RenderBuffers); rb = allocate_renderbuffer_locked(ctx, renderbuffer, "glNamedRenderbufferStorageMultisampleEXT"); - _mesa_HashUnlockMutex(&ctx->Shared->RenderBuffers); } + _mesa_HashUnlockMutex(&ctx->Shared->RenderBuffers); + renderbuffer_storage(ctx, rb, internalformat, width, height, samples, samples, "glNamedRenderbufferStorageMultisample"); @@ -3193,13 +3211,14 @@ _mesa_GetNamedRenderbufferParameterivEXT(GLuint renderbuffer, GLenum pname, { GET_CURRENT_CONTEXT(ctx); - struct gl_renderbuffer *rb = _mesa_lookup_renderbuffer(ctx, renderbuffer); + _mesa_HashLockMutex(&ctx->Shared->RenderBuffers); + struct gl_renderbuffer *rb = + _mesa_lookup_renderbuffer_locked(ctx, renderbuffer); if (!rb || rb == &DummyRenderbuffer) { - _mesa_HashLockMutex(&ctx->Shared->RenderBuffers); rb = allocate_renderbuffer_locked(ctx, renderbuffer, "glGetNamedRenderbufferParameterivEXT"); - _mesa_HashUnlockMutex(&ctx->Shared->RenderBuffers); } + _mesa_HashUnlockMutex(&ctx->Shared->RenderBuffers); get_render_buffer_parameteriv(ctx, rb, pname, params, "glGetNamedRenderbufferParameterivEXT");