From b2c33dbdec6e1abecee77c28398275a7337d6154 Mon Sep 17 00:00:00 2001

From: Serhii Popovych <spopovyc@redhat.com>

Date: Mon, 9 Jul 2018 11:31:16 +0200

Subject: [PATCH 26/89] object: fix OBJ_PROP_LINK_UNREF_ON_RELEASE ambivalence

MIME-Version: 1.0

Content-Type: text/plain; charset=UTF-8

Content-Transfer-Encoding: 8bit

RH-Author: Serhii Popovych <spopovyc@redhat.com>

Message-id: <1531135878-18813-2-git-send-email-spopovyc@redhat.com>

Patchwork-id: 81265

O-Subject: [RHEL-7.6 qemu-kvm-rhev PATCH v2 1/3] object: fix OBJ_PROP_LINK_UNREF_ON_RELEASE ambivalence

Bugzilla: 1556678

RH-Acked-by: Laurent Vivier <lvivier@redhat.com>

RH-Acked-by: Marc-André Lureau <marcandre.lureau@redhat.com>

RH-Acked-by: David Gibson <dgibson@redhat.com>

From: Marc-André Lureau <marcandre.lureau@redhat.com>

A link property can be set during creation, with

object_property_add_link() and later with object_property_set_link().

add_link() doesn't add a reference to the target object, while

set_link() does.

Furthemore, OBJ_PROP_LINK_UNREF_ON_RELEASE flags, set during add_link,

says whether a reference must be released when the property is destroyed.

This can lead to leaks if the property was later set_link(), as the

added reference is never released.

Instead, rename OBJ_PROP_LINK_UNREF_ON_RELEASE to OBJ_PROP_LINK_STRONG

and use that has an indication on how the link handle reference

management in set_link().

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>

Message-id: 20180531195119.22021-3-marcandre.lureau@redhat.com

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>

(cherry picked from commit 265b578c584b1a86c7028790deaa2f4392dd0a65)

Signed-off-by: Serhii Popovych <spopovyc@redhat.com>

Signed-off-by: Miroslav Rezanina <mrezanin@redhat.com>

Conflicts:

	hw/dma/xlnx-zdma.c

This file is missing in 2.12.0.

---

 hw/core/qdev-properties.c |  2 +-

 hw/core/qdev.c            |  2 +-

 hw/display/xlnx_dp.c      |  2 +-

 hw/dma/xilinx_axidma.c    |  4 ++--

 hw/i386/pc.c              |  2 +-

 hw/i386/pc_piix.c         |  2 +-

 hw/i386/pc_q35.c          |  2 +-

 hw/ipmi/ipmi.c            |  2 +-

 hw/net/xilinx_axienet.c   |  4 ++--

 hw/ssi/xilinx_spips.c     |  2 +-

 include/qom/object.h      | 12 +++++++++---

 net/can/can_host.c        |  2 +-

 net/colo-compare.c        |  2 +-

 qom/object.c              |  8 +++++---

 target/arm/cpu.c          |  4 ++--

 ui/console.c              |  2 +-

 16 files changed, 31 insertions(+), 23 deletions(-)

diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c

index 5bbc2d9..f3a83a3 100644

--- a/hw/core/qdev-properties.c

+++ b/hw/core/qdev-properties.c

@@ -1309,7 +1309,7 @@ static void create_link_property(Object *obj, Property *prop, Error **errp)

     object_property_add_link(obj, prop->name, prop->link_type,

                              child,

                              qdev_prop_allow_set_link_before_realize,

-                             OBJ_PROP_LINK_UNREF_ON_RELEASE,

+                             OBJ_PROP_LINK_STRONG,

                              errp);

 }

diff --git a/hw/core/qdev.c b/hw/core/qdev.c

index f6f9247..ce7c316 100644

--- a/hw/core/qdev.c

+++ b/hw/core/qdev.c

@@ -435,7 +435,7 @@ void qdev_init_gpio_out_named(DeviceState *dev, qemu_irq *pins,

         object_property_add_link(OBJECT(dev), propname, TYPE_IRQ,

                                  (Object **)&pins[i],

                                  object_property_allow_set_link,

-                                 OBJ_PROP_LINK_UNREF_ON_RELEASE,

+                                 OBJ_PROP_LINK_STRONG,

                                  &error_abort);

         g_free(propname);

     }

diff --git a/hw/display/xlnx_dp.c b/hw/display/xlnx_dp.c

index 6715b9c..b737e1d 100644

--- a/hw/display/xlnx_dp.c

+++ b/hw/display/xlnx_dp.c

@@ -1221,7 +1221,7 @@ static void xlnx_dp_init(Object *obj)

     object_property_add_link(obj, "dpdma", TYPE_XLNX_DPDMA,

                              (Object **) &s->dpdma,

                              xlnx_dp_set_dpdma,

-                             OBJ_PROP_LINK_UNREF_ON_RELEASE,

+                             OBJ_PROP_LINK_STRONG,

                              &error_abort);

     /*

diff --git a/hw/dma/xilinx_axidma.c b/hw/dma/xilinx_axidma.c

index 9b48103..401a328 100644

--- a/hw/dma/xilinx_axidma.c

+++ b/hw/dma/xilinx_axidma.c

@@ -525,12 +525,12 @@ static void xilinx_axidma_realize(DeviceState *dev, Error **errp)

     object_property_add_link(OBJECT(ds), "dma", TYPE_XILINX_AXI_DMA,

                              (Object **)&ds->dma,

                              object_property_allow_set_link,

-                             OBJ_PROP_LINK_UNREF_ON_RELEASE,

+                             OBJ_PROP_LINK_STRONG,

                              &local_err);

     object_property_add_link(OBJECT(cs), "dma", TYPE_XILINX_AXI_DMA,

                              (Object **)&cs->dma,

                              object_property_allow_set_link,

-                             OBJ_PROP_LINK_UNREF_ON_RELEASE,

+                             OBJ_PROP_LINK_STRONG,

                              &local_err);

     if (local_err) {

         goto xilinx_axidma_realize_fail;

diff --git a/hw/i386/pc.c b/hw/i386/pc.c

index 265decf..7160969 100644

--- a/hw/i386/pc.c

+++ b/hw/i386/pc.c

@@ -485,7 +485,7 @@ void pc_cmos_init(PCMachineState *pcms,

                              TYPE_ISA_DEVICE,

                              (Object **)&pcms->rtc,

                              object_property_allow_set_link,

-                             OBJ_PROP_LINK_UNREF_ON_RELEASE, &error_abort);

+                             OBJ_PROP_LINK_STRONG, &error_abort);

     object_property_set_link(OBJECT(pcms), OBJECT(s),

                              "rtc_state", &error_abort);

diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c

index 8a355df..00549c4 100644

--- a/hw/i386/pc_piix.c

+++ b/hw/i386/pc_piix.c

@@ -290,7 +290,7 @@ static void pc_init1(MachineState *machine,

                                  TYPE_HOTPLUG_HANDLER,

                                  (Object **)&pcms->acpi_dev,

                                  object_property_allow_set_link,

-                                 OBJ_PROP_LINK_UNREF_ON_RELEASE, &error_abort);

+                                 OBJ_PROP_LINK_STRONG, &error_abort);

         object_property_set_link(OBJECT(machine), OBJECT(piix4_pm),

                                  PC_MACHINE_ACPI_DEVICE_PROP, &error_abort);

     }

diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c

index e17807e..a273bd5 100644

--- a/hw/i386/pc_q35.c

+++ b/hw/i386/pc_q35.c

@@ -194,7 +194,7 @@ static void pc_q35_init(MachineState *machine)

                              TYPE_HOTPLUG_HANDLER,

                              (Object **)&pcms->acpi_dev,

                              object_property_allow_set_link,

-                             OBJ_PROP_LINK_UNREF_ON_RELEASE, &error_abort);

+                             OBJ_PROP_LINK_STRONG, &error_abort);

     object_property_set_link(OBJECT(machine), OBJECT(lpc),

                              PC_MACHINE_ACPI_DEVICE_PROP, &error_abort);

diff --git a/hw/ipmi/ipmi.c b/hw/ipmi/ipmi.c

index 9be281f..63c0317 100644

--- a/hw/ipmi/ipmi.c

+++ b/hw/ipmi/ipmi.c

@@ -104,7 +104,7 @@ void ipmi_bmc_find_and_link(Object *obj, Object **bmc)

 {

     object_property_add_link(obj, "bmc", TYPE_IPMI_BMC, bmc,

                              isa_ipmi_bmc_check,

-                             OBJ_PROP_LINK_UNREF_ON_RELEASE,

+                             OBJ_PROP_LINK_STRONG,

                              &error_abort);

 }

diff --git a/hw/net/xilinx_axienet.c b/hw/net/xilinx_axienet.c

index d4c2c89..cc880a3 100644

--- a/hw/net/xilinx_axienet.c

+++ b/hw/net/xilinx_axienet.c

@@ -951,12 +951,12 @@ static void xilinx_enet_realize(DeviceState *dev, Error **errp)

     object_property_add_link(OBJECT(ds), "enet", "xlnx.axi-ethernet",

                              (Object **) &ds->enet,

                              object_property_allow_set_link,

-                             OBJ_PROP_LINK_UNREF_ON_RELEASE,

+                             OBJ_PROP_LINK_STRONG,

                              &local_err);

     object_property_add_link(OBJECT(cs), "enet", "xlnx.axi-ethernet",

                              (Object **) &cs->enet,

                              object_property_allow_set_link,

-                             OBJ_PROP_LINK_UNREF_ON_RELEASE,

+                             OBJ_PROP_LINK_STRONG,

                              &local_err);

     if (local_err) {

         goto xilinx_enet_realize_fail;

diff --git a/hw/ssi/xilinx_spips.c b/hw/ssi/xilinx_spips.c

index 426f971..068878c 100644

--- a/hw/ssi/xilinx_spips.c

+++ b/hw/ssi/xilinx_spips.c

@@ -1345,7 +1345,7 @@ static void xlnx_zynqmp_qspips_init(Object *obj)

     object_property_add_link(obj, "stream-connected-dma", TYPE_STREAM_SLAVE,

                              (Object **)&rq->dma,

                              object_property_allow_set_link,

-                             OBJ_PROP_LINK_UNREF_ON_RELEASE,

+                             OBJ_PROP_LINK_STRONG,

                              NULL);

 }

diff --git a/include/qom/object.h b/include/qom/object.h

index 96ce81b..fc4555d 100644

--- a/include/qom/object.h

+++ b/include/qom/object.h

@@ -1103,6 +1103,11 @@ char *object_property_get_str(Object *obj, const char *name,

  * @errp: returns an error if this function fails

  *

  * Writes an object's canonical path to a property.

+ *

+ * If the link property was created with

+ * OBJ_PROP_LINK_STRONG bit, the old target object is

+ * unreferenced, and a reference is added to the new target object.

+ *

  */

 void object_property_set_link(Object *obj, Object *value,

                               const char *name, Error **errp);

@@ -1393,7 +1398,7 @@ void object_property_add_child(Object *obj, const char *name,

 typedef enum {

     /* Unref the link pointer when the property is deleted */

-    OBJ_PROP_LINK_UNREF_ON_RELEASE = 0x1,

+    OBJ_PROP_LINK_STRONG = 0x1,

 } ObjectPropertyLinkFlags;

 /**

@@ -1431,8 +1436,9 @@ void object_property_allow_set_link(const Object *, const char *,

  * link property.  The reference count for *@child is

  * managed by the property from after the function returns till the

  * property is deleted with object_property_del().  If the

- * @flags OBJ_PROP_LINK_UNREF_ON_RELEASE bit is set,

- * the reference count is decremented when the property is deleted.

+ * @flags OBJ_PROP_LINK_STRONG bit is set,

+ * the reference count is decremented when the property is deleted or

+ * modified.

  */

 void object_property_add_link(Object *obj, const char *name,

                               const char *type, Object **child,

diff --git a/net/can/can_host.c b/net/can/can_host.c

index c3d2652..c79347a 100644

--- a/net/can/can_host.c

+++ b/net/can/can_host.c

@@ -77,7 +77,7 @@ static void can_host_instance_init(Object *obj)

     object_property_add_link(obj, "canbus", TYPE_CAN_BUS,

                              (Object **)&ch->bus,

                              object_property_allow_set_link,

-                             OBJ_PROP_LINK_UNREF_ON_RELEASE,

+                             OBJ_PROP_LINK_STRONG,

                              &error_abort);

 }

diff --git a/net/colo-compare.c b/net/colo-compare.c

index 23b2d2c..63469b1 100644

--- a/net/colo-compare.c

+++ b/net/colo-compare.c

@@ -980,7 +980,7 @@ static void colo_compare_init(Object *obj)

     object_property_add_link(obj, "iothread", TYPE_IOTHREAD,

                             (Object **)&s->iothread,

                             object_property_allow_set_link,

-                            OBJ_PROP_LINK_UNREF_ON_RELEASE, NULL);

+                            OBJ_PROP_LINK_STRONG, NULL);

     s->vnet_hdr = false;

     object_property_add_bool(obj, "vnet_hdr_support", compare_get_vnet_hdr,

diff --git a/qom/object.c b/qom/object.c

index 76a89af..c4f1d36 100644

--- a/qom/object.c

+++ b/qom/object.c

@@ -1564,9 +1564,11 @@ static void object_set_link_property(Object *obj, Visitor *v,

         return;

     }

-    object_ref(new_target);

     *child = new_target;

-    object_unref(old_target);

+    if (prop->flags == OBJ_PROP_LINK_STRONG) {

+        object_ref(new_target);

+        object_unref(old_target);

+    }

 }

 static Object *object_resolve_link_property(Object *parent, void *opaque, const gchar *part)

@@ -1581,7 +1583,7 @@ static void object_release_link_property(Object *obj, const char *name,

 {

     LinkProperty *prop = opaque;

-    if ((prop->flags & OBJ_PROP_LINK_UNREF_ON_RELEASE) && *prop->child) {

+    if ((prop->flags & OBJ_PROP_LINK_STRONG) && *prop->child) {

         object_unref(*prop->child);

     }

     g_free(prop);

diff --git a/target/arm/cpu.c b/target/arm/cpu.c

index 4255e9c..9d030e0 100644

--- a/target/arm/cpu.c

+++ b/target/arm/cpu.c

@@ -672,7 +672,7 @@ static void arm_cpu_post_init(Object *obj)

                                  TYPE_MEMORY_REGION,

                                  (Object **)&cpu->secure_memory,

                                  qdev_prop_allow_set_link_before_realize,

-                                 OBJ_PROP_LINK_UNREF_ON_RELEASE,

+                                 OBJ_PROP_LINK_STRONG,

                                  &error_abort);

 #endif

     }

@@ -700,7 +700,7 @@ static void arm_cpu_post_init(Object *obj)

     if (arm_feature(&cpu->env, ARM_FEATURE_M_SECURITY)) {

         object_property_add_link(obj, "idau", TYPE_IDAU_INTERFACE, &cpu->idau,

                                  qdev_prop_allow_set_link_before_realize,

-                                 OBJ_PROP_LINK_UNREF_ON_RELEASE,

+                                 OBJ_PROP_LINK_STRONG,

                                  &error_abort);

         qdev_property_add_static(DEVICE(obj), &arm_cpu_initsvtor_property,

                                  &error_abort);

diff --git a/ui/console.c b/ui/console.c

index 73b2d3c..4431e77 100644

--- a/ui/console.c

+++ b/ui/console.c

@@ -1305,7 +1305,7 @@ static QemuConsole *new_console(DisplayState *ds, console_type_t console_type,

     object_property_add_link(obj, "device", TYPE_DEVICE,

                              (Object **)&s->device,

                              object_property_allow_set_link,

-                             OBJ_PROP_LINK_UNREF_ON_RELEASE,

+                             OBJ_PROP_LINK_STRONG,

                              &error_abort);

     object_property_add_uint32_ptr(obj, "head",

                                    &s->head, &error_abort);

-- 

1.8.3.1