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

[Lionel Landwerlin]

Hi Andrii,

Again sorry, I don't think this is the right fix.
I'm sending another patch to fix the parsing of MI_BATCH_BUFFER_START 
which seems to be the actual issue.

Thanks for working on this,

-
Lionel

On 14/08/18 10:04, asimiklit.work at gmail.com wrote:
> From: Andrii Simiklit <asimiklit.work at gmail.com>
>
> The "gen_group_get_length" function can return a negative value
> and it can lead to the out of bounds group_iter.
>
> v2: printing of "unknown command type" was added
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107544
> Signed-off-by: Andrii Simiklit <andrii.simiklit at globallogic.com>
> ---
>   src/intel/common/gen_decoder.c | 13 +++++++++++--
>   1 file changed, 11 insertions(+), 2 deletions(-)
>
> diff --git a/src/intel/common/gen_decoder.c b/src/intel/common/gen_decoder.c
> index ec0a486..b36facf 100644
> --- a/src/intel/common/gen_decoder.c
> +++ b/src/intel/common/gen_decoder.c
> @@ -770,6 +770,13 @@ gen_group_get_length(struct gen_group *group, const uint32_t *p)
>               return -1;
>         }
>      }
> +   default: {
> +      fprintf(stderr, "Unknown command type %u in '%s::%s'\n",
> +            type,
> +            (group->parent && group->parent->name) ? group->parent->name : "UNKNOWN",
> +            group->name ? group->name : "UNKNOWN");
> +      break;
> +   }
>      }
>   
>      return -1;
> @@ -803,8 +810,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;