qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH] test-vmstate: fix bad GTree usage, use-after-free


From: Marc-André Lureau
Subject: Re: [PATCH] test-vmstate: fix bad GTree usage, use-after-free
Date: Thu, 27 Aug 2020 22:35:34 +0400

Hi

On Thu, Aug 27, 2020 at 8:34 PM Auger Eric <eric.auger@redhat.com> wrote:
Hi Marc-Andre

On 8/27/20 6:18 PM, marcandre.lureau@redhat.com wrote:
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
>
> According to g_tree_foreach() documentation:
> "The tree may not be modified while iterating over it (you can't
> add/remove items)."

Hum I did not see that.
>
> Fixes: 9a85e4b8f6 ("migration: Support gtree migration")
> Cc: Eric Auger <eric.auger@redhat.com>
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>  tests/test-vmstate.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/tests/test-vmstate.c b/tests/test-vmstate.c
> index f7b3868881..31aefa78f0 100644
> --- a/tests/test-vmstate.c
> +++ b/tests/test-vmstate.c
> @@ -1078,7 +1078,6 @@ static gboolean diff_tree(gpointer key, gpointer value, gpointer data)
>      struct match_node_data d = {tp->tree2, key, value};

>      g_tree_foreach(tp->tree2, tp->match_node, &d);
> -    g_tree_remove(tp->tree1, key);
it does not test the same thing I am afraid. If one of the trees
contains more elements than the others this won't be detected.


-    assert(g_tree_nnodes(tree1) == 0);

Was simply checking that all nodes from tree1 were deleted.

Whatever else must have been checked elsewhere or differently by new code.
 
Also there is another case of removal inside the traversal in the
match_node(): in match_interval_mapping_node() and match_domain_node()


Yes, but they don't update the traversed tree.

Thanks

Eric


>      return false;
>  }

> @@ -1088,7 +1087,7 @@ static void compare_trees(GTree *tree1, GTree *tree2,
>      struct tree_cmp_data tp = {tree1, tree2, function};

>      g_tree_foreach(tree1, diff_tree, &tp);
> -    assert(g_tree_nnodes(tree1) == 0);> +    g_tree_destroy(g_tree_ref(tree1));
>      assert(g_tree_nnodes(tree2) == 0);
>  }

>




--
Marc-André Lureau

reply via email to

[Prev in Thread] Current Thread [Next in Thread]