[PULL 07/10] migration/options: Fix leaks in StrOrNull qdev accessors

[Fabiano Rosas <farosas@suse.de>]

Fix a couple of possible leaks detected by Coverity. Both are
currently harmless. This code is only used for the very specific
purpose of maintaining compatibility of a few migration options which
can be set via QEMU command line (-global migration.tls-*). The
command line interface is not supported and only used during
development and testing.

1) The setter function set_StrOrNull() is invoked whenever the -global
migration.tls-* command line options are set. The way it could leak is
that the temporary "StrOrNull *str_or_null" object is allocated before
calling the visitor, which could fail and cause an early return of the
function, leaving *ptr unset and str_or_null leaking.

2) The getter function get_StrOrNull() is unreachable code. It's only
there to provide a complete implementation of the property. Still, the
way it could leak is that the temporary "StrOrNull *str_or_null" might
be allocated and is simply never returned to the caller nor freed.

Fix the possible leaks:

1) at set_StrOrNull(): change the allocation of str_or_null to happen
only after the visit call has returned successfully.

2) at get_StrOrNull(): assert that the object is non-NULL, there is no
need for a temporary object.

The reason it should be non-NULL is that the property is initialized
by the default setter of the qdev property. The initialization is
unlikely to fail because the call to the setter is setup by qdev,
which has boilerplate ensuring the to-be-set object is allocated and
of the correct type. Moreover, passing NULL via command line to
-global migration.tls-* is not possible.

A programming error could result in an invalid call to the setter,
which would leave the object NULL and cause a crash in the getter, but
that's not a worthwhile scenario to protect against given the low
probability of this code being even reached.

While here, update the comment about why there's no QNULL in this
StrOrNull property to be more clear.

Fixes: CID 1643919
Fixes: CID 1643920
Cc: Markus Armbruster <armbru@redhat.com>
Reported-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Peter Xu <peterx@redhat.com>
Reviewed-by: Prasad Pandit <pjp@fedoraproject.org>
Link: https://lore.kernel.org/qemu-devel/20260312204619.1969-1-farosas@suse.de
Signed-off-by: Fabiano Rosas <farosas@suse.de>
---
 migration/options.c | 35 +++++++++++++++++++++--------------
 1 file changed, 21 insertions(+), 14 deletions(-)

diff --git a/migration/options.c b/migration/options.c
index f33b297929..658c578191 100644
--- a/migration/options.c
+++ b/migration/options.c
@@ -220,14 +220,12 @@ static void get_StrOrNull(Object *obj, Visitor *v, const char *name,
     StrOrNull **ptr = object_field_prop_ptr(obj, prop);
     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 by qdev. 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);
 }
 
@@ -236,16 +234,25 @@ static void set_StrOrNull(Object *obj, Visitor *v, const char *name,
 {
     const Property *prop = opaque;
     StrOrNull **ptr = object_field_prop_ptr(obj, prop);
-    StrOrNull *str_or_null = g_new0(StrOrNull, 1);
+    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.
+     * This property only applies to the command line usage of
+     * migration's TLS options (-global migration.tls-*) where the
+     * NULL value cannot be provided as input (only strings are
+     * allowed). Therefore, this StrOrNull implementation never
+     * produces a QNULL value to avoid ever returning values outside
+     * the range of what was previously handled by consumers of the
+     * TLS options.
      */
+    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