From: Eric Auger <eric.auger@redhat.com>
To: eric.auger.pro@gmail.com, eric.auger@redhat.com,
dgilbert@redhat.com, quintela@redhat.com, qemu-devel@nongnu.org,
rjones@redhat.com, marcandre.lureau@redhat.com
Cc: armbru@redhat.com, philmd@linaro.org, berrange@redhat.com
Subject: [PATCH v4] test-vmstate: fix bad GTree usage, use-after-free
Date: Tue, 28 Feb 2023 10:29:44 +0100 [thread overview]
Message-ID: <20230228092944.341317-1-eric.auger@redhat.com> (raw)
According to g_tree_foreach() documentation:
"The tree may not be modified while iterating over it (you can't
add/remove items)."
compare_trees()/diff_tree() fail to respect this rule.
Historically GLib2 used a slice allocator for the GTree APIs
which did not immediately release the memory back to the system
allocator. As a result QEMU's use-after-free bug was not visible.
With GLib > 2.75.3 however, GLib2 has switched to using malloc
and now a SIGSEGV can be observed while running test-vmstate.
Get rid of the node removal within the tree traversal. Also
check the trees have the same number of nodes before the actual
diff.
Fixes: 9a85e4b8f6 ("migration: Support gtree migration")
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1518
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Signed-off-by: Eric Auger <eric.auger@redhat.com>
Reported-by: Richard W.M. Jones <rjones@redhat.com>
Tested-by: Richard W.M. Jones <rjones@redhat.com>
Reviewed-by: Richard W.M. Jones <rjones@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
---
v3 -> v4:
- Adopt Daniel's wording and add his R-b
v2 -> v3:
- Enhance the commit message with Rich's explanations
- Add Roch's T-b and R-b
v1 -> v2:
- respin of Marc-André's patch from Aug 2020, which can be
found at
https://lore.kernel.org/qemu-devel/20200827161826.1165971-1-marcandre.lureau@redhat.com/
This fell through the cracks and now we hit a SIGSEGV
---
tests/unit/test-vmstate.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/tests/unit/test-vmstate.c b/tests/unit/test-vmstate.c
index 79357b29ca..0b7d5ecd68 100644
--- a/tests/unit/test-vmstate.c
+++ b/tests/unit/test-vmstate.c
@@ -1073,7 +1073,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);
return false;
}
@@ -1082,9 +1081,9 @@ static void compare_trees(GTree *tree1, GTree *tree2,
{
struct tree_cmp_data tp = {tree1, tree2, function};
+ assert(g_tree_nnodes(tree1) == g_tree_nnodes(tree2));
g_tree_foreach(tree1, diff_tree, &tp);
- assert(g_tree_nnodes(tree1) == 0);
- assert(g_tree_nnodes(tree2) == 0);
+ g_tree_destroy(g_tree_ref(tree1));
}
static void diff_domain(TestGTreeDomain *d1, TestGTreeDomain *d2)
--
2.38.1
next reply other threads:[~2023-02-28 9:30 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-02-28 9:29 Eric Auger [this message]
2023-02-28 10:41 ` [PATCH v4] test-vmstate: fix bad GTree usage, use-after-free Juan Quintela
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20230228092944.341317-1-eric.auger@redhat.com \
--to=eric.auger@redhat.com \
--cc=armbru@redhat.com \
--cc=berrange@redhat.com \
--cc=dgilbert@redhat.com \
--cc=eric.auger.pro@gmail.com \
--cc=marcandre.lureau@redhat.com \
--cc=philmd@linaro.org \
--cc=qemu-devel@nongnu.org \
--cc=quintela@redhat.com \
--cc=rjones@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).