|Summary:||intel/decoder: out of bounds group_iter|
|Component:||Drivers/DRI/i965||Assignee:||Intel 3D Bugs Mailing List <intel-3d-bugs>|
|Status:||RESOLVED FIXED||QA Contact:||Intel 3D Bugs Mailing List <intel-3d-bugs>|
|i915 platform:||i915 features:|
Description asimiklit 2018-08-10 11:37:50 UTC
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 This behavior lead my program to crash because 'group_iter' go out of bounds when it prints BLEND_STATE on HSW. I suggested the following solution for it: https://patchwork.freedesktop.org/patch/243647/
Comment 1 Lionel Landwerlin 2018-08-10 11:55:45 UTC
This should have been fixed in this commit : commit 4f128f7850e64829b8b1399ef0bdfd1eec7f4504 Author: Lionel Landwerlin <email@example.com> Date: Tue May 1 22:14:12 2018 +0100 intel: decoder: identify groups with fixed length
Comment 2 asimiklit 2018-08-11 09:25:31 UTC
Created attachment 141041 [details] SIGBUS (In reply to Lionel Landwerlin from comment #1) > This should have been fixed in this commit : > > commit 4f128f7850e64829b8b1399ef0bdfd1eec7f4504 > Author: Lionel Landwerlin <firstname.lastname@example.org> > Date: Tue May 1 22:14:12 2018 +0100 > > intel: decoder: identify groups with fixed length I have re-checked it on the latest mesa yesterday but unfortunately it is still reproducible. I am going to suggest the updated patch in the mailing thread.
Comment 3 Lionel Landwerlin 2018-08-11 17:32:14 UTC
Could you attach the file that is causing the crash? Thanks!
Comment 4 asimiklit 2018-08-13 15:54:53 UTC
Created attachment 141069 [details] simple reproducer (In reply to Lionel Landwerlin from comment #3) > Could you attach the file that is causing the crash? > Thanks! Hi, The simple reproducer is attached. I think that my patch can help avoid some issues with new command types in "batchbuffer" in the future. But it is not enough to fix this issue. As far as I understood currently the decoder tries to determine the length of the structure BLEND_STATE based on command type. But BLEND_STATE is placed in "statebuffer" and does not have any headers just a data. We create the following structure in gen_decoder.c:387 struct gen_group *group = create_group(ctx, "", atts, ctx->group, false); previous_group->next = group;//previous_group->name is "BLEND_STATE" with the following settings: group->fixed_length is false and group->variable is true That is why we tried to determine the length based on command type. Is it expected behavior? Regards, Andrii.
Comment 5 Lionel Landwerlin 2018-08-14 00:24:23 UTC
Thanks, unfortunately there seemed to be more than one issue. I've sent https://patchwork.freedesktop.org/series/48139/ which fixes the problem on my machine. Let me know if it does for you too.
Comment 6 Lionel Landwerlin 2018-08-27 17:35:06 UTC
Should be fixed on master with : commit 440a988bd1478bb33dafcbb8575473bc643ae383 Author: Lionel Landwerlin <email@example.com> Date: Sat Aug 25 18:22:00 2018 +0100 intel: decoder: handle 0 sized structs and : commit f430a37fa75f534c3a114b0ec546fa14f05f5da1 Author: Lionel Landwerlin <firstname.lastname@example.org> Date: Tue Aug 14 11:22:12 2018 +0100 intel: decoder: unify MI_BB_START field naming
Comment 7 Mark Janes 2018-08-27 23:53:38 UTC
Lionel, does this merit a new piglit test?
Comment 8 Lionel Landwerlin 2018-08-28 10:01:41 UTC
(In reply to Mark Janes from comment #7) > Lionel, does this merit a new piglit test? This kind of a debug feature. I don't really expect users to run with this enabled. That being said, I would like to add some tests within mesa so we can spot when we break the debug tools.