mesa/st: fix possible crash related to arb invalid memory access

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 android-rpi#1 0x7f833c0b48 in update_textures ../src/mesa/state_tracker/st_atom_texture.c:266 android-rpi#2 0x7f82b2d120 in st_validate_state ../src/mesa/state_tracker/st_util.h:128 android-rpi#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 <[email protected]> Reviewed-by: Marek Olšák <[email protected]> Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/21175>
peyo-hd · Feb 10, 2023 · b2c340c · b2c340c
1 parent 620baf9
commit b2c340c
Show file tree

Hide file tree

Showing 11 changed files with 37 additions and 38 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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);