diff mbox

test-coroutine: Fix coroutine pool corruption

Message ID 1470827847-15983-1-git-send-email-kwolf@redhat.com
State New
Headers show

Commit Message

Kevin Wolf Aug. 10, 2016, 11:17 a.m. UTC
The test case overwrites the Coroutine object with 0xff as a way to
assert that the coroutine isn't used any more. However, this means that
the coroutine pool now contains a corrupted object and later test cases
may get this corrupted object and crash.

This patch saves the real content of the object and restores it after
completing the test. The only use of the coroutine pool between those
two points is the deletion of co2. As this only means an insertion at
the head of an SLIST (release_pool or alloc_pool), it doesn't access the
invalid list pointers that co1 has during this period.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 tests/test-coroutine.c | 7 +++++++
 1 file changed, 7 insertions(+)

Comments

Stefan Hajnoczi Aug. 11, 2016, 8:46 a.m. UTC | #1
On Wed, Aug 10, 2016 at 01:17:27PM +0200, Kevin Wolf wrote:
> The test case overwrites the Coroutine object with 0xff as a way to
> assert that the coroutine isn't used any more. However, this means that
> the coroutine pool now contains a corrupted object and later test cases
> may get this corrupted object and crash.
> 
> This patch saves the real content of the object and restores it after
> completing the test. The only use of the coroutine pool between those
> two points is the deletion of co2. As this only means an insertion at
> the head of an SLIST (release_pool or alloc_pool), it doesn't access the
> invalid list pointers that co1 has during this period.
> 
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
>  tests/test-coroutine.c | 7 +++++++
>  1 file changed, 7 insertions(+)

It's a really invasive test that has given us trouble before, but it
does test something useful...

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Kevin Wolf Aug. 12, 2016, 9:01 a.m. UTC | #2
Am 10.08.2016 um 13:17 hat Kevin Wolf geschrieben:
> The test case overwrites the Coroutine object with 0xff as a way to
> assert that the coroutine isn't used any more. However, this means that
> the coroutine pool now contains a corrupted object and later test cases
> may get this corrupted object and crash.
> 
> This patch saves the real content of the object and restores it after
> completing the test. The only use of the coroutine pool between those
> two points is the deletion of co2. As this only means an insertion at
> the head of an SLIST (release_pool or alloc_pool), it doesn't access the
> invalid list pointers that co1 has during this period.
> 
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>

Applied to block-next.

Kevin
diff mbox

Patch

diff --git a/tests/test-coroutine.c b/tests/test-coroutine.c
index ee5e06d..6431dd6 100644
--- a/tests/test-coroutine.c
+++ b/tests/test-coroutine.c
@@ -139,13 +139,20 @@  static void test_co_queue(void)
 {
     Coroutine *c1;
     Coroutine *c2;
+    Coroutine tmp;
 
     c2 = qemu_coroutine_create(c2_fn, NULL);
     c1 = qemu_coroutine_create(c1_fn, c2);
 
     qemu_coroutine_enter(c1);
+
+    /* c1 shouldn't be used any more now; make sure we segfault if it is */
+    tmp = *c1;
     memset(c1, 0xff, sizeof(Coroutine));
     qemu_coroutine_enter(c2);
+
+    /* Must restore the coroutine now to avoid corrupted pool */
+    *c1 = tmp;
 }
 
 /*