[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] test-vmstate: fix bad GTree usage, use-after-free
From: |
Auger Eric |
Subject: |
Re: [PATCH] test-vmstate: fix bad GTree usage, use-after-free |
Date: |
Thu, 27 Aug 2020 18:34:03 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.5.0 |
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.
Also there is another case of removal inside the traversal in the
match_node(): in match_interval_mapping_node() and match_domain_node()
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);
> }
>
>