From 895cd2d0982b44b885480cd396176f0d19abf9cb Mon Sep 17 00:00:00 2001 From: Jonathan Lebon Date: Mon, 28 Apr 2025 09:31:54 -0400 Subject: [PATCH 1/2] Don't call `g_variant_store()` with `NULL` A GVariant may require 0 bytes to serialize. In which case, `data_size` will be 0 and we'll call `g_malloc0 (0)` which returns `NULL`. The `data` argument of `g_variant_store` is declared as non-nullable, so we shouldn't pass it `NULL`. In older glib versions, this wasn't explicitly checked and it still transparently worked for our case because there was nothing to actually serialize. Starting from v2.84.1, a precondition was added: https://gitlab.gnome.org/GNOME/glib/-/commit/39c05b13123b12622ee2c93c170dbf20b573f6ac Let's respect the API here and stop calling the function if it's not needed. Note that `g_variant_new_from_data()` supports being passed `NULL` and 0 for the data and size arguments. --- modulemd/modulemd-util.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/modulemd/modulemd-util.c b/modulemd/modulemd-util.c index 99a5c53f..535519eb 100644 --- a/modulemd/modulemd-util.c +++ b/modulemd/modulemd-util.c @@ -363,9 +363,13 @@ modulemd_variant_deep_copy (GVariant *variant) { const GVariantType *data_type = g_variant_get_type (variant); gsize data_size = g_variant_get_size (variant); - gpointer data = g_malloc0 (data_size); + gpointer data = NULL; - g_variant_store (variant, data); + if (data_size > 0) + { + data = g_malloc0 (data_size); + g_variant_store (variant, data); + } return g_variant_ref_sink (g_variant_new_from_data ( data_type, data, data_size, FALSE, destroy_variant_data, data)); From 9a2c6fb3b327e8a2f51c9c01b2aa0a03d28f5416 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Petr=20P=C3=ADsa=C5=99?= Date: Tue, 29 Apr 2025 16:03:53 +0200 Subject: [PATCH 2/2] Test for "Don't call `g_variant_store()` with `NULL`" For https://github.com/fedora-modularity/libmodulemd/issues/623 --- modulemd/meson.build | 1 + .../tests/test-modulemd-variant_deep_copy.c | 69 +++++++++++++++++++ 2 files changed, 70 insertions(+) create mode 100644 modulemd/tests/test-modulemd-variant_deep_copy.c diff --git a/modulemd/meson.build b/modulemd/meson.build index e927d43c..d58ea8f1 100644 --- a/modulemd/meson.build +++ b/modulemd/meson.build @@ -365,6 +365,7 @@ c_tests = { 'service_level' : [ 'tests/test-modulemd-service-level.c' ], 'translation' : [ 'tests/test-modulemd-translation.c' ], 'translation_entry' : [ 'tests/test-modulemd-translation-entry.c' ], +'variant_deep_copy' : [ 'tests/test-modulemd-variant_deep_copy.c' ], 'obsoletes' : [ 'tests/test-modulemd-obsoletes.c' ], 'quoting' : [ 'tests/test-modulemd-quoting.c' ], } diff --git a/modulemd/tests/test-modulemd-variant_deep_copy.c b/modulemd/tests/test-modulemd-variant_deep_copy.c new file mode 100644 index 00000000..279136c0 --- /dev/null +++ b/modulemd/tests/test-modulemd-variant_deep_copy.c @@ -0,0 +1,69 @@ +/* + * This file is part of libmodulemd + * Copyright (C) 2025 Red Hat, Inc. + * + * Fedora-License-Identifier: MIT + * SPDX-2.0-License-Identifier: MIT + * SPDX-3.0-License-Identifier: MIT + * + * This program is free software. + * For more information on the license, see COPYING. + * For more information on free software, see . + */ + +#include +#include +#include +#include + +#include "private/modulemd-util.h" + +/* + * modulemd_variant_deep_copy() triggered a GLib critical warning + * from glib >= 2.84.1 when parsing a /data/xmd modulemd-stream-v2 element + * with {} value (an empty flow mapping). This test exhibits that code path + * and relies on G_DEBUG=fatal-criticals environment variable to crash the + * test. + * . + */ +static void +test_empty_a_sv (void) +{ + g_autoptr (GVariantDict) dictionary = + NULL; /* g_variant_dict_end() does not free */ + g_autoptr (GVariant) input = NULL; + g_autoptr (GVariant) output = NULL; + + /* Build a GVariant with an empty dictionary, results to an "a{sv}" of + * zero size. */ + dictionary = g_variant_dict_new (NULL); + input = g_variant_dict_end (dictionary); + + /* Exhibit the library. */ + output = modulemd_variant_deep_copy (input); + g_assert_true (output != NULL); + + /* Compare the content. */ + g_assert_true (g_variant_get_type (output) == g_variant_get_type (input)); + g_assert_true (g_variant_get_size (output) == g_variant_get_size (input)); +} + + +int +main (int argc, char *argv[]) +{ + setlocale (LC_ALL, ""); + g_test_init (&argc, &argv, NULL); + g_test_set_nonfatal_assertions (); + + if (!g_setenv ("G_DEBUG", "fatal-criticals", TRUE)) + { + g_fprintf (stderr, "Failed to set G_DEBUG environment variable.\n"); + exit (EXIT_FAILURE); + } + + g_test_add_func ("/modulemd/util/variant_deep_copy/empty_a{sv}", + test_empty_a_sv); + + return g_test_run (); +}