[Mesa-dev] [PATCH] r600g: Fix out of bounds access
Marek Olšák
maraeo at gmail.com
Mon Mar 20 16:33:24 UTC 2017
Pushed, thanks.
Marek
On Wed, Feb 8, 2017 at 5:16 PM, Bartosz Tomczyk
<bartosz.tomczyk86 at gmail.com> wrote:
> fc_sp variable should indicate number of elements in
> fc_stack array, but fc_sp was increased at beginning of fc_pushlevel
> function. It leads to situation where idx=0 was never used, and last
> 32 element was stored outside fs_stack array.
> ---
> src/gallium/drivers/r600/r600_asm.h | 3 ++-
> src/gallium/drivers/r600/r600_shader.c | 39 +++++++++++++++++-----------------
> 2 files changed, 22 insertions(+), 20 deletions(-)
>
> diff --git a/src/gallium/drivers/r600/r600_asm.h b/src/gallium/drivers/r600/r600_asm.h
> index 1629399d8f..b12913d437 100644
> --- a/src/gallium/drivers/r600/r600_asm.h
> +++ b/src/gallium/drivers/r600/r600_asm.h
> @@ -25,6 +25,7 @@
>
> #include "r600_pipe.h"
> #include "r600_isa.h"
> +#include "tgsi/tgsi_exec.h"
>
> struct r600_bytecode_alu_src {
> unsigned sel;
> @@ -238,7 +239,7 @@ struct r600_bytecode {
> unsigned force_add_cf;
> uint32_t *bytecode;
> uint32_t fc_sp;
> - struct r600_cf_stack_entry fc_stack[32];
> + struct r600_cf_stack_entry fc_stack[TGSI_EXEC_MAX_NESTING];
> struct r600_stack_info stack;
> unsigned ar_loaded;
> unsigned ar_reg;
> diff --git a/src/gallium/drivers/r600/r600_shader.c b/src/gallium/drivers/r600/r600_shader.c
> index b80a3f8b62..34cc002c38 100644
> --- a/src/gallium/drivers/r600/r600_shader.c
> +++ b/src/gallium/drivers/r600/r600_shader.c
> @@ -8646,14 +8646,15 @@ static void fc_set_mid(struct r600_shader_ctx *ctx, int fc_sp)
>
> static void fc_pushlevel(struct r600_shader_ctx *ctx, int type)
> {
> - ctx->bc->fc_sp++;
> + assert(ctx->bc->fc_sp < ARRAY_SIZE(ctx->bc->fc_stack));
> ctx->bc->fc_stack[ctx->bc->fc_sp].type = type;
> ctx->bc->fc_stack[ctx->bc->fc_sp].start = ctx->bc->cf_last;
> + ctx->bc->fc_sp++;
> }
>
> static void fc_poplevel(struct r600_shader_ctx *ctx)
> {
> - struct r600_cf_stack_entry *sp = &ctx->bc->fc_stack[ctx->bc->fc_sp];
> + struct r600_cf_stack_entry *sp = &ctx->bc->fc_stack[ctx->bc->fc_sp - 1];
> free(sp->mid);
> sp->mid = NULL;
> sp->num_mid = 0;
> @@ -8749,24 +8750,24 @@ static int tgsi_else(struct r600_shader_ctx *ctx)
> r600_bytecode_add_cfinst(ctx->bc, CF_OP_ELSE);
> ctx->bc->cf_last->pop_count = 1;
>
> - fc_set_mid(ctx, ctx->bc->fc_sp);
> - ctx->bc->fc_stack[ctx->bc->fc_sp].start->cf_addr = ctx->bc->cf_last->id;
> + fc_set_mid(ctx, ctx->bc->fc_sp - 1);
> + ctx->bc->fc_stack[ctx->bc->fc_sp - 1].start->cf_addr = ctx->bc->cf_last->id;
> return 0;
> }
>
> static int tgsi_endif(struct r600_shader_ctx *ctx)
> {
> pops(ctx, 1);
> - if (ctx->bc->fc_stack[ctx->bc->fc_sp].type != FC_IF) {
> + if (ctx->bc->fc_stack[ctx->bc->fc_sp - 1].type != FC_IF) {
> R600_ERR("if/endif unbalanced in shader\n");
> return -1;
> }
>
> - if (ctx->bc->fc_stack[ctx->bc->fc_sp].mid == NULL) {
> - ctx->bc->fc_stack[ctx->bc->fc_sp].start->cf_addr = ctx->bc->cf_last->id + 2;
> - ctx->bc->fc_stack[ctx->bc->fc_sp].start->pop_count = 1;
> + if (ctx->bc->fc_stack[ctx->bc->fc_sp - 1].mid == NULL) {
> + ctx->bc->fc_stack[ctx->bc->fc_sp - 1].start->cf_addr = ctx->bc->cf_last->id + 2;
> + ctx->bc->fc_stack[ctx->bc->fc_sp - 1].start->pop_count = 1;
> } else {
> - ctx->bc->fc_stack[ctx->bc->fc_sp].mid[0]->cf_addr = ctx->bc->cf_last->id + 2;
> + ctx->bc->fc_stack[ctx->bc->fc_sp - 1].mid[0]->cf_addr = ctx->bc->cf_last->id + 2;
> }
> fc_poplevel(ctx);
>
> @@ -8793,7 +8794,7 @@ static int tgsi_endloop(struct r600_shader_ctx *ctx)
>
> r600_bytecode_add_cfinst(ctx->bc, CF_OP_LOOP_END);
>
> - if (ctx->bc->fc_stack[ctx->bc->fc_sp].type != FC_LOOP) {
> + if (ctx->bc->fc_stack[ctx->bc->fc_sp - 1].type != FC_LOOP) {
> R600_ERR("loop/endloop in shader code are not paired.\n");
> return -EINVAL;
> }
> @@ -8803,12 +8804,12 @@ static int tgsi_endloop(struct r600_shader_ctx *ctx)
> LOOP START point to CF after LOOP END
> BRK/CONT point to LOOP END CF
> */
> - ctx->bc->cf_last->cf_addr = ctx->bc->fc_stack[ctx->bc->fc_sp].start->id + 2;
> + ctx->bc->cf_last->cf_addr = ctx->bc->fc_stack[ctx->bc->fc_sp - 1].start->id + 2;
>
> - ctx->bc->fc_stack[ctx->bc->fc_sp].start->cf_addr = ctx->bc->cf_last->id + 2;
> + ctx->bc->fc_stack[ctx->bc->fc_sp - 1].start->cf_addr = ctx->bc->cf_last->id + 2;
>
> - for (i = 0; i < ctx->bc->fc_stack[ctx->bc->fc_sp].num_mid; i++) {
> - ctx->bc->fc_stack[ctx->bc->fc_sp].mid[i]->cf_addr = ctx->bc->cf_last->id;
> + for (i = 0; i < ctx->bc->fc_stack[ctx->bc->fc_sp - 1].num_mid; i++) {
> + ctx->bc->fc_stack[ctx->bc->fc_sp - 1].mid[i]->cf_addr = ctx->bc->cf_last->id;
> }
> /* XXX add LOOPRET support */
> fc_poplevel(ctx);
> @@ -8823,7 +8824,7 @@ static int tgsi_loop_breakc(struct r600_shader_ctx *ctx)
>
> for (fscp = ctx->bc->fc_sp; fscp > 0; fscp--)
> {
> - if (FC_LOOP == ctx->bc->fc_stack[fscp].type)
> + if (FC_LOOP == ctx->bc->fc_stack[fscp - 1].type)
> break;
> }
> if (fscp == 0) {
> @@ -8842,14 +8843,14 @@ static int tgsi_loop_breakc(struct r600_shader_ctx *ctx)
> r = r600_bytecode_add_cfinst(ctx->bc, CF_OP_LOOP_BREAK);
> if (r)
> return r;
> - fc_set_mid(ctx, fscp);
> + fc_set_mid(ctx, fscp - 1);
>
> return tgsi_endif(ctx);
> } else {
> r = emit_logic_pred(ctx, ALU_OP2_PRED_SETE_INT, CF_OP_ALU_BREAK);
> if (r)
> return r;
> - fc_set_mid(ctx, fscp);
> + fc_set_mid(ctx, fscp - 1);
> }
>
> return 0;
> @@ -8861,7 +8862,7 @@ static int tgsi_loop_brk_cont(struct r600_shader_ctx *ctx)
>
> for (fscp = ctx->bc->fc_sp; fscp > 0; fscp--)
> {
> - if (FC_LOOP == ctx->bc->fc_stack[fscp].type)
> + if (FC_LOOP == ctx->bc->fc_stack[fscp - 1].type)
> break;
> }
>
> @@ -8872,7 +8873,7 @@ static int tgsi_loop_brk_cont(struct r600_shader_ctx *ctx)
>
> r600_bytecode_add_cfinst(ctx->bc, ctx->inst_info->op);
>
> - fc_set_mid(ctx, fscp);
> + fc_set_mid(ctx, fscp - 1);
>
> return 0;
> }
> --
> 2.11.1
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
More information about the mesa-dev
mailing list