Mesa (staging/20.1): i965: Fix out-of-bounds access to brw_stage_state::surf_offset

[GitLab Mirror]

Module: Mesa
Branch: staging/20.1
Commit: b896c506b8ced880ed64687b7b2d4ec0ce3f841f
URL:    http://cgit.freedesktop.org/mesa/mesa/commit/?id=b896c506b8ced880ed64687b7b2d4ec0ce3f841f

Author: Danylo Piliaiev <danylo.piliaiev at globallogic.com>
Date:   Fri Mar 27 16:55:52 2020 +0200

i965: Fix out-of-bounds access to brw_stage_state::surf_offset

../src/mesa/drivers/dri/i965/brw_wm_surface_state.c:1378:32: runtime error: index 3503345872 out of bounds for type 'uint32_t [149]'

brw_assign_common_binding_table_offsets has the following comment:
 "Unused groups are initialized to 0xd0d0d0d0 to make it obvious that they're
 unused but also make sure that addition of small offsets to them will
 trigger some of our asserts that surface indices are < BRW_MAX_SURFACES."

Cc: <mesa-stable at lists.freedesktop.org>
Signed-off-by: Danylo Piliaiev <danylo.piliaiev at globallogic.com>
Reviewed-by: Caio Marcelo de Oliveira Filho <caio.oliveira at intel.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/4350>
(cherry picked from commit 784358bd6e6d59c521133c2a31fa9b88f8e18598)

---

 .pick_status.json                                |  2 +-
 src/mesa/drivers/dri/i965/brw_wm_surface_state.c | 52 +++++++++++++-----------
 2 files changed, 30 insertions(+), 24 deletions(-)

diff --git a/.pick_status.json b/.pick_status.json
index aa8a62590b2..11992791b29 100644
--- a/.pick_status.json
+++ b/.pick_status.json
@@ -22,7 +22,7 @@
         "description": "i965: Fix out-of-bounds access to brw_stage_state::surf_offset",
         "nominated": true,
         "nomination_type": 0,
-        "resolution": 0,
+        "resolution": 1,
         "master_sha": null,
         "because_sha": null
     },
diff --git a/src/mesa/drivers/dri/i965/brw_wm_surface_state.c b/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
index a200468ac2a..cd0128f273b 100644
--- a/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
+++ b/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
@@ -1364,33 +1364,39 @@ brw_upload_ubo_surfaces(struct brw_context *brw, struct gl_program *prog,
                  prog->info.num_abos == 0))
       return;
 
-   uint32_t *ubo_surf_offsets =
-      &stage_state->surf_offset[prog_data->binding_table.ubo_start];
-
-   for (int i = 0; i < prog->info.num_ubos; i++) {
-      struct gl_buffer_binding *binding =
-         &ctx->UniformBufferBindings[prog->sh.UniformBlocks[i]->Binding];
-      upload_buffer_surface(brw, binding, &ubo_surf_offsets[i],
-                            ISL_FORMAT_R32G32B32A32_FLOAT, 0);
+   if (prog->info.num_ubos) {
+      assert(prog_data->binding_table.ubo_start < BRW_MAX_SURFACES);
+      uint32_t *ubo_surf_offsets =
+         &stage_state->surf_offset[prog_data->binding_table.ubo_start];
+
+      for (int i = 0; i < prog->info.num_ubos; i++) {
+         struct gl_buffer_binding *binding =
+            &ctx->UniformBufferBindings[prog->sh.UniformBlocks[i]->Binding];
+         upload_buffer_surface(brw, binding, &ubo_surf_offsets[i],
+                               ISL_FORMAT_R32G32B32A32_FLOAT, 0);
+      }
    }
 
-   uint32_t *ssbo_surf_offsets =
-      &stage_state->surf_offset[prog_data->binding_table.ssbo_start];
-   uint32_t *abo_surf_offsets = ssbo_surf_offsets + prog->info.num_ssbos;
-
-   for (int i = 0; i < prog->info.num_abos; i++) {
-      struct gl_buffer_binding *binding =
-         &ctx->AtomicBufferBindings[prog->sh.AtomicBuffers[i]->Binding];
-      upload_buffer_surface(brw, binding, &abo_surf_offsets[i],
-                            ISL_FORMAT_RAW, RELOC_WRITE);
-   }
+   if (prog->info.num_ssbos || prog->info.num_abos) {
+      assert(prog_data->binding_table.ssbo_start < BRW_MAX_SURFACES);
+      uint32_t *ssbo_surf_offsets =
+         &stage_state->surf_offset[prog_data->binding_table.ssbo_start];
+      uint32_t *abo_surf_offsets = ssbo_surf_offsets + prog->info.num_ssbos;
+
+      for (int i = 0; i < prog->info.num_abos; i++) {
+         struct gl_buffer_binding *binding =
+            &ctx->AtomicBufferBindings[prog->sh.AtomicBuffers[i]->Binding];
+         upload_buffer_surface(brw, binding, &abo_surf_offsets[i],
+                               ISL_FORMAT_RAW, RELOC_WRITE);
+      }
 
-   for (int i = 0; i < prog->info.num_ssbos; i++) {
-      struct gl_buffer_binding *binding =
-         &ctx->ShaderStorageBufferBindings[prog->sh.ShaderStorageBlocks[i]->Binding];
+      for (int i = 0; i < prog->info.num_ssbos; i++) {
+         struct gl_buffer_binding *binding =
+            &ctx->ShaderStorageBufferBindings[prog->sh.ShaderStorageBlocks[i]->Binding];
 
-      upload_buffer_surface(brw, binding, &ssbo_surf_offsets[i],
-                            ISL_FORMAT_RAW, RELOC_WRITE);
+         upload_buffer_surface(brw, binding, &ssbo_surf_offsets[i],
+                               ISL_FORMAT_RAW, RELOC_WRITE);
+      }
    }
 
    stage_state->push_constants_dirty = true;