[Mesa-dev] [PATCH] glsl: fix glsl_struct_field size calculations for shader cache

[Timothy Arceri]

Whoops.

Reviewed-by: Timothy Arceri <tarceri at itsqueeze.com>

Thanks!

On 24/08/17 23:42, Nicolai Hähnle wrote:
> From: Nicolai Hähnle <nicolai.haehnle at amd.com>
> 
> Found by address sanitizer:
> 
> ==22621==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x61400000cbd8 at pc 0x7f561610a4ff bp 0x7ffca85f9d50 sp 0x7ffca85f94f8
> READ of size 344 at 0x61400000cbd8 thread T0
>      #0 0x7f561610a4fe  (/usr/lib/x86_64-linux-gnu/libasan.so.3+0x5f4fe)
>      #1 0x7f560bb305a5 in memcpy /usr/include/x86_64-linux-gnu/bits/string3.h:53
>      #2 0x7f560bb305a5 in blob_write_bytes ../../../mesa-src/src/compiler/glsl/blob.c:136
>      #3 0x7f560be7d7ff in encode_type_to_blob ../../../mesa-src/src/compiler/glsl/shader_cache.cpp:153
>      #4 0x7f560be81222 in write_program_resource_data ../../../mesa-src/src/compiler/glsl/shader_cache.cpp:950
>      #5 0x7f560be81222 in write_program_resource_list ../../../mesa-src/src/compiler/glsl/shader_cache.cpp:1118
>      #6 0x7f560be81222 in shader_cache_write_program_metadata(gl_context*, gl_shader_program*) ../../../mesa-src/src/compiler/glsl/shader_cache.cpp:1407
>      #7 0x7f560b825fdb in link_program ../../../mesa-src/src/mesa/main/shaderapi.c:1163
> 
> Fixes: 073a84ff60db ("glsl: stop adding pointers from glsl_struct_field to the cache")
> ---
>   src/compiler/glsl/shader_cache.cpp | 11 ++++-------
>   1 file changed, 4 insertions(+), 7 deletions(-)
> 
> diff --git a/src/compiler/glsl/shader_cache.cpp b/src/compiler/glsl/shader_cache.cpp
> index aa6c067d041..8eb7a5cb792 100644
> --- a/src/compiler/glsl/shader_cache.cpp
> +++ b/src/compiler/glsl/shader_cache.cpp
> @@ -69,24 +69,23 @@ extern "C" {
>   
>   static void
>   compile_shaders(struct gl_context *ctx, struct gl_shader_program *prog) {
>      for (unsigned i = 0; i < prog->NumShaders; i++) {
>         _mesa_glsl_compile_shader(ctx, prog->Shaders[i], false, false, true);
>      }
>   }
>   
>   static void
>   get_struct_type_field_and_pointer_sizes(size_t *s_field_size,
> -                                        size_t *s_field_ptrs,
> -                                        unsigned num_fields)
> +                                        size_t *s_field_ptrs)
>   {
> -   *s_field_size = sizeof(glsl_struct_field) * num_fields;
> +   *s_field_size = sizeof(glsl_struct_field);
>      *s_field_ptrs =
>        sizeof(((glsl_struct_field *)0)->type) +
>        sizeof(((glsl_struct_field *)0)->name);
>   }
>   
>   static void
>   encode_type_to_blob(struct blob *blob, const glsl_type *type)
>   {
>      uint32_t encoding;
>   
> @@ -133,22 +132,21 @@ encode_type_to_blob(struct blob *blob, const glsl_type *type)
>         blob_write_uint32(blob, type->length);
>         encode_type_to_blob(blob, type->fields.array);
>         return;
>      case GLSL_TYPE_STRUCT:
>      case GLSL_TYPE_INTERFACE:
>         blob_write_uint32(blob, (type->base_type) << 24);
>         blob_write_string(blob, type->name);
>         blob_write_uint32(blob, type->length);
>   
>         size_t s_field_size, s_field_ptrs;
> -      get_struct_type_field_and_pointer_sizes(&s_field_size, &s_field_ptrs,
> -                                              type->length);
> +      get_struct_type_field_and_pointer_sizes(&s_field_size, &s_field_ptrs);
>   
>         for (unsigned i = 0; i < type->length; i++) {
>            encode_type_to_blob(blob, type->fields.structure[i].type);
>            blob_write_string(blob, type->fields.structure[i].name);
>   
>            /* Write the struct field skipping the pointers */
>            blob_write_bytes(blob,
>                             ((char *)&type->fields.structure[i]) + s_field_ptrs,
>                             s_field_size - s_field_ptrs);
>         }
> @@ -206,22 +204,21 @@ decode_type_from_blob(struct blob_reader *blob)
>         unsigned length = blob_read_uint32(blob);
>         return glsl_type::get_array_instance(decode_type_from_blob(blob),
>                                              length);
>      }
>      case GLSL_TYPE_STRUCT:
>      case GLSL_TYPE_INTERFACE: {
>         char *name = blob_read_string(blob);
>         unsigned num_fields = blob_read_uint32(blob);
>   
>         size_t s_field_size, s_field_ptrs;
> -      get_struct_type_field_and_pointer_sizes(&s_field_size, &s_field_ptrs,
> -                                              num_fields);
> +      get_struct_type_field_and_pointer_sizes(&s_field_size, &s_field_ptrs);
>   
>         glsl_struct_field *fields =
>            (glsl_struct_field *) malloc(s_field_size * num_fields);
>         for (unsigned i = 0; i < num_fields; i++) {
>            fields[i].type = decode_type_from_blob(blob);
>            fields[i].name = blob_read_string(blob);
>   
>            blob_copy_bytes(blob, ((uint8_t *) &fields[i]) + s_field_ptrs,
>                            s_field_size - s_field_ptrs);
>         }
>