From 884cf1d39e574bccc40155541bf4faa25db34fbd Mon Sep 17 00:00:00 2001 From: Ian Romanick Date: Fri, 9 Jan 2026 10:51:59 -0800 Subject: [PATCH] mesa: Fix segfaults in _mesa_delete_program and _mesa_reference_program_ MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `_mesa_delete_program` and `_mesa_reference_program_` both use fields in `sh` without determining whether `sh` or `arb` is valid. Herein lies the problem. I cannot see any reliable way to determine which is valid without knowing what call path you are in. There are two possible methods, but neither seems particularly reliable. We could check whether `shader_program` is `NULL`, or we could check whether `Parameters` (only used for ARB assembly shaders) is `NULL`. Instead of doing that, I chose to add a new field that captures the `is_arb_asm` parameter already passed to `_mesa_init_gl_program`. This seemed the most reliable. It is possible that more `assert(!prog->is_arb_asm)` and `assert(prog->is_arb_asm)` should be sprinkled throughout the code base. I don't know how this has not already been a problem. Starting with 492a176cbb2 ("util: increase SHA1_DIGEST_LENGTH to 32 (BLAKE3_KEY_LEN)"), I observed segfaults in `_mesa_uniform_detach_all_driver_storage` while freeing ARB assembly shaders. Those shaders should never hit this path. Not all assembly shaders hit this path. In fact, **nothing** in the open shader-db encounters this problem. This is presumably why pre-merge CI didn't catch this problem. In my closed shader-db, shaders from the following applications hit this: - rocketbirds-hardboiled-chicken - shadowrun-returns - windward - ziggurat I believe it was just blind luck that most of the time the fields of `sh::data` that were access through this path, `sh::BindlessSamplers`, and `sh::BindlessImages` happened to line up with fields of `arb` that contained `NULL`. Changing the size of the hash key changed that luck. Fixes: ca16c271fa7 ("mesa: make struct in gl_program a union and remove FIXME") Reviewed-by: Marek Olšák Part-of: --- src/mesa/main/shader_types.h | 3 +++ src/mesa/program/program.c | 11 ++++++++--- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/src/mesa/main/shader_types.h b/src/mesa/main/shader_types.h index b672a50ae76..3668c68a052 100644 --- a/src/mesa/main/shader_types.h +++ b/src/mesa/main/shader_types.h @@ -508,6 +508,9 @@ struct gl_program /** whether to skip VARYING_SLOT_PSIZ in st_translate_stream_output_info() */ bool skip_pointsize_xfb; + /** Determine whether ::sh or ::arb (below) is valid. */ + bool is_arb_asm; + /** A bitfield indicating which vertex shader inputs consume two slots * * This is used for mapping from single-slot input locations in the GL API diff --git a/src/mesa/program/program.c b/src/mesa/program/program.c index fe8c7674db9..78cdb7973af 100644 --- a/src/mesa/program/program.c +++ b/src/mesa/program/program.c @@ -197,6 +197,7 @@ _mesa_init_gl_program(struct gl_program *prog, mesa_shader_stage stage, prog->Format = GL_PROGRAM_FORMAT_ASCII_ARB; prog->info.stage = stage; prog->info.use_legacy_math_rules = is_arb_asm; + prog->is_arb_asm = is_arb_asm; /* Uniforms that lack an initializer in the shader code have an initial * value of zero. This includes sampler uniforms. @@ -260,8 +261,10 @@ _mesa_delete_program(struct gl_context *ctx, struct gl_program *prog) } ralloc_free(prog->nir); - ralloc_free(prog->sh.BindlessSamplers); - ralloc_free(prog->sh.BindlessImages); + if (!prog->is_arb_asm) { + ralloc_free(prog->sh.BindlessSamplers); + ralloc_free(prog->sh.BindlessImages); + } ralloc_free(prog->driver_cache_blob); ralloc_free(prog); } @@ -315,7 +318,8 @@ _mesa_reference_program_(struct gl_context *ctx, if (p_atomic_dec_zero(&oldProg->RefCount)) { assert(ctx); - _mesa_reference_shader_program_data(&oldProg->sh.data, NULL); + if (!oldProg->is_arb_asm) + _mesa_reference_shader_program_data(&oldProg->sh.data, NULL); _mesa_delete_program(ctx, oldProg); } @@ -374,6 +378,7 @@ gl_external_samplers(const struct gl_program *prog) GLbitfield external_samplers = 0; GLbitfield mask = prog->SamplersUsed; + assert(!prog->is_arb_asm); while (mask) { int idx = u_bit_scan(&mask); if (prog->sh.SamplerTargets[idx] == TEXTURE_EXTERNAL_INDEX)