Skip to content

Commit b2c340c

Browse files
Patrick LerdaMarge Bot
Patrick Lerda
authored and
Marge Bot
committed
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 #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 <[email protected]> Reviewed-by: Marek Olšák <[email protected]> Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/21175>
1 parent 620baf9 commit b2c340c

11 files changed

+37
-38
lines changed

src/compiler/glsl/gl_nir_link_varyings.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2219,7 +2219,7 @@ remove_unused_io_vars(nir_shader *producer, nir_shader *consumer,
22192219
progress = true;
22202220

22212221
if (mode == nir_var_shader_in) {
2222-
if (!prog->IsES && prog->data->Version <= 120) {
2222+
if (!prog->IsES && prog->GLSL_Version <= 120) {
22232223
/* On page 25 (page 31 of the PDF) of the GLSL 1.20 spec:
22242224
*
22252225
* Only those varying variables used (i.e. read) in

src/compiler/glsl/gl_nir_linker.c

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -904,12 +904,11 @@ validate_sampler_array_indexing(const struct gl_constants *consts,
904904
"expressions is forbidden in GLSL %s %u";
905905
/* Backend has indicated that it has no dynamic indexing support. */
906906
if (no_dynamic_indexing) {
907-
linker_error(prog, msg, prog->IsES ? "ES" : "",
908-
prog->data->Version);
907+
linker_error(prog, msg, prog->IsES ? "ES" : "", prog->GLSL_Version);
909908
return false;
910909
} else {
911910
linker_warning(prog, msg, prog->IsES ? "ES" : "",
912-
prog->data->Version);
911+
prog->GLSL_Version);
913912
}
914913
}
915914
}
@@ -933,8 +932,8 @@ gl_nir_link_glsl(const struct gl_constants *consts,
933932
* with loop induction variable. This check emits a warning or error
934933
* depending if backend can handle dynamic indexing.
935934
*/
936-
if ((!prog->IsES && prog->data->Version < 130) ||
937-
(prog->IsES && prog->data->Version < 300)) {
935+
if ((!prog->IsES && prog->GLSL_Version < 130) ||
936+
(prog->IsES && prog->GLSL_Version < 300)) {
938937
if (!validate_sampler_array_indexing(consts, prog))
939938
return false;
940939
}

src/compiler/glsl/link_interface_blocks.cpp

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,7 @@ interstage_member_mismatch(struct gl_shader_program *prog,
6969
* interpolation qualifiers of variables of the same name do not
7070
* match."
7171
*/
72-
if (prog->IsES || prog->data->Version < 440)
72+
if (prog->IsES || prog->GLSL_Version < 440)
7373
if (c->fields.structure[i].interpolation !=
7474
p->fields.structure[i].interpolation)
7575
return true;
@@ -88,7 +88,7 @@ interstage_member_mismatch(struct gl_shader_program *prog,
8888
* The table in Section 9.2.1 Linked Shaders of the GLSL ES 3.2 spec
8989
* says that sample need not match for varyings.
9090
*/
91-
if (!prog->IsES || prog->data->Version < 310)
91+
if (!prog->IsES || prog->GLSL_Version < 310)
9292
if (c->fields.structure[i].centroid !=
9393
p->fields.structure[i].centroid)
9494
return true;
@@ -456,7 +456,7 @@ validate_interstage_inout_blocks(struct gl_shader_program *prog,
456456
continue;
457457

458458
/* Built-in interface redeclaration check. */
459-
if (prog->SeparateShader && !prog->IsES && prog->data->Version >= 150 &&
459+
if (prog->SeparateShader && !prog->IsES && prog->GLSL_Version >= 150 &&
460460
var->data.how_declared == ir_var_declared_implicitly &&
461461
var->data.used && !producer_iface) {
462462
linker_error(prog, "missing output builtin block %s redeclaration "
@@ -477,7 +477,7 @@ validate_interstage_inout_blocks(struct gl_shader_program *prog,
477477
ir_variable *producer_def = definitions.lookup(var);
478478

479479
/* Built-in interface redeclaration check. */
480-
if (prog->SeparateShader && !prog->IsES && prog->data->Version >= 150 &&
480+
if (prog->SeparateShader && !prog->IsES && prog->GLSL_Version >= 150 &&
481481
var->data.how_declared == ir_var_declared_implicitly &&
482482
var->data.used && !producer_iface) {
483483
linker_error(prog, "missing input builtin block %s redeclaration "

src/compiler/glsl/link_varyings.cpp

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -148,7 +148,7 @@ cross_validate_types_and_qualifiers(const struct gl_constants *consts,
148148
* OpenGLES 3.0 drivers, so we relax the checking in all cases.
149149
*/
150150
if (false /* always skip the centroid check */ &&
151-
prog->data->Version < (prog->IsES ? 310 : 430) &&
151+
prog->GLSL_Version < (prog->IsES ? 310 : 430) &&
152152
input->data.centroid != output->data.centroid) {
153153
linker_error(prog,
154154
"%s shader output `%s' %s centroid qualifier, "
@@ -203,7 +203,7 @@ cross_validate_types_and_qualifiers(const struct gl_constants *consts,
203203
* and fragment shaders must match."
204204
*/
205205
if (input->data.explicit_invariant != output->data.explicit_invariant &&
206-
prog->data->Version < (prog->IsES ? 300 : 420)) {
206+
prog->GLSL_Version < (prog->IsES ? 300 : 420)) {
207207
linker_error(prog,
208208
"%s shader output `%s' %s invariant qualifier, "
209209
"but %s shader input %s invariant qualifier\n",
@@ -240,7 +240,7 @@ cross_validate_types_and_qualifiers(const struct gl_constants *consts,
240240
output_interpolation = INTERP_MODE_SMOOTH;
241241
}
242242
if (input_interpolation != output_interpolation &&
243-
prog->data->Version < 440) {
243+
prog->GLSL_Version < 440) {
244244
if (!consts->AllowGLSLCrossStageInterpolationMismatch) {
245245
linker_error(prog,
246246
"%s shader output `%s' specifies %s "
@@ -642,7 +642,7 @@ validate_first_and_last_interface_explicit_locations(const struct gl_constants *
642642
static bool
643643
static_input_output_matching(struct gl_shader_program *prog)
644644
{
645-
return prog->data->Version >= (prog->IsES ? 0 : 420);
645+
return prog->GLSL_Version >= (prog->IsES ? 0 : 420);
646646
}
647647

648648
/**

src/compiler/glsl/linker.cpp

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -529,7 +529,7 @@ analyze_clip_cull_usage(struct gl_shader_program *prog,
529529
info->clip_distance_array_size = 0;
530530
info->cull_distance_array_size = 0;
531531

532-
if (prog->data->Version >= (prog->IsES ? 300 : 130)) {
532+
if (prog->GLSL_Version >= (prog->IsES ? 300 : 130)) {
533533
/* From section 7.1 (Vertex Shader Special Variables) of the
534534
* GLSL 1.30 spec:
535535
*
@@ -649,7 +649,7 @@ validate_vertex_shader_executable(struct gl_shader_program *prog,
649649
* All GLSL ES Versions are similar to GLSL 1.40--failing to write to
650650
* gl_Position is not an error.
651651
*/
652-
if (prog->data->Version < (prog->IsES ? 300 : 140)) {
652+
if (prog->GLSL_Version < (prog->IsES ? 300 : 140)) {
653653
find_variable gl_Position("gl_Position");
654654
find_assignments(shader->ir, &gl_Position);
655655
if (!gl_Position.found) {
@@ -1066,7 +1066,8 @@ cross_validate_globals(const struct gl_constants *consts,
10661066
if (!consts->AllowGLSLRelaxedES &&
10671067
prog->IsES && !var->get_interface_type() &&
10681068
existing->data.precision != var->data.precision) {
1069-
if ((existing->data.used && var->data.used) || prog->data->Version >= 300) {
1069+
if ((existing->data.used && var->data.used) ||
1070+
prog->GLSL_Version >= 300) {
10701071
linker_error(prog, "declarations for %s `%s` have "
10711072
"mismatching precision qualifiers\n",
10721073
mode_string(var), var->name);
@@ -1974,7 +1975,7 @@ link_fs_inout_layout_qualifiers(struct gl_shader_program *prog,
19741975
bool pixel_center_integer = false;
19751976

19761977
if (linked_shader->Stage != MESA_SHADER_FRAGMENT ||
1977-
(prog->data->Version < 150 &&
1978+
(prog->GLSL_Version < 150 &&
19781979
!prog->ARB_fragment_coord_conventions_enable))
19791980
return;
19801981

@@ -2052,8 +2053,7 @@ link_gs_inout_layout_qualifiers(struct gl_shader_program *prog,
20522053
/* No in/out qualifiers defined for anything but GLSL 1.50+
20532054
* geometry shaders so far.
20542055
*/
2055-
if (gl_prog->info.stage != MESA_SHADER_GEOMETRY ||
2056-
prog->data->Version < 150)
2056+
if (gl_prog->info.stage != MESA_SHADER_GEOMETRY || prog->GLSL_Version < 150)
20572057
return;
20582058

20592059
int vertices_out = -1;
@@ -2963,7 +2963,7 @@ assign_attribute_or_color_locations(void *mem_ctx,
29632963
}
29642964
}
29652965
} else if (target_index == MESA_SHADER_FRAGMENT ||
2966-
(prog->IsES && prog->data->Version >= 300)) {
2966+
(prog->IsES && prog->GLSL_Version >= 300)) {
29672967
linker_error(prog, "overlapping location is assigned "
29682968
"to %s `%s' %d %d %d\n", string, var->name,
29692969
used_locations, use_mask, attr);
@@ -3629,7 +3629,7 @@ link_shaders(struct gl_context *ctx, struct gl_shader_program *prog)
36293629
goto done;
36303630
}
36313631

3632-
prog->data->Version = max_version;
3632+
prog->GLSL_Version = max_version;
36333633
prog->IsES = prog->Shaders[0]->IsES;
36343634

36353635
/* 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)
38173817
lower_named_interface_blocks(mem_ctx, prog->_LinkedShaders[i]);
38183818
}
38193819

3820-
if (prog->IsES && prog->data->Version == 100)
3820+
if (prog->IsES && prog->GLSL_Version == 100)
38213821
if (!validate_invariant_builtins(prog,
38223822
prog->_LinkedShaders[MESA_SHADER_VERTEX],
38233823
prog->_LinkedShaders[MESA_SHADER_FRAGMENT]))

src/compiler/glsl/serialize.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1259,7 +1259,7 @@ serialize_glsl_program(struct blob *blob, struct gl_context *ctx,
12591259

12601260
write_hash_tables(blob, prog);
12611261

1262-
blob_write_uint32(blob, prog->data->Version);
1262+
blob_write_uint32(blob, prog->GLSL_Version);
12631263
blob_write_uint32(blob, prog->IsES);
12641264
blob_write_uint32(blob, prog->data->linked_stages);
12651265

@@ -1318,7 +1318,7 @@ deserialize_glsl_program(struct blob_reader *blob, struct gl_context *ctx,
13181318

13191319
read_hash_tables(blob, prog);
13201320

1321-
prog->data->Version = blob_read_uint32(blob);
1321+
prog->GLSL_Version = blob_read_uint32(blob);
13221322
prog->IsES = blob_read_uint32(blob);
13231323
prog->data->linked_stages = blob_read_uint32(blob);
13241324

src/mesa/main/shader_types.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -345,8 +345,6 @@ struct gl_shader_program_data
345345
enum gl_link_status LinkStatus; /**< GL_LINK_STATUS */
346346
GLchar *InfoLog;
347347

348-
unsigned Version; /**< GLSL version used for linking */
349-
350348
/* Mask of stages this program was linked against */
351349
unsigned linked_stages;
352350

@@ -489,6 +487,8 @@ struct gl_shader_program
489487
* #extension ARB_fragment_coord_conventions: enable
490488
*/
491489
GLboolean ARB_fragment_coord_conventions_enable;
490+
491+
unsigned GLSL_Version; /**< GLSL version used for linking */
492492
};
493493

494494
/**

src/mesa/main/shaderapi.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1389,8 +1389,8 @@ link_program(struct gl_context *ctx, struct gl_shader_program *shProg,
13891389
}
13901390
if (file) {
13911391
fprintf(file, "[require]\nGLSL%s >= %u.%02u\n",
1392-
shProg->IsES ? " ES" : "",
1393-
shProg->data->Version / 100, shProg->data->Version % 100);
1392+
shProg->IsES ? " ES" : "", shProg->GLSL_Version / 100,
1393+
shProg->GLSL_Version % 100);
13941394
if (shProg->SeparateShader)
13951395
fprintf(file, "GL_ARB_separate_shader_objects\nSSO ENABLED\n");
13961396
fprintf(file, "\n");

src/mesa/state_tracker/st_atom_sampler.c

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -225,8 +225,9 @@ update_shader_samplers(struct st_context *st,
225225
if (samplers_used & 1 &&
226226
(ctx->Texture.Unit[tex_unit]._Current->Target != GL_TEXTURE_BUFFER ||
227227
st->texture_buffer_sampler)) {
228-
st_convert_sampler_from_unit(st, sampler, tex_unit,
229-
prog->sh.data && prog->sh.data->Version >= 130);
228+
st_convert_sampler_from_unit(
229+
st, sampler, tex_unit,
230+
prog->shader_program && prog->shader_program->GLSL_Version >= 130);
230231
states[unit] = sampler;
231232
} else {
232233
states[unit] = NULL;

src/mesa/state_tracker/st_atom_texture.c

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -104,9 +104,8 @@ st_get_sampler_views(struct st_context *st,
104104
return 0;
105105

106106
unsigned num_textures = util_last_bit(samplers_used);
107-
108-
/* prog->sh.data is NULL if it's ARB_fragment_program */
109-
bool glsl130 = (prog->sh.data ? prog->sh.data->Version : 0) >= 130;
107+
const bool glsl130 =
108+
(prog->shader_program ? prog->shader_program->GLSL_Version : 0) >= 130;
110109

111110
/* loop over sampler units (aka tex image units) */
112111
for (unit = 0; unit < num_textures; unit++) {

src/mesa/state_tracker/st_texture.c

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -538,16 +538,16 @@ st_create_texture_handle_from_unit(struct st_context *st,
538538
struct pipe_context *pipe = st->pipe;
539539
struct pipe_sampler_view *view;
540540
struct pipe_sampler_state sampler = {0};
541+
const bool glsl130 =
542+
(prog->shader_program ? prog->shader_program->GLSL_Version : 0) >= 130;
541543

542544
/* TODO: Clarify the interaction of ARB_bindless_texture and EXT_texture_sRGB_decode */
543-
view = st_update_single_texture(st, texUnit, prog->sh.data->Version >= 130,
544-
true, false);
545+
view = st_update_single_texture(st, texUnit, glsl130, true, false);
545546
if (!view)
546547
return 0;
547548

548549
if (view->target != PIPE_BUFFER)
549-
st_convert_sampler_from_unit(st, &sampler, texUnit,
550-
prog->sh.data && prog->sh.data->Version >= 130);
550+
st_convert_sampler_from_unit(st, &sampler, texUnit, glsl130);
551551

552552
assert(st->ctx->Texture.Unit[texUnit]._Current);
553553

0 commit comments

Comments
 (0)