[Mesa-dev] [RFC PATCH] i965: book space at the end of p->store for SEND opcodes to avoid invalid memory access

[Samuel Iglesias Gonsalvez]

The caller to brw_next_insn() could be brw_send_indirect_message()
as a result of creating a SEND instruction after the OR
used to load the indirect descriptor to an address register.

In that case, the pointer to the OR instruction is
p->store[p->nr_insn - 1] and as it will be saved to specify additional
descriptor bits later throught brw_set_*_message() calls, we need to
have that pointer to valid memory.

If we realloc when processing the SEND instruction from the same
brw_send_indirect_message() call, the old p->store could be free'd and
the saved OR instruction pointer becomes invalid. Then undefined results
will happen when accessing to it to specify the additional descriptor
bits.

To avoid that, we only reallocate when we are close enough to the end
of p->store and not generating a SEND instruction or when we are really
running out of store size after several consecutive SEND at the end of
the store table. That should be enough to avoid this invalid memory
access problem.

Fixes ~120 dEQP-GLES31.functional.ssbo.* tests.

Signed-off-by: Samuel Iglesias Gonsalvez <siglesias at igalia.com>
---
 src/mesa/drivers/dri/i965/brw_eu_emit.c | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/src/mesa/drivers/dri/i965/brw_eu_emit.c b/src/mesa/drivers/dri/i965/brw_eu_emit.c
index bf2fee9..3b97cfa 100644
--- a/src/mesa/drivers/dri/i965/brw_eu_emit.c
+++ b/src/mesa/drivers/dri/i965/brw_eu_emit.c
@@ -810,7 +810,17 @@ brw_next_insn(struct brw_codegen *p, unsigned opcode)
    const struct brw_device_info *devinfo = p->devinfo;
    brw_inst *insn;
 
-   if (p->nr_insn + 1 > p->store_size) {
+
+   /* Book enough room for one or more consecutive SEND* instructions at the
+    * end of the p->store table in order to avoid reallocating p->store in
+    * the middle of brw_send_indirect_message().
+    *
+    * The 32 value was chosen arbitrary to make this problem less likely to
+    * happen and because it has low impact in p->store (its initial size is
+    * 1024).
+    */
+   if ((p->nr_insn + 32 > p->store_size && opcode != BRW_OPCODE_SEND) ||
+       (p->nr_insn + 1 > p->store_size)) {
       p->store_size <<= 1;
       p->store = reralloc(p->mem_ctx, p->store, brw_inst, p->store_size);
    }
-- 
2.1.4