Mesa (staging/21.0): ir3: Prevent oob writes to inputs/outputs array

[GitLab Mirror]

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

Author: Connor Abbott <cwabbott0 at gmail.com>
Date:   Thu Apr 22 15:52:05 2021 +0200

ir3: Prevent oob writes to inputs/outputs array

Don't setup inputs and outputs if we aren't using
load_input/store_output intrinsics. While it's mostly harmless, there
may be more outputs than expected which would lead to an oob write of
the outputs array when setting the register id to INVALID_REG.

Also be more paranoid with asserts to catch this.

Fixes: a6291b1 ("freedreno/ir3: rework setup_{input,output} to make struct varyings work")
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/7274>
(cherry picked from commit decfea2f4e8128afeb6d4bb0d0c9c0740a01f766)

---

 .pick_status.json                    |  2 +-
 src/freedreno/ir3/ir3_compiler_nir.c | 50 ++++++++++++++++++++++++++++++------
 2 files changed, 43 insertions(+), 9 deletions(-)

diff --git a/.pick_status.json b/.pick_status.json
index 8f8f12beaf6..144c28002e7 100644
--- a/.pick_status.json
+++ b/.pick_status.json
@@ -616,7 +616,7 @@
         "description": "ir3: Prevent oob writes to inputs/outputs array",
         "nominated": true,
         "nomination_type": 1,
-        "resolution": 0,
+        "resolution": 1,
         "master_sha": null,
         "because_sha": "a6291b1b1177f5728e2e1998225f0b8676c6e710"
     },
diff --git a/src/freedreno/ir3/ir3_compiler_nir.c b/src/freedreno/ir3/ir3_compiler_nir.c
index 713fb20df8f..cbbbcf8e517 100644
--- a/src/freedreno/ir3/ir3_compiler_nir.c
+++ b/src/freedreno/ir3/ir3_compiler_nir.c
@@ -3001,6 +3001,7 @@ setup_input(struct ir3_context *ctx, nir_intrinsic_instr *intr)
 	so->inputs[n].slot = slot;
 	so->inputs[n].compmask |= compmask;
 	so->inputs_count = MAX2(so->inputs_count, n + 1);
+	compile_assert(ctx, so->inputs_count < ARRAY_SIZE(so->inputs));
 	so->inputs[n].flat = !coord;
 
 	if (ctx->so->type == MESA_SHADER_FRAGMENT) {
@@ -3293,6 +3294,31 @@ setup_output(struct ir3_context *ctx, nir_intrinsic_instr *intr)
 	}
 }
 
+static bool
+uses_load_input(struct ir3_shader_variant *so)
+{
+	return so->type == MESA_SHADER_VERTEX || so->type == MESA_SHADER_FRAGMENT;
+}
+
+static bool
+uses_store_output(struct ir3_shader_variant *so)
+{
+	switch (so->type) {
+		case MESA_SHADER_VERTEX:
+			return !so->key.has_gs && !so->key.tessellation;
+		case MESA_SHADER_TESS_EVAL:
+			return !so->key.has_gs;
+		case MESA_SHADER_GEOMETRY:
+		case MESA_SHADER_FRAGMENT:
+			return true;
+		case MESA_SHADER_TESS_CTRL:
+		case MESA_SHADER_COMPUTE:
+			return false;
+		default:
+			unreachable("unknown stage");
+	}
+}
+
 static void
 emit_instructions(struct ir3_context *ctx)
 {
@@ -3323,14 +3349,22 @@ emit_instructions(struct ir3_context *ctx)
 		}
 	}
 
-	/* TODO: for GS/HS/DS, load_input isn't used. but ctx->s->num_inputs is non-zero
-	 * likely the same for num_outputs in cases where store_output isn't used
-	 */
-	ctx->so->inputs_count = ctx->s->num_inputs;
-	ctx->ninputs = ctx->s->num_inputs * 4;
-	ctx->noutputs = ctx->s->num_outputs * 4;
-	ctx->inputs  = rzalloc_array(ctx, struct ir3_instruction *, ctx->ninputs);
-	ctx->outputs = rzalloc_array(ctx, struct ir3_instruction *, ctx->noutputs);
+	if (uses_load_input(ctx->so)) {
+		ctx->so->inputs_count = ctx->s->num_inputs;
+		compile_assert(ctx, ctx->so->inputs_count < ARRAY_SIZE(ctx->so->inputs));
+		ctx->ninputs = ctx->s->num_inputs * 4;
+		ctx->inputs  = rzalloc_array(ctx, struct ir3_instruction *, ctx->ninputs);
+	} else {
+		ctx->ninputs = 0;
+		ctx->so->inputs_count = 0;
+	}
+
+	if (uses_store_output(ctx->so)) {
+		ctx->noutputs = ctx->s->num_outputs * 4;
+		ctx->outputs = rzalloc_array(ctx, struct ir3_instruction *, ctx->noutputs);
+	} else {
+		ctx->noutputs = 0;
+	}
 
 	ctx->ir = ir3_create(ctx->compiler, ctx->so);