From b2c340c106e8195f509d0c096784baf2966a7d65 Mon Sep 17 00:00:00 2001 From: Patrick Lerda Date: Wed, 8 Feb 2023 15:28:08 +0100 Subject: [PATCH] mesa/st: fix possible crash related to arb invalid memory access MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This invalid memory access is a consequence of wrong assumptions, for instance: "prog->sh.data is NULL if it's ARB_fragment_program" This issue is triggered with piglit/fp-formats -auto -fbo: ==9747==ERROR: AddressSanitizer: heap-use-after-free on address 0x007f7c812d90 at pc 0x007f833c09f8 bp 0x007fd7eca750 sp 0x007fd7eca768 READ of size 4 at 0x007f7c812d90 thread T0 #0 0x7f833c09f4 in st_get_sampler_views ../src/mesa/state_tracker/st_atom_texture.c:109 #1 0x7f833c0b48 in update_textures ../src/mesa/state_tracker/st_atom_texture.c:266 #2 0x7f82b2d120 in st_validate_state ../src/mesa/state_tracker/st_util.h:128 #3 0x7f82b2d120 in prepare_draw ../src/mesa/state_tracker/st_draw.c:88 #4 0x7f82b2de64 in st_draw_gallium ../src/mesa/state_tracker/st_draw.c:141 #5 0x7f83105940 in _mesa_draw_arrays ../src/mesa/main/draw.c:1202 #6 0x7f8d5fa5cc in piglit_draw_rect_from_arrays piglit/tests/util/piglit-util-gl.c:711 #7 0x7f8d5fac34 in piglit_draw_rect_custom piglit/tests/util/piglit-util-gl.c:833 #8 0x4019e0 in piglit_display piglit/tests/shaders/fp-formats.c:67 #9 0x7f8d643fc4 in run_test piglit/tests/util/piglit-framework-gl/piglit_fbo_framework.c:52 #10 0x401624 in main piglit/tests/shaders/fp-formats.c:39 Signed-off-by: Patrick Lerda Reviewed-by: Marek Olšák Part-of: --- src/compiler/glsl/gl_nir_link_varyings.c | 2 +- src/compiler/glsl/gl_nir_linker.c | 9 ++++----- src/compiler/glsl/link_interface_blocks.cpp | 8 ++++---- src/compiler/glsl/link_varyings.cpp | 8 ++++---- src/compiler/glsl/linker.cpp | 18 +++++++++--------- src/compiler/glsl/serialize.cpp | 4 ++-- src/mesa/main/shader_types.h | 4 ++-- src/mesa/main/shaderapi.c | 4 ++-- src/mesa/state_tracker/st_atom_sampler.c | 5 +++-- src/mesa/state_tracker/st_atom_texture.c | 5 ++--- src/mesa/state_tracker/st_texture.c | 8 ++++---- 11 files changed, 37 insertions(+), 38 deletions(-) diff --git a/src/compiler/glsl/gl_nir_link_varyings.c b/src/compiler/glsl/gl_nir_link_varyings.c index e91a24212fda..a4c6a0f7403c 100644 --- a/src/compiler/glsl/gl_nir_link_varyings.c +++ b/src/compiler/glsl/gl_nir_link_varyings.c @@ -2219,7 +2219,7 @@ remove_unused_io_vars(nir_shader *producer, nir_shader *consumer, progress = true; if (mode == nir_var_shader_in) { - if (!prog->IsES && prog->data->Version <= 120) { + if (!prog->IsES && prog->GLSL_Version <= 120) { /* On page 25 (page 31 of the PDF) of the GLSL 1.20 spec: * * Only those varying variables used (i.e. read) in diff --git a/src/compiler/glsl/gl_nir_linker.c b/src/compiler/glsl/gl_nir_linker.c index 998ca74f0cb7..ad6c185efc9e 100644 --- a/src/compiler/glsl/gl_nir_linker.c +++ b/src/compiler/glsl/gl_nir_linker.c @@ -904,12 +904,11 @@ validate_sampler_array_indexing(const struct gl_constants *consts, "expressions is forbidden in GLSL %s %u"; /* Backend has indicated that it has no dynamic indexing support. */ if (no_dynamic_indexing) { - linker_error(prog, msg, prog->IsES ? "ES" : "", - prog->data->Version); + linker_error(prog, msg, prog->IsES ? "ES" : "", prog->GLSL_Version); return false; } else { linker_warning(prog, msg, prog->IsES ? "ES" : "", - prog->data->Version); + prog->GLSL_Version); } } } @@ -933,8 +932,8 @@ gl_nir_link_glsl(const struct gl_constants *consts, * with loop induction variable. This check emits a warning or error * depending if backend can handle dynamic indexing. */ - if ((!prog->IsES && prog->data->Version < 130) || - (prog->IsES && prog->data->Version < 300)) { + if ((!prog->IsES && prog->GLSL_Version < 130) || + (prog->IsES && prog->GLSL_Version < 300)) { if (!validate_sampler_array_indexing(consts, prog)) return false; } diff --git a/src/compiler/glsl/link_interface_blocks.cpp b/src/compiler/glsl/link_interface_blocks.cpp index 61e6a1f02645..aa24993ce44b 100644 --- a/src/compiler/glsl/link_interface_blocks.cpp +++ b/src/compiler/glsl/link_interface_blocks.cpp @@ -69,7 +69,7 @@ interstage_member_mismatch(struct gl_shader_program *prog, * interpolation qualifiers of variables of the same name do not * match." */ - if (prog->IsES || prog->data->Version < 440) + if (prog->IsES || prog->GLSL_Version < 440) if (c->fields.structure[i].interpolation != p->fields.structure[i].interpolation) return true; @@ -88,7 +88,7 @@ interstage_member_mismatch(struct gl_shader_program *prog, * The table in Section 9.2.1 Linked Shaders of the GLSL ES 3.2 spec * says that sample need not match for varyings. */ - if (!prog->IsES || prog->data->Version < 310) + if (!prog->IsES || prog->GLSL_Version < 310) if (c->fields.structure[i].centroid != p->fields.structure[i].centroid) return true; @@ -456,7 +456,7 @@ validate_interstage_inout_blocks(struct gl_shader_program *prog, continue; /* Built-in interface redeclaration check. */ - if (prog->SeparateShader && !prog->IsES && prog->data->Version >= 150 && + if (prog->SeparateShader && !prog->IsES && prog->GLSL_Version >= 150 && var->data.how_declared == ir_var_declared_implicitly && var->data.used && !producer_iface) { linker_error(prog, "missing output builtin block %s redeclaration " @@ -477,7 +477,7 @@ validate_interstage_inout_blocks(struct gl_shader_program *prog, ir_variable *producer_def = definitions.lookup(var); /* Built-in interface redeclaration check. */ - if (prog->SeparateShader && !prog->IsES && prog->data->Version >= 150 && + if (prog->SeparateShader && !prog->IsES && prog->GLSL_Version >= 150 && var->data.how_declared == ir_var_declared_implicitly && var->data.used && !producer_iface) { linker_error(prog, "missing input builtin block %s redeclaration " diff --git a/src/compiler/glsl/link_varyings.cpp b/src/compiler/glsl/link_varyings.cpp index 5a4fedc300c5..0ef618ac1146 100644 --- a/src/compiler/glsl/link_varyings.cpp +++ b/src/compiler/glsl/link_varyings.cpp @@ -148,7 +148,7 @@ cross_validate_types_and_qualifiers(const struct gl_constants *consts, * OpenGLES 3.0 drivers, so we relax the checking in all cases. */ if (false /* always skip the centroid check */ && - prog->data->Version < (prog->IsES ? 310 : 430) && + prog->GLSL_Version < (prog->IsES ? 310 : 430) && input->data.centroid != output->data.centroid) { linker_error(prog, "%s shader output `%s' %s centroid qualifier, " @@ -203,7 +203,7 @@ cross_validate_types_and_qualifiers(const struct gl_constants *consts, * and fragment shaders must match." */ if (input->data.explicit_invariant != output->data.explicit_invariant && - prog->data->Version < (prog->IsES ? 300 : 420)) { + prog->GLSL_Version < (prog->IsES ? 300 : 420)) { linker_error(prog, "%s shader output `%s' %s invariant qualifier, " "but %s shader input %s invariant qualifier\n", @@ -240,7 +240,7 @@ cross_validate_types_and_qualifiers(const struct gl_constants *consts, output_interpolation = INTERP_MODE_SMOOTH; } if (input_interpolation != output_interpolation && - prog->data->Version < 440) { + prog->GLSL_Version < 440) { if (!consts->AllowGLSLCrossStageInterpolationMismatch) { linker_error(prog, "%s shader output `%s' specifies %s " @@ -642,7 +642,7 @@ validate_first_and_last_interface_explicit_locations(const struct gl_constants * static bool static_input_output_matching(struct gl_shader_program *prog) { - return prog->data->Version >= (prog->IsES ? 0 : 420); + return prog->GLSL_Version >= (prog->IsES ? 0 : 420); } /** diff --git a/src/compiler/glsl/linker.cpp b/src/compiler/glsl/linker.cpp index 4c0db3552169..53197a04b930 100644 --- a/src/compiler/glsl/linker.cpp +++ b/src/compiler/glsl/linker.cpp @@ -529,7 +529,7 @@ analyze_clip_cull_usage(struct gl_shader_program *prog, info->clip_distance_array_size = 0; info->cull_distance_array_size = 0; - if (prog->data->Version >= (prog->IsES ? 300 : 130)) { + if (prog->GLSL_Version >= (prog->IsES ? 300 : 130)) { /* From section 7.1 (Vertex Shader Special Variables) of the * GLSL 1.30 spec: * @@ -649,7 +649,7 @@ validate_vertex_shader_executable(struct gl_shader_program *prog, * All GLSL ES Versions are similar to GLSL 1.40--failing to write to * gl_Position is not an error. */ - if (prog->data->Version < (prog->IsES ? 300 : 140)) { + if (prog->GLSL_Version < (prog->IsES ? 300 : 140)) { find_variable gl_Position("gl_Position"); find_assignments(shader->ir, &gl_Position); if (!gl_Position.found) { @@ -1066,7 +1066,8 @@ cross_validate_globals(const struct gl_constants *consts, if (!consts->AllowGLSLRelaxedES && prog->IsES && !var->get_interface_type() && existing->data.precision != var->data.precision) { - if ((existing->data.used && var->data.used) || prog->data->Version >= 300) { + if ((existing->data.used && var->data.used) || + prog->GLSL_Version >= 300) { linker_error(prog, "declarations for %s `%s` have " "mismatching precision qualifiers\n", mode_string(var), var->name); @@ -1974,7 +1975,7 @@ link_fs_inout_layout_qualifiers(struct gl_shader_program *prog, bool pixel_center_integer = false; if (linked_shader->Stage != MESA_SHADER_FRAGMENT || - (prog->data->Version < 150 && + (prog->GLSL_Version < 150 && !prog->ARB_fragment_coord_conventions_enable)) return; @@ -2052,8 +2053,7 @@ link_gs_inout_layout_qualifiers(struct gl_shader_program *prog, /* No in/out qualifiers defined for anything but GLSL 1.50+ * geometry shaders so far. */ - if (gl_prog->info.stage != MESA_SHADER_GEOMETRY || - prog->data->Version < 150) + if (gl_prog->info.stage != MESA_SHADER_GEOMETRY || prog->GLSL_Version < 150) return; int vertices_out = -1; @@ -2963,7 +2963,7 @@ assign_attribute_or_color_locations(void *mem_ctx, } } } else if (target_index == MESA_SHADER_FRAGMENT || - (prog->IsES && prog->data->Version >= 300)) { + (prog->IsES && prog->GLSL_Version >= 300)) { linker_error(prog, "overlapping location is assigned " "to %s `%s' %d %d %d\n", string, var->name, used_locations, use_mask, attr); @@ -3629,7 +3629,7 @@ link_shaders(struct gl_context *ctx, struct gl_shader_program *prog) goto done; } - prog->data->Version = max_version; + prog->GLSL_Version = max_version; prog->IsES = prog->Shaders[0]->IsES; /* Some shaders have to be linked with some other shaders present. @@ -3817,7 +3817,7 @@ link_shaders(struct gl_context *ctx, struct gl_shader_program *prog) lower_named_interface_blocks(mem_ctx, prog->_LinkedShaders[i]); } - if (prog->IsES && prog->data->Version == 100) + if (prog->IsES && prog->GLSL_Version == 100) if (!validate_invariant_builtins(prog, prog->_LinkedShaders[MESA_SHADER_VERTEX], prog->_LinkedShaders[MESA_SHADER_FRAGMENT])) diff --git a/src/compiler/glsl/serialize.cpp b/src/compiler/glsl/serialize.cpp index c8b55d86af52..9e7a50ebea93 100644 --- a/src/compiler/glsl/serialize.cpp +++ b/src/compiler/glsl/serialize.cpp @@ -1259,7 +1259,7 @@ serialize_glsl_program(struct blob *blob, struct gl_context *ctx, write_hash_tables(blob, prog); - blob_write_uint32(blob, prog->data->Version); + blob_write_uint32(blob, prog->GLSL_Version); blob_write_uint32(blob, prog->IsES); blob_write_uint32(blob, prog->data->linked_stages); @@ -1318,7 +1318,7 @@ deserialize_glsl_program(struct blob_reader *blob, struct gl_context *ctx, read_hash_tables(blob, prog); - prog->data->Version = blob_read_uint32(blob); + prog->GLSL_Version = blob_read_uint32(blob); prog->IsES = blob_read_uint32(blob); prog->data->linked_stages = blob_read_uint32(blob); diff --git a/src/mesa/main/shader_types.h b/src/mesa/main/shader_types.h index 9222ae337494..3117a1d0dc0c 100644 --- a/src/mesa/main/shader_types.h +++ b/src/mesa/main/shader_types.h @@ -345,8 +345,6 @@ struct gl_shader_program_data enum gl_link_status LinkStatus; /**< GL_LINK_STATUS */ GLchar *InfoLog; - unsigned Version; /**< GLSL version used for linking */ - /* Mask of stages this program was linked against */ unsigned linked_stages; @@ -489,6 +487,8 @@ struct gl_shader_program * #extension ARB_fragment_coord_conventions: enable */ GLboolean ARB_fragment_coord_conventions_enable; + + unsigned GLSL_Version; /**< GLSL version used for linking */ }; /** diff --git a/src/mesa/main/shaderapi.c b/src/mesa/main/shaderapi.c index bce384fa5396..577308f20c8a 100644 --- a/src/mesa/main/shaderapi.c +++ b/src/mesa/main/shaderapi.c @@ -1389,8 +1389,8 @@ link_program(struct gl_context *ctx, struct gl_shader_program *shProg, } if (file) { fprintf(file, "[require]\nGLSL%s >= %u.%02u\n", - shProg->IsES ? " ES" : "", - shProg->data->Version / 100, shProg->data->Version % 100); + shProg->IsES ? " ES" : "", shProg->GLSL_Version / 100, + shProg->GLSL_Version % 100); if (shProg->SeparateShader) fprintf(file, "GL_ARB_separate_shader_objects\nSSO ENABLED\n"); fprintf(file, "\n"); diff --git a/src/mesa/state_tracker/st_atom_sampler.c b/src/mesa/state_tracker/st_atom_sampler.c index 1383e1d8c134..e0d3215a9442 100644 --- a/src/mesa/state_tracker/st_atom_sampler.c +++ b/src/mesa/state_tracker/st_atom_sampler.c @@ -225,8 +225,9 @@ update_shader_samplers(struct st_context *st, if (samplers_used & 1 && (ctx->Texture.Unit[tex_unit]._Current->Target != GL_TEXTURE_BUFFER || st->texture_buffer_sampler)) { - st_convert_sampler_from_unit(st, sampler, tex_unit, - prog->sh.data && prog->sh.data->Version >= 130); + st_convert_sampler_from_unit( + st, sampler, tex_unit, + prog->shader_program && prog->shader_program->GLSL_Version >= 130); states[unit] = sampler; } else { states[unit] = NULL; diff --git a/src/mesa/state_tracker/st_atom_texture.c b/src/mesa/state_tracker/st_atom_texture.c index 0ebc5b88bdac..9ab69f13d4d5 100644 --- a/src/mesa/state_tracker/st_atom_texture.c +++ b/src/mesa/state_tracker/st_atom_texture.c @@ -104,9 +104,8 @@ st_get_sampler_views(struct st_context *st, return 0; unsigned num_textures = util_last_bit(samplers_used); - - /* prog->sh.data is NULL if it's ARB_fragment_program */ - bool glsl130 = (prog->sh.data ? prog->sh.data->Version : 0) >= 130; + const bool glsl130 = + (prog->shader_program ? prog->shader_program->GLSL_Version : 0) >= 130; /* loop over sampler units (aka tex image units) */ for (unit = 0; unit < num_textures; unit++) { diff --git a/src/mesa/state_tracker/st_texture.c b/src/mesa/state_tracker/st_texture.c index e16bc7cca20f..c38b4423f16f 100644 --- a/src/mesa/state_tracker/st_texture.c +++ b/src/mesa/state_tracker/st_texture.c @@ -538,16 +538,16 @@ st_create_texture_handle_from_unit(struct st_context *st, struct pipe_context *pipe = st->pipe; struct pipe_sampler_view *view; struct pipe_sampler_state sampler = {0}; + const bool glsl130 = + (prog->shader_program ? prog->shader_program->GLSL_Version : 0) >= 130; /* TODO: Clarify the interaction of ARB_bindless_texture and EXT_texture_sRGB_decode */ - view = st_update_single_texture(st, texUnit, prog->sh.data->Version >= 130, - true, false); + view = st_update_single_texture(st, texUnit, glsl130, true, false); if (!view) return 0; if (view->target != PIPE_BUFFER) - st_convert_sampler_from_unit(st, &sampler, texUnit, - prog->sh.data && prog->sh.data->Version >= 130); + st_convert_sampler_from_unit(st, &sampler, texUnit, glsl130); assert(st->ctx->Texture.Unit[texUnit]._Current);