[Mesa-dev] [PATCH] intel/decoder: fix the possible out of bounds group_iter

[Lionel Landwerlin]

Andrey also opened a bug about this issue : 
https://bugs.freedesktop.org/show_bug.cgi?id=107544

It feels like it should be fixed on master though. get_length() 
shouldn't return -1 for structs anymore.
We should probably return 1 at end of get_length() so that the decoder 
prints out "unknown instruction".
That would help spot potential errors and updates needed to genxml.

-
Lionel


On 10/08/18 16:48, Rafael Antognolli wrote:
> On Thu, Aug 09, 2018 at 03:00:30PM +0300, andrey simiklit wrote:
>> Hi,
>>
>> Sorry I missed the main thought here.
>> The "gen_group_get_length" function returns int
>> but the "iter_group_offset_bits" function returns uint32_t
>> So uint32_t(int(-32)) = 0xFFFFFFE0U and it looks like unexpected behavior for
>> me:
>> iter_group_offset_bits(iter, iter->group_iter + 1) < 0xFFFFFFE0U;
> That's fine, I think the original commit message is good enough to
> understand this change. Feel free to add this extra bit too if you want,
> but I don't think it's needed.
>
> Reviewed-by: Rafael Antognolli <rafael.antognolli at intel.com>
>
>> Regards,
>> Andrii.
>>
>> On Thu, Aug 9, 2018 at 2:35 PM, Andrii Simiklit <asimiklit.work at gmail.com>
>> wrote:
>>
>>      The "gen_group_get_length" function can return a negative value
>>      and it can lead to the out of bounds group_iter.
>>
>>      Signed-off-by: Andrii Simiklit <andrii.simiklit at globallogic.com>
>>      ---
>>       src/intel/common/gen_decoder.c | 6 ++++--
>>       1 file changed, 4 insertions(+), 2 deletions(-)
>>
>>      diff --git a/src/intel/common/gen_decoder.c b/src/intel/common/gen_decoder
>>      .c
>>      index ec0a486..f09bd87 100644
>>      --- a/src/intel/common/gen_decoder.c
>>      +++ b/src/intel/common/gen_decoder.c
>>      @@ -803,8 +803,10 @@ static bool
>>       iter_more_groups(const struct gen_field_iterator *iter)
>>       {
>>          if (iter->group->variable) {
>>      -      return iter_group_offset_bits(iter, iter->group_iter + 1) <
>>      -              (gen_group_get_length(iter->group, iter->p) * 32);
>>      +      const int length = gen_group_get_length(iter->group, iter->p);
>>      +      return length > 0 &&
>>      +             iter_group_offset_bits(iter, iter->group_iter + 1) <
>>      +              (length * 32);
>>          } else {
>>             return (iter->group_iter + 1) < iter->group->group_count ||
>>                iter->group->next != NULL;
>>      --
>>      2.7.4
>>
>>
>>