[PATCH v2] migration/options: Fix leaks in StrOrNull qdev accessors

Fabiano Rosas posted 1 patch 1 week, 3 days ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20260127150916.23329-1-farosas@suse.de
Maintainers: Peter Xu <peterx@redhat.com>, Fabiano Rosas <farosas@suse.de>
migration/options.c | 32 ++++++++++++++++----------------
1 file changed, 16 insertions(+), 16 deletions(-)
[PATCH v2] migration/options: Fix leaks in StrOrNull qdev accessors
Posted by Fabiano Rosas 1 week, 3 days ago
Fix a couple of leaks detected by Coverity. Both are currently
harmless.

- set_StrOrNull: the visitor should never fail unless there's a
programming error and a property of different type has been passed in.

Change it to only allocate memory after the visit call has returned
successfully.

- get_StrOrNull: the whole of the getter is unused, it's only purpose at
the moment is to provide a complete implementation of the StrOrNull
property. If it were used, it would always receive a non-NULL pointer
because this property is part of s->parameters and always initialized
by the setter.

Assert non-NULL instead of allocating a new object.

Fixes: CID 1643919
Fixes: CID 1643920
Reported-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Fabiano Rosas <farosas@suse.de>
---
 migration/options.c | 32 ++++++++++++++++----------------
 1 file changed, 16 insertions(+), 16 deletions(-)

diff --git a/migration/options.c b/migration/options.c
index 1ffe85a2d8..93d11bba60 100644
--- a/migration/options.c
+++ b/migration/options.c
@@ -216,36 +216,36 @@ const size_t migration_properties_count = ARRAY_SIZE(migration_properties);
 static void get_StrOrNull(Object *obj, Visitor *v, const char *name,
                           void *opaque, Error **errp)
 {
-    const Property *prop = opaque;
-    StrOrNull **ptr = object_field_prop_ptr(obj, prop);
+    StrOrNull **ptr = object_field_prop_ptr(obj, opaque);
     StrOrNull *str_or_null = *ptr;
 
-    if (!str_or_null) {
-        str_or_null = g_new0(StrOrNull, 1);
-        str_or_null->type = QTYPE_QSTRING;
-        str_or_null->u.s = g_strdup("");
-    } else {
-        /* the setter doesn't allow QNULL */
-        assert(str_or_null->type != QTYPE_QNULL);
-    }
+    /*
+     * The property should never be NULL because it's part of
+     * s->parameters and a default value is always set. It should also
+     * never be QNULL as the setter doesn't allow it.
+     */
+    assert(str_or_null && str_or_null->type != QTYPE_QNULL);
     visit_type_str(v, name, &str_or_null->u.s, errp);
 }
 
 static void set_StrOrNull(Object *obj, Visitor *v, const char *name,
                           void *opaque, Error **errp)
 {
-    const Property *prop = opaque;
-    StrOrNull **ptr = object_field_prop_ptr(obj, prop);
-    StrOrNull *str_or_null = g_new0(StrOrNull, 1);
+    StrOrNull **ptr = object_field_prop_ptr(obj, opaque);
+    StrOrNull *str_or_null;
+    char *str;
+
+    if (!visit_type_str(v, name, &str, errp)) {
+        return;
+    }
 
     /*
      * Only str to keep compatibility, QNULL was never used via
      * command line.
      */
+    str_or_null = g_new0(StrOrNull, 1);
     str_or_null->type = QTYPE_QSTRING;
-    if (!visit_type_str(v, name, &str_or_null->u.s, errp)) {
-        return;
-    }
+    str_or_null->u.s = str;
 
     qapi_free_StrOrNull(*ptr);
     *ptr = str_or_null;
-- 
2.51.0
Re: [PATCH v2] migration/options: Fix leaks in StrOrNull qdev accessors
Posted by Peter Xu 1 week, 3 days ago
On Tue, Jan 27, 2026 at 12:09:16PM -0300, Fabiano Rosas wrote:
> Fix a couple of leaks detected by Coverity. Both are currently
> harmless.
> 
> - set_StrOrNull: the visitor should never fail unless there's a
> programming error and a property of different type has been passed in.
> 
> Change it to only allocate memory after the visit call has returned
> successfully.
> 
> - get_StrOrNull: the whole of the getter is unused, it's only purpose at
> the moment is to provide a complete implementation of the StrOrNull
> property. If it were used, it would always receive a non-NULL pointer
> because this property is part of s->parameters and always initialized
> by the setter.
> 
> Assert non-NULL instead of allocating a new object.
> 
> Fixes: CID 1643919
> Fixes: CID 1643920
> Reported-by: Peter Maydell <peter.maydell@linaro.org>
> Signed-off-by: Fabiano Rosas <farosas@suse.de>

Reviewed-by: Peter Xu <peterx@redhat.com>

-- 
Peter Xu