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

[Rafael Antognolli]

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
> 
> 
>