All of lore.kernel.org
 help / color / mirror / Atom feed
From: Daniel Henrique Barboza <danielhb413@gmail.com>
To: qemu-devel@nongnu.org
Cc: Daniel Henrique Barboza <danielhb413@gmail.com>,
	Peter Maydell <peter.maydell@linaro.org>,
	Markus Armbruster <armbru@redhat.com>,
	qemu-arm@nongnu.org
Subject: [PATCH 1/1] hw/arm: do not free machine->fdt in arm_load_dtb()
Date: Thu, 23 Mar 2023 13:10:53 -0300	[thread overview]
Message-ID: <20230323161053.412356-2-danielhb413@gmail.com> (raw)
In-Reply-To: <20230323161053.412356-1-danielhb413@gmail.com>

At this moment, arm_load_dtb() can free machine->fdt when
binfo->dtb_filename is NULL. If there's no 'dtb_filename', 'fdt' will be
retrieved by binfo->get_dtb(). If get_dtb() returns machine->fdt, as is
the case of machvirt_dtb() from hw/arm/virt.c, fdt now has a pointer to
machine->fdt. And, in that case, the existing g_free(fdt) at the end of
arm_load_dtb() will make machine->fdt point to an invalid memory region.

After the command 'dumpdtb' were introduced a couple of releases ago,
running it with any ARM machine that uses arm_load_dtb() will crash
QEMU.

One alternative would be to mark machine->fdt = NULL when exiting
arm_load_dtb() when freeing the fdt. Another is to not free the fdt and,
instead, update machine->fdt with the new fdt generated. This will
enable dumpdtb for all ARM machines that uses arm_load_dtb(), regardless
of having 'dtb_filename' or not.

Cc: Peter Maydell <peter.maydell@linaro.org>
Cc: qemu-arm@nongnu.org
Fixes: bf353ad55590f ("qmp/hmp, device_tree.c: introduce dumpdtb")
Reported-by: Markus Armbruster <armbru@redhat.com>i
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
---
 hw/arm/boot.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/hw/arm/boot.c b/hw/arm/boot.c
index 50e5141116..9418cc3373 100644
--- a/hw/arm/boot.c
+++ b/hw/arm/boot.c
@@ -689,7 +689,8 @@ int arm_load_dtb(hwaddr addr, const struct arm_boot_info *binfo,
     qemu_register_reset_nosnapshotload(qemu_fdt_randomize_seeds,
                                        rom_ptr_for_as(as, addr, size));
 
-    g_free(fdt);
+    /* Set ms->fdt for 'dumpdtb' QMP/HMP command */
+    ms->fdt = fdt;
 
     return size;
 
-- 
2.39.2


  reply	other threads:[~2023-03-23 16:11 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-03-23 16:10 [PATCH 0/1] fix dumpdtb crash with ARM machines Daniel Henrique Barboza
2023-03-23 16:10 ` Daniel Henrique Barboza [this message]
2023-03-23 17:38   ` [PATCH 1/1] hw/arm: do not free machine->fdt in arm_load_dtb() Peter Maydell
2023-03-23 17:54     ` Daniel Henrique Barboza
2023-03-23 17:59       ` Peter Maydell
2023-03-23 20:15         ` Daniel Henrique Barboza

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=20230323161053.412356-2-danielhb413@gmail.com \
    --to=danielhb413@gmail.com \
    --cc=armbru@redhat.com \
    --cc=peter.maydell@linaro.org \
    --cc=qemu-arm@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.