* [PATCH i-g-t] tests/gem_storedw_loop: Fix use after free for bufmgr
@ 2015-09-22 18:02 robert.beckett
2015-09-22 19:08 ` Jesse Barnes
0 siblings, 1 reply; 6+ messages in thread
From: robert.beckett @ 2015-09-22 18:02 UTC (permalink / raw)
To: intel-gfx
From: Robert Beckett <robert.beckett@intel.com>
Move bufmgr destruction to end of tests. This avoids use after free on
successive tests.
This could be squashed with the following patch to aid bisectability:
commit 0679702150157706a6def66b893b29c16345f4db
Author: Jesse Barnes <jbarnes@virtuousgeek.org>
Date: Wed Aug 5 16:06:31 2015 -0700
tests/gem_storedw_loop: add new store_dword test to unify per-ring ones v2
Signed-off-by: Robert Beckett <robert.beckett@intel.com>
Cc: Jesse Barnes <jbarnes@virtuousgeek.org>
---
tests/gem_storedw_loop.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tests/gem_storedw_loop.c b/tests/gem_storedw_loop.c
index 10b12d4..e7ebcc2 100644
--- a/tests/gem_storedw_loop.c
+++ b/tests/gem_storedw_loop.c
@@ -128,7 +128,6 @@ store_test(int ring, int count)
drm_intel_bo_unreference(target_buffer);
intel_batchbuffer_free(batch);
- drm_intel_bufmgr_destroy(bufmgr);
}
struct ring {
@@ -181,5 +180,6 @@ igt_main
}
}
+ drm_intel_bufmgr_destroy(bufmgr);
close(fd);
}
--
1.7.9.5
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH i-g-t] tests/gem_storedw_loop: Fix use after free for bufmgr
2015-09-22 18:02 [PATCH i-g-t] tests/gem_storedw_loop: Fix use after free for bufmgr robert.beckett
@ 2015-09-22 19:08 ` Jesse Barnes
2015-09-23 16:01 ` Daniel Vetter
0 siblings, 1 reply; 6+ messages in thread
From: Jesse Barnes @ 2015-09-22 19:08 UTC (permalink / raw)
To: robert.beckett, intel-gfx
On 09/22/2015 11:02 AM, robert.beckett@intel.com wrote:
> From: Robert Beckett <robert.beckett@intel.com>
>
> Move bufmgr destruction to end of tests. This avoids use after free on
> successive tests.
>
> This could be squashed with the following patch to aid bisectability:
>
> commit 0679702150157706a6def66b893b29c16345f4db
> Author: Jesse Barnes <jbarnes@virtuousgeek.org>
> Date: Wed Aug 5 16:06:31 2015 -0700
>
> tests/gem_storedw_loop: add new store_dword test to unify per-ring ones v2
>
> Signed-off-by: Robert Beckett <robert.beckett@intel.com>
> Cc: Jesse Barnes <jbarnes@virtuousgeek.org>
> ---
> tests/gem_storedw_loop.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/tests/gem_storedw_loop.c b/tests/gem_storedw_loop.c
> index 10b12d4..e7ebcc2 100644
> --- a/tests/gem_storedw_loop.c
> +++ b/tests/gem_storedw_loop.c
> @@ -128,7 +128,6 @@ store_test(int ring, int count)
>
> drm_intel_bo_unreference(target_buffer);
> intel_batchbuffer_free(batch);
> - drm_intel_bufmgr_destroy(bufmgr);
> }
>
> struct ring {
> @@ -181,5 +180,6 @@ igt_main
> }
> }
>
> + drm_intel_bufmgr_destroy(bufmgr);
> close(fd);
> }
>
Looks good, thanks. Unfortunately too late to squash...
Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>
Jesse
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH i-g-t] tests/gem_storedw_loop: Fix use after free for bufmgr
2015-09-22 19:08 ` Jesse Barnes
@ 2015-09-23 16:01 ` Daniel Vetter
2015-09-23 16:27 ` Thomas Wood
0 siblings, 1 reply; 6+ messages in thread
From: Daniel Vetter @ 2015-09-23 16:01 UTC (permalink / raw)
To: Jesse Barnes; +Cc: intel-gfx
On Tue, Sep 22, 2015 at 12:08:05PM -0700, Jesse Barnes wrote:
> On 09/22/2015 11:02 AM, robert.beckett@intel.com wrote:
> > From: Robert Beckett <robert.beckett@intel.com>
> >
> > Move bufmgr destruction to end of tests. This avoids use after free on
> > successive tests.
> >
> > This could be squashed with the following patch to aid bisectability:
> >
> > commit 0679702150157706a6def66b893b29c16345f4db
> > Author: Jesse Barnes <jbarnes@virtuousgeek.org>
> > Date: Wed Aug 5 16:06:31 2015 -0700
> >
> > tests/gem_storedw_loop: add new store_dword test to unify per-ring ones v2
> >
> > Signed-off-by: Robert Beckett <robert.beckett@intel.com>
> > Cc: Jesse Barnes <jbarnes@virtuousgeek.org>
> > ---
> > tests/gem_storedw_loop.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/tests/gem_storedw_loop.c b/tests/gem_storedw_loop.c
> > index 10b12d4..e7ebcc2 100644
> > --- a/tests/gem_storedw_loop.c
> > +++ b/tests/gem_storedw_loop.c
> > @@ -128,7 +128,6 @@ store_test(int ring, int count)
> >
> > drm_intel_bo_unreference(target_buffer);
> > intel_batchbuffer_free(batch);
> > - drm_intel_bufmgr_destroy(bufmgr);
> > }
> >
> > struct ring {
> > @@ -181,5 +180,6 @@ igt_main
> > }
> > }
> >
> > + drm_intel_bufmgr_destroy(bufmgr);
> > close(fd);
> > }
> >
>
> Looks good, thanks. Unfortunately too late to squash...
>
> Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>
No one from vpg has igt commit access so you need to push patches for
them. I've done that now.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH i-g-t] tests/gem_storedw_loop: Fix use after free for bufmgr
2015-09-23 16:01 ` Daniel Vetter
@ 2015-09-23 16:27 ` Thomas Wood
2015-09-24 10:13 ` [PATCH i-g-t] gem_storedw_loop: fix segfault when listing subtests Thomas Wood
0 siblings, 1 reply; 6+ messages in thread
From: Thomas Wood @ 2015-09-23 16:27 UTC (permalink / raw)
To: Daniel Vetter; +Cc: Intel Graphics Development
On 23 September 2015 at 17:01, Daniel Vetter <daniel@ffwll.ch> wrote:
> On Tue, Sep 22, 2015 at 12:08:05PM -0700, Jesse Barnes wrote:
>> On 09/22/2015 11:02 AM, robert.beckett@intel.com wrote:
>> > From: Robert Beckett <robert.beckett@intel.com>
>> >
>> > Move bufmgr destruction to end of tests. This avoids use after free on
>> > successive tests.
>> >
>> > This could be squashed with the following patch to aid bisectability:
>> >
>> > commit 0679702150157706a6def66b893b29c16345f4db
>> > Author: Jesse Barnes <jbarnes@virtuousgeek.org>
>> > Date: Wed Aug 5 16:06:31 2015 -0700
>> >
>> > tests/gem_storedw_loop: add new store_dword test to unify per-ring ones v2
>> >
>> > Signed-off-by: Robert Beckett <robert.beckett@intel.com>
>> > Cc: Jesse Barnes <jbarnes@virtuousgeek.org>
>> > ---
>> > tests/gem_storedw_loop.c | 2 +-
>> > 1 file changed, 1 insertion(+), 1 deletion(-)
>> >
>> > diff --git a/tests/gem_storedw_loop.c b/tests/gem_storedw_loop.c
>> > index 10b12d4..e7ebcc2 100644
>> > --- a/tests/gem_storedw_loop.c
>> > +++ b/tests/gem_storedw_loop.c
>> > @@ -128,7 +128,6 @@ store_test(int ring, int count)
>> >
>> > drm_intel_bo_unreference(target_buffer);
>> > intel_batchbuffer_free(batch);
>> > - drm_intel_bufmgr_destroy(bufmgr);
>> > }
>> >
>> > struct ring {
>> > @@ -181,5 +180,6 @@ igt_main
>> > }
>> > }
>> >
>> > + drm_intel_bufmgr_destroy(bufmgr);
>> > close(fd);
>> > }
>> >
>>
>> Looks good, thanks. Unfortunately too late to squash...
>>
>> Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>
>
> No one from vpg has igt commit access so you need to push patches for
> them. I've done that now.
Unfortunately this patch causes --list-subtest to fail, since the call
to drm_intel_bufmgr_destroy is called with NULL which causes a
segmentation fault. The calls to drm_intel_bufmgr_destroy and close
should be in an igt_fixture block.
> -Daniel
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH i-g-t] gem_storedw_loop: fix segfault when listing subtests
2015-09-23 16:27 ` Thomas Wood
@ 2015-09-24 10:13 ` Thomas Wood
2015-09-24 15:23 ` Jesse Barnes
0 siblings, 1 reply; 6+ messages in thread
From: Thomas Wood @ 2015-09-24 10:13 UTC (permalink / raw)
To: intel-gfx; +Cc: Daniel Vetter
Commit fb66cd5 (tests/gem_storedw_loop: Fix use after free for bufmgr)
introduced a segmentation fault when listing subtests because
drm_intel_bufmgr_destroy is called with NULL. Move this and the call to
the close function inside an igt_fixture block to prevent them being
called when listing subtests.
Cc: Robert Beckett <robert.beckett@intel.com>
Cc: Jesse Barnes <jbarnes@virtuousgeek.org>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: Thomas Wood <thomas.wood@intel.com>
---
tests/gem_storedw_loop.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/tests/gem_storedw_loop.c b/tests/gem_storedw_loop.c
index e7ebcc2..e17e190 100644
--- a/tests/gem_storedw_loop.c
+++ b/tests/gem_storedw_loop.c
@@ -180,6 +180,8 @@ igt_main
}
}
- drm_intel_bufmgr_destroy(bufmgr);
- close(fd);
+ igt_fixture {
+ drm_intel_bufmgr_destroy(bufmgr);
+ close(fd);
+ }
}
--
1.9.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH i-g-t] gem_storedw_loop: fix segfault when listing subtests
2015-09-24 10:13 ` [PATCH i-g-t] gem_storedw_loop: fix segfault when listing subtests Thomas Wood
@ 2015-09-24 15:23 ` Jesse Barnes
0 siblings, 0 replies; 6+ messages in thread
From: Jesse Barnes @ 2015-09-24 15:23 UTC (permalink / raw)
To: Thomas Wood, intel-gfx; +Cc: Daniel Vetter
On 09/24/2015 03:13 AM, Thomas Wood wrote:
> Commit fb66cd5 (tests/gem_storedw_loop: Fix use after free for bufmgr)
> introduced a segmentation fault when listing subtests because
> drm_intel_bufmgr_destroy is called with NULL. Move this and the call to
> the close function inside an igt_fixture block to prevent them being
> called when listing subtests.
>
> Cc: Robert Beckett <robert.beckett@intel.com>
> Cc: Jesse Barnes <jbarnes@virtuousgeek.org>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Signed-off-by: Thomas Wood <thomas.wood@intel.com>
> ---
> tests/gem_storedw_loop.c | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/tests/gem_storedw_loop.c b/tests/gem_storedw_loop.c
> index e7ebcc2..e17e190 100644
> --- a/tests/gem_storedw_loop.c
> +++ b/tests/gem_storedw_loop.c
> @@ -180,6 +180,8 @@ igt_main
> }
> }
>
> - drm_intel_bufmgr_destroy(bufmgr);
> - close(fd);
> + igt_fixture {
> + drm_intel_bufmgr_destroy(bufmgr);
> + close(fd);
> + }
> }
>
Um, yes. :) I won't pretend to know how the fixture stuff ought to
work, so:
Acked-by: Jesse Barnes <jbarnes@virtuousgeek.org>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2015-09-24 15:23 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-09-22 18:02 [PATCH i-g-t] tests/gem_storedw_loop: Fix use after free for bufmgr robert.beckett
2015-09-22 19:08 ` Jesse Barnes
2015-09-23 16:01 ` Daniel Vetter
2015-09-23 16:27 ` Thomas Wood
2015-09-24 10:13 ` [PATCH i-g-t] gem_storedw_loop: fix segfault when listing subtests Thomas Wood
2015-09-24 15:23 ` Jesse Barnes
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox