On 2025/09/11 5:41, Peter Xu wrote:
On Sat, Sep 06, 2025 at 04:11:11AM +0200, Akihiko Odaki wrote:
Children are automatically unparented so manually unparenting is
unnecessary.

Worse, automatic unparenting happens before the insntance_finalize()
callback of the parent gets called, so object_unparent() calls in
the callback will refer to objects that are already unparented, which
is semantically incorrect.

Signed-off-by: Akihiko Odaki <[email protected]>
---
  hw/vfio/pci.c | 4 ----
  1 file changed, 4 deletions(-)

diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
index 
07257d0fa049b09fc296ac2279a6fafbdf93d277..2e909c190f86a722e1022fa7c45a96d2dde8d58e
 100644
--- a/hw/vfio/pci.c
+++ b/hw/vfio/pci.c
@@ -2000,7 +2000,6 @@ static void vfio_bars_finalize(VFIOPCIDevice *vdev)
          vfio_region_finalize(&bar->region);
          if (bar->mr) {
              assert(bar->size);
-            object_unparent(OBJECT(bar->mr));
              g_free(bar->mr);
              bar->mr = NULL;
          }
@@ -2008,9 +2007,6 @@ static void vfio_bars_finalize(VFIOPCIDevice *vdev)
if (vdev->vga) {
          vfio_vga_quirk_finalize(vdev);
-        for (i = 0; i < ARRAY_SIZE(vdev->vga->region); i++) {
-            object_unparent(OBJECT(&vdev->vga->region[i].mem));
-        }
          g_free(vdev->vga);
      }
  }

So the 2nd object_unparent() here should be no-op, seeing empty list of
properties (but shouldn't causing anything severe), is that correct?

No. The object is finalized with the first object_unparent() if there is no referrer other than the parent. The second object_unparent() will access the finalized, invalid object in that case.


I think it still makes some sense to theoretically allow object_unparent()
to happen, at least when it happens before owner's finalize().  IIUC that
was the intention of the doc, pairing the memory_region_init*() operation.

Perhaps so, but this patch is only about the case where object_unparent() is called in finalize().

Regards,
Akihiko Odaki

Reply via email to