[Mesa-dev] [PATCH] nv50, nvc0: avoid reading out of bounds when getting bogus so info

[Ilia Mirkin]

On Wed, Oct 19, 2016 at 4:19 AM, Samuel Pitoiset
<samuel.pitoiset at gmail.com> wrote:
> radeonsi does the same check, seems like correct.
>
> How did you catch this? Does this fix a CTS test or something else?

It doesn't *fix* anything. In my debugging I began to suspect TF. So I
set a breakpoint, printed the pso, and noticed that the register index
was 255. This happens because the state tracker tries to stick the
GS's TF settings onto the VS. I also confirmed this with valgrind.

>
> Reviewed-by: Samuel Pitoiset <samuel.pitoiset at gmail.com>

Thanks!

>
>
> On 10/19/2016 06:08 AM, Ilia Mirkin wrote:
>>
>> The state tracker tries to attach the info to the wrong shader. This is
>> easy enough to protect against.
>>
>> Signed-off-by: Ilia Mirkin <imirkin at alum.mit.edu>
>> ---
>>  src/gallium/drivers/nouveau/nv50/nv50_program.c | 3 +++
>>  src/gallium/drivers/nouveau/nvc0/nvc0_program.c | 7 +++++--
>>  2 files changed, 8 insertions(+), 2 deletions(-)
>>
>> diff --git a/src/gallium/drivers/nouveau/nv50/nv50_program.c
>> b/src/gallium/drivers/nouveau/nv50/nv50_program.c
>> index 1e39427..9081cd8 100644
>> --- a/src/gallium/drivers/nouveau/nv50/nv50_program.c
>> +++ b/src/gallium/drivers/nouveau/nv50/nv50_program.c
>> @@ -308,6 +308,9 @@ nv50_program_create_strmout_state(const struct
>> nv50_ir_prog_info *info,
>>        const unsigned r = pso->output[i].register_index;
>>        b = pso->output[i].output_buffer;
>>
>> +      if (r >= info->numOutputs)
>> +         continue;
>> +
>>        for (c = 0; c < pso->output[i].num_components; ++c)
>>           so->map[base[b] + p + c] = info->out[r].slot[s + c];
>>     }
>> diff --git a/src/gallium/drivers/nouveau/nvc0/nvc0_program.c
>> b/src/gallium/drivers/nouveau/nvc0/nvc0_program.c
>> index 867d84a..50f8083 100644
>> --- a/src/gallium/drivers/nouveau/nvc0/nvc0_program.c
>> +++ b/src/gallium/drivers/nouveau/nvc0/nvc0_program.c
>> @@ -509,11 +509,14 @@ nvc0_program_create_tfb_state(const struct
>> nv50_ir_prog_info *info,
>>     for (i = 0; i < pso->num_outputs; ++i) {
>>        unsigned s = pso->output[i].start_component;
>>        unsigned p = pso->output[i].dst_offset;
>> +      const unsigned r = pso->output[i].register_index;
>>        b = pso->output[i].output_buffer;
>>
>> +      if (r >= info->numOutputs)
>> +         continue;
>> +
>>        for (c = 0; c < pso->output[i].num_components; ++c)
>> -         tfb->varying_index[b][p++] =
>> -            info->out[pso->output[i].register_index].slot[s + c];
>> +         tfb->varying_index[b][p++] = info->out[r].slot[s + c];
>>
>>        tfb->varying_count[b] = MAX2(tfb->varying_count[b], p);
>>        tfb->stream[b] = pso->output[i].stream;
>>
>
> --
> -Samuel