@samitouri / QOSamiQemu / commits / 11d1b7da67

migration/options: Fix leaks in StrOrNull qdev accessors

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>

Fabiano Rosas committed Mar 12, 2026 at 17:46 UTC 11d1b7da67d585a8cf7d4b265a1447a049f34109

1 file changed +21 -14

migration/options.c

+21 -14

@@ -220,14 +220,12 @@ static void get_StrOrNull(Object *obj, Visitor *v, const char *name,

220 StrOrNull **ptr = object_field_prop_ptr(obj, prop);

221 StrOrNull *str_or_null = *ptr;

222

223 - if (!str_or_null) {

224 - str_or_null = g_new0(StrOrNull, 1);

225 - str_or_null->type = QTYPE_QSTRING;

226 - str_or_null->u.s = g_strdup("");

227 - } else {

228 - /* the setter doesn't allow QNULL */

229 - assert(str_or_null->type != QTYPE_QNULL);

230 - }

223 + /*

224 + * The property should never be NULL because it's part of

225 + * s->parameters and a default value is always set by qdev. It

226 + * should also never be QNULL as the setter doesn't allow it.

227 + */

228 + assert(str_or_null && str_or_null->type != QTYPE_QNULL);

229 visit_type_str(v, name, &str_or_null->u.s, errp);

230 }

231

@@ -236,16 +234,25 @@ static void set_StrOrNull(Object *obj, Visitor *v, const char *name,

234 {

235 const Property *prop = opaque;

236 StrOrNull **ptr = object_field_prop_ptr(obj, prop);

239 - StrOrNull *str_or_null = g_new0(StrOrNull, 1);

237 + StrOrNull *str_or_null;

238 + char *str;

239 +

240 + if (!visit_type_str(v, name, &str, errp)) {

241 + return;

242 + }

243

244 /*

242 - * Only str to keep compatibility, QNULL was never used via

243 - * command line.

245 + * This property only applies to the command line usage of

246 + * migration's TLS options (-global migration.tls-*) where the

247 + * NULL value cannot be provided as input (only strings are

248 + * allowed). Therefore, this StrOrNull implementation never

249 + * produces a QNULL value to avoid ever returning values outside

250 + * the range of what was previously handled by consumers of the

251 + * TLS options.

252 */

253 + str_or_null = g_new0(StrOrNull, 1);

254 str_or_null->type = QTYPE_QSTRING;

246 - if (!visit_type_str(v, name, &str_or_null->u.s, errp)) {

247 - return;

248 - }

255 + str_or_null->u.s = str;

256

257 qapi_free_StrOrNull(*ptr);

258 *ptr = str_or_null;