mirror of
https://gitlab.freedesktop.org/mesa/mesa.git
synced 2026-01-24 01:40:22 +01:00
mesa: Fix segfaults in _mesa_delete_program and _mesa_reference_program_
`_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 with492a176cbb("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:ca16c271fa("mesa: make struct in gl_program a union and remove FIXME") Reviewed-by: Marek Olšák <marek.olsak@amd.com> Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/39243>
This commit is contained in:
parent
0a3f3fd193
commit
884cf1d39e
2 changed files with 11 additions and 3 deletions
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue