qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: John Snow <jsnow@redhat.com>
To: qemu-devel@nongnu.org
Cc: peter.maydell@linaro.org, jsnow@redhat.com,
	Igor Mammedov <imammedo@redhat.com>
Subject: [Qemu-devel] [PULL 01/13] ide: ahci: unparent children buses before freeing their memory
Date: Mon, 18 Sep 2017 20:11:35 -0400	[thread overview]
Message-ID: <20170919001147.23182-3-jsnow@redhat.com> (raw)
In-Reply-To: <20170919001147.23182-1-jsnow@redhat.com>

From: Igor Mammedov <imammedo@redhat.com>

Fixes read after freeing error reported
  https://lists.gnu.org/archive/html/qemu-devel/2017-08/msg04243.html
  Message-Id: <59a56959-ca12-ea75-33fa-ff07eba1b090@redhat.com>

ich9-ahci device creates ide buses and attaches them as QOM children
at realize time, however it forgets to properly clean them up
at unrealize time and frees memory containing these children,
with following call-chain:

   qdev_device_add()
     object_property_set_bool('realized', true)
       device_set_realized()
          ...
          pci_qdev_realize() -> pci_ich9_ahci_realize() -> ahci_realize()
               ...
               s->dev = g_new0(AHCIDevice, ports);
               ...
                  AHCIDevice *ad = &s->dev[i];
                  ide_bus_new(&ad->port, sizeof(ad->port), qdev, i, 1);
                  ^^^ creates bus in memory allocated by above gnew()
                      and adds it as child propety to ahci device
          ...
          hotplug_handler_plug(); -> goto post_realize_fail;
          pci_qdev_unrealize() -> pci_ich9_uninit() -> ahci_uninit()
              ...
               g_free(s->dev);
               ^^^ free memory that holds children busses

          return with error from device_set_realized()

As result later when qdev_device_add() tries to unparent ich9-ahci
after failed device_set_realized(),
    object_unparent() -> object_property_del_child()
iterates over existing QOM children including buses added by
ide_bus_new() and tries to unparent them, which causes access to
freed memory where they where located.

Reported-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: Igor Mammedov <imammedo@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Tested-by: Thomas Huth <thuth@redhat.com>
Reviewed-by: John Snow <jsnow@redhat.com>
Message-id: 1503938085-169486-1-git-send-email-imammedo@redhat.com
Signed-off-by: John Snow <jsnow@redhat.com>
---
 hw/ide/ahci.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c
index 406a1b5..ccbe091 100644
--- a/hw/ide/ahci.c
+++ b/hw/ide/ahci.c
@@ -1495,6 +1495,7 @@ void ahci_uninit(AHCIState *s)
 
             ide_exit(s);
         }
+        object_unparent(OBJECT(&ad->port));
     }
 
     g_free(s->dev);
-- 
2.9.5

  parent reply	other threads:[~2017-09-19  0:11 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-09-19  0:11 [Qemu-devel] [PULL 13/13] hw/block/fdc: Convert to realize John Snow
2017-09-19  0:11 ` [Qemu-devel] [PULL 00/13] Ide patches John Snow
2017-09-19 10:04   ` Peter Maydell
2017-09-19  0:11 ` John Snow [this message]
2017-09-19  0:11 ` [Qemu-devel] [PULL 02/13] hw/ide/microdrive: Mark the dscm1xxxx device with user_creatable = false John Snow
2017-09-19  0:11 ` [Qemu-devel] [PULL 03/13] IDE: replace DEBUG_IDE with tracing system John Snow
2017-09-19  0:11 ` [Qemu-devel] [PULL 04/13] IDE: Add register hints to tracing John Snow
2017-09-19  0:11 ` [Qemu-devel] [PULL 05/13] IDE: add tracing for data ports John Snow
2017-09-19  0:11 ` [Qemu-devel] [PULL 06/13] ATAPI: Replace DEBUG_IDE_ATAPI with tracing events John Snow
2017-09-19  0:11 ` [Qemu-devel] [PULL 07/13] IDE: replace DEBUG_AIO with trace events John Snow
2017-09-19  0:11 ` [Qemu-devel] [PULL 08/13] AHCI: Replace DPRINTF with trace-events John Snow
2017-09-19  0:11 ` [Qemu-devel] [PULL 09/13] AHCI: Rework IRQ constants John Snow
2017-09-19  0:11 ` [Qemu-devel] [PULL 10/13] AHCI: pretty-print FIS to buffer instead of stderr John Snow
2017-09-19  0:11 ` [Qemu-devel] [PULL 11/13] AHCI: remove DPRINTF macro John Snow
2017-09-19  0:11 ` [Qemu-devel] [PULL 12/13] hw/ide: Convert DeviceClass init to realize John Snow
2017-09-19  0:11 ` [Qemu-devel] [PULL 13/13] hw/block/fdc: Convert " John Snow

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=20170919001147.23182-3-jsnow@redhat.com \
    --to=jsnow@redhat.com \
    --cc=imammedo@redhat.com \
    --cc=peter.maydell@linaro.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 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).