On 14/08/18 12:55, asimiklit.work wrote:
Hi Lionel,
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,
Thanks for your fast reply.
I agree that it is not correct patch for this issue but anyway
"iter_more_groups" function will still work incorrectly
for unknown instructions when the "iter->group->variable" field is true.
I guess that this case should be fixed.
Please let me know if I am incorrect.

Hey Andrii,

We shouldn't even get to use the iterator if it's an unknown instruction.
The decoder should just advance dword by dword until it finds something that makes sense again.

If we run into that problem, I think we should fix the caller.

Cheers,

-
Lionel


Regards,
Andrii.

On 2018-08-14 1:26 PM, Lionel Landwerlin wrote:
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.w...@gmail.com wrote:
From: Andrii Simiklit <asimiklit.w...@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.simik...@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;



_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Reply via email to