From patchwork Tue Jun 27 16:09:25 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Alex_Benn=C3=A9e?= X-Patchwork-Id: 1800696 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@legolas.ozlabs.org Authentication-Results: legolas.ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=nongnu.org (client-ip=209.51.188.17; helo=lists.gnu.org; envelope-from=qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org; receiver=) Authentication-Results: legolas.ozlabs.org; dkim=pass (2048-bit key; unprotected) header.d=linaro.org header.i=@linaro.org header.a=rsa-sha256 header.s=google header.b=pCejVcqP; dkim-atps=neutral Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-ECDSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by legolas.ozlabs.org (Postfix) with ESMTPS id 4Qr8wp3gjvz20ZQ for ; Wed, 28 Jun 2023 02:16:42 +1000 (AEST) Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1qEBMR-0001Ku-Nb; Tue, 27 Jun 2023 12:16:13 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1qEBMK-00017Z-OC for qemu-devel@nongnu.org; Tue, 27 Jun 2023 12:16:04 -0400 Received: from mail-wr1-x42d.google.com ([2a00:1450:4864:20::42d]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1qEBMB-00089v-Oa for qemu-devel@nongnu.org; Tue, 27 Jun 2023 12:16:04 -0400 Received: by mail-wr1-x42d.google.com with SMTP id ffacd0b85a97d-313e09a5b19so23736f8f.0 for ; Tue, 27 Jun 2023 09:15:55 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; t=1687882554; x=1690474554; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=XTGDyBwpOUnJ81QARU+1/OI2J0gTxsiGK9l42g4HfaQ=; b=pCejVcqPGQxvYe8n4oFODd6R3PVt1ZZO2lw77skvQoRyC14sfdGyUjWuy3QGGrZmf7 sVx6kNDl8KpPiE2hfEQSETYRXOieevhsUCDHJERYlqsgXj+OZ39XwVA9cHyD7c1pSUKw 6cYhOu8hbeh56m6Bqgojq8d4NusoOnvC0E07pDbgjOGGt4N7swRppKj3yx/NA1FZtflh KHS2szQhKmSmDZRzzobkz+NaH43CusyC3tJZXos2RDqgkQ+S3uj6VU5qr3Y7bAq0Y8T8 30fgQxKTNxWe2F2n7SR+7KVP7F1qUY0LLPvVRN7fmNTgeBn846orY3Ej0JkdnnaRRqSK dhXg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1687882554; x=1690474554; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=XTGDyBwpOUnJ81QARU+1/OI2J0gTxsiGK9l42g4HfaQ=; b=ikzPtnfT6TXnNyre+JHpT/2x38mwRrDTr73bk2UyofhwgAHea+s+DZ2rDqDdVVFNZ3 AmktsZOgs+VEX5wiG5ymncTdVesAsN4Esp8s/c8+QfUOsNVgTmGlIm+cJKItqCYUmR8B QyWLX9LPxqHKdtZ1deFYu7zOtFcu8a1DRCOMGugwOn/iZUv24yXViRJj3bzo7FYnMV/k 2Ew5Jd2wjMTT+2Y5INug8wSNHhLZZmf1g6WM0VreebNRtA73GZwr75fuFmMJWOPWv60v u4EO7AjQpzjJPF63I3I5WKpIIL9GxEWDV+JdIVUC+NMAXcQVXngnyDpCpRtNNjC5LMTt i/OA== X-Gm-Message-State: AC+VfDx8pzkWvvgLL3BOckuTwEhSxhPFOkFQQecUQEmFHYfjXZ7GIl8G lP4rJC3KpJWo3w0ZuUtkT7/7ug== X-Google-Smtp-Source: ACHHUZ5MDo42ueo1GNps3I0eEt8E00poCRNwesNK3GkO5HQH6CQFy062xP3k0wstSVjCq+BmTAu+Bw== X-Received: by 2002:a05:6000:1206:b0:313:f977:479f with SMTP id e6-20020a056000120600b00313f977479fmr3352329wrx.20.1687882554217; Tue, 27 Jun 2023 09:15:54 -0700 (PDT) Received: from zen.linaroharston ([85.9.250.243]) by smtp.gmail.com with ESMTPSA id c1-20020a056000104100b0031402a910edsm1501373wrx.86.2023.06.27.09.15.51 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 27 Jun 2023 09:15:53 -0700 (PDT) Received: from zen.lan (localhost [127.0.0.1]) by zen.linaroharston (Postfix) with ESMTP id 2026A1FFCB; Tue, 27 Jun 2023 17:09:46 +0100 (BST) From: =?utf-8?q?Alex_Benn=C3=A9e?= To: qemu-devel@nongnu.org Cc: Wainer dos Santos Moschetta , Juan Quintela , Thomas Huth , Cleber Rosa , Leonardo Bras , Beraldo Leal , Peter Maydell , Bin Meng , Yanan Wang , Darren Kenny , Alexander Bulekov , Marcel Apfelbaum , Peter Xu , Radoslaw Biernacki , Laurent Vivier , Paolo Bonzini , Eduardo Habkost , qemu-arm@nongnu.org, Stefan Hajnoczi , Richard Henderson , Bandan Das , =?utf-8?q?Philippe_Mathieu-Daud=C3=A9?= , =?utf-8?q?Dani?= =?utf-8?q?el_P=2E_Berrang=C3=A9?= , Alexandre Iooss , Marcin Juszkiewicz , Leif Lindholm , Laurent Vivier , Qiuhao Li , Mahmoud Mandour , Riku Voipio , =?utf-8?q?Alex_Benn=C3=A9e?= Subject: [PATCH v3 18/36] plugins: fix memory leak while parsing options Date: Tue, 27 Jun 2023 17:09:25 +0100 Message-Id: <20230627160943.2956928-19-alex.bennee@linaro.org> X-Mailer: git-send-email 2.39.2 In-Reply-To: <20230627160943.2956928-1-alex.bennee@linaro.org> References: <20230627160943.2956928-1-alex.bennee@linaro.org> MIME-Version: 1.0 Received-SPF: pass client-ip=2a00:1450:4864:20::42d; envelope-from=alex.bennee@linaro.org; helo=mail-wr1-x42d.google.com X-Spam_score_int: -20 X-Spam_score: -2.1 X-Spam_bar: -- X-Spam_report: (-2.1 / 5.0 requ) BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, RCVD_IN_DNSWL_NONE=-0.0001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001, T_SCC_BODY_TEXT_LINE=-0.01 autolearn=unavailable autolearn_force=no X-Spam_action: no action X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org It was hard to track down this leak as it was an internal allocation by glib and the backtraces did not give much away. The autofree was freeing the allocation with g_free() but not taking care of the individual strings. They should have been freed with g_strfreev() instead. Searching the glib source code for the correct string free function led to: G_DEFINE_AUTO_CLEANUP_FREE_FUNC(GStrv, g_strfreev, NULL) and indeed if you read to the bottom of the documentation page you will find: typedef gchar** GStrv; A typedef alias for gchar**. This is mostly useful when used together with g_auto(). So fix up all the g_autofree g_strsplit case that smugly thought they had de-allocation covered. Reviewed-by: Richard Henderson Message-Id: <20230623122100.1640995-20-alex.bennee@linaro.org> Reviewed-by: Philippe Mathieu-Daudé Signed-off-by: Alex Bennée --- contrib/plugins/cache.c | 2 +- contrib/plugins/drcov.c | 2 +- contrib/plugins/execlog.c | 2 +- contrib/plugins/hotblocks.c | 2 +- contrib/plugins/hotpages.c | 2 +- contrib/plugins/howvec.c | 2 +- contrib/plugins/hwprofile.c | 2 +- contrib/plugins/lockstep.c | 2 +- tests/plugin/bb.c | 2 +- tests/plugin/insn.c | 2 +- tests/plugin/mem.c | 2 +- tests/plugin/syscall.c | 2 +- 12 files changed, 12 insertions(+), 12 deletions(-) diff --git a/contrib/plugins/cache.c b/contrib/plugins/cache.c index 2e25184a7f..5036213f1b 100644 --- a/contrib/plugins/cache.c +++ b/contrib/plugins/cache.c @@ -772,7 +772,7 @@ int qemu_plugin_install(qemu_plugin_id_t id, const qemu_info_t *info, for (i = 0; i < argc; i++) { char *opt = argv[i]; - g_autofree char **tokens = g_strsplit(opt, "=", 2); + g_auto(GStrv) tokens = g_strsplit(opt, "=", 2); if (g_strcmp0(tokens[0], "iblksize") == 0) { l1_iblksize = STRTOLL(tokens[1]); diff --git a/contrib/plugins/drcov.c b/contrib/plugins/drcov.c index b4a855adaf..686ae0a537 100644 --- a/contrib/plugins/drcov.c +++ b/contrib/plugins/drcov.c @@ -148,7 +148,7 @@ int qemu_plugin_install(qemu_plugin_id_t id, const qemu_info_t *info, int argc, char **argv) { for (int i = 0; i < argc; i++) { - g_autofree char **tokens = g_strsplit(argv[i], "=", 2); + g_auto(GStrv) tokens = g_strsplit(argv[i], "=", 2); if (g_strcmp0(tokens[0], "filename") == 0) { file_name = g_strdup(tokens[1]); } diff --git a/contrib/plugins/execlog.c b/contrib/plugins/execlog.c index e255bd21fd..7129d526f8 100644 --- a/contrib/plugins/execlog.c +++ b/contrib/plugins/execlog.c @@ -227,7 +227,7 @@ QEMU_PLUGIN_EXPORT int qemu_plugin_install(qemu_plugin_id_t id, for (int i = 0; i < argc; i++) { char *opt = argv[i]; - g_autofree char **tokens = g_strsplit(opt, "=", 2); + g_auto(GStrv) tokens = g_strsplit(opt, "=", 2); if (g_strcmp0(tokens[0], "ifilter") == 0) { parse_insn_match(tokens[1]); } else if (g_strcmp0(tokens[0], "afilter") == 0) { diff --git a/contrib/plugins/hotblocks.c b/contrib/plugins/hotblocks.c index 062200a7a4..6b74d25fea 100644 --- a/contrib/plugins/hotblocks.c +++ b/contrib/plugins/hotblocks.c @@ -135,7 +135,7 @@ int qemu_plugin_install(qemu_plugin_id_t id, const qemu_info_t *info, { for (int i = 0; i < argc; i++) { char *opt = argv[i]; - g_autofree char **tokens = g_strsplit(opt, "=", 2); + g_auto(GStrv) tokens = g_strsplit(opt, "=", 2); if (g_strcmp0(tokens[0], "inline") == 0) { if (!qemu_plugin_bool_parse(tokens[0], tokens[1], &do_inline)) { fprintf(stderr, "boolean argument parsing failed: %s\n", opt); diff --git a/contrib/plugins/hotpages.c b/contrib/plugins/hotpages.c index 0d12910af6..8316ae50c7 100644 --- a/contrib/plugins/hotpages.c +++ b/contrib/plugins/hotpages.c @@ -169,7 +169,7 @@ int qemu_plugin_install(qemu_plugin_id_t id, const qemu_info_t *info, for (i = 0; i < argc; i++) { char *opt = argv[i]; - g_autofree char **tokens = g_strsplit(opt, "=", -1); + g_auto(GStrv) tokens = g_strsplit(opt, "=", -1); if (g_strcmp0(tokens[0], "sortby") == 0) { if (g_strcmp0(tokens[1], "reads") == 0) { diff --git a/contrib/plugins/howvec.c b/contrib/plugins/howvec.c index 4a5ec3d936..0ed01ea931 100644 --- a/contrib/plugins/howvec.c +++ b/contrib/plugins/howvec.c @@ -333,7 +333,7 @@ QEMU_PLUGIN_EXPORT int qemu_plugin_install(qemu_plugin_id_t id, for (i = 0; i < argc; i++) { char *p = argv[i]; - g_autofree char **tokens = g_strsplit(p, "=", -1); + g_auto(GStrv) tokens = g_strsplit(p, "=", -1); if (g_strcmp0(tokens[0], "inline") == 0) { if (!qemu_plugin_bool_parse(tokens[0], tokens[1], &do_inline)) { fprintf(stderr, "boolean argument parsing failed: %s\n", p); diff --git a/contrib/plugins/hwprofile.c b/contrib/plugins/hwprofile.c index 691d4edb0c..739ac0c66b 100644 --- a/contrib/plugins/hwprofile.c +++ b/contrib/plugins/hwprofile.c @@ -263,7 +263,7 @@ int qemu_plugin_install(qemu_plugin_id_t id, const qemu_info_t *info, for (i = 0; i < argc; i++) { char *opt = argv[i]; - g_autofree char **tokens = g_strsplit(opt, "=", 2); + g_auto(GStrv) tokens = g_strsplit(opt, "=", 2); if (g_strcmp0(tokens[0], "track") == 0) { if (g_strcmp0(tokens[1], "read") == 0) { diff --git a/contrib/plugins/lockstep.c b/contrib/plugins/lockstep.c index a41ffe83fa..e36f0b9562 100644 --- a/contrib/plugins/lockstep.c +++ b/contrib/plugins/lockstep.c @@ -323,7 +323,7 @@ QEMU_PLUGIN_EXPORT int qemu_plugin_install(qemu_plugin_id_t id, for (i = 0; i < argc; i++) { char *p = argv[i]; - g_autofree char **tokens = g_strsplit(p, "=", 2); + g_auto(GStrv) tokens = g_strsplit(p, "=", 2); if (g_strcmp0(tokens[0], "verbose") == 0) { if (!qemu_plugin_bool_parse(tokens[0], tokens[1], &verbose)) { diff --git a/tests/plugin/bb.c b/tests/plugin/bb.c index 7d470a1011..df50d1fd3b 100644 --- a/tests/plugin/bb.c +++ b/tests/plugin/bb.c @@ -104,7 +104,7 @@ QEMU_PLUGIN_EXPORT int qemu_plugin_install(qemu_plugin_id_t id, for (i = 0; i < argc; i++) { char *opt = argv[i]; - g_autofree char **tokens = g_strsplit(opt, "=", 2); + g_auto(GStrv) tokens = g_strsplit(opt, "=", 2); if (g_strcmp0(tokens[0], "inline") == 0) { if (!qemu_plugin_bool_parse(tokens[0], tokens[1], &do_inline)) { fprintf(stderr, "boolean argument parsing failed: %s\n", opt); diff --git a/tests/plugin/insn.c b/tests/plugin/insn.c index 9bd6e44f73..5fd3017c2b 100644 --- a/tests/plugin/insn.c +++ b/tests/plugin/insn.c @@ -189,7 +189,7 @@ QEMU_PLUGIN_EXPORT int qemu_plugin_install(qemu_plugin_id_t id, { for (int i = 0; i < argc; i++) { char *opt = argv[i]; - g_autofree char **tokens = g_strsplit(opt, "=", 2); + g_auto(GStrv) tokens = g_strsplit(opt, "=", 2); if (g_strcmp0(tokens[0], "inline") == 0) { if (!qemu_plugin_bool_parse(tokens[0], tokens[1], &do_inline)) { fprintf(stderr, "boolean argument parsing failed: %s\n", opt); diff --git a/tests/plugin/mem.c b/tests/plugin/mem.c index 4570f7d815..f3b9f696a0 100644 --- a/tests/plugin/mem.c +++ b/tests/plugin/mem.c @@ -83,7 +83,7 @@ QEMU_PLUGIN_EXPORT int qemu_plugin_install(qemu_plugin_id_t id, for (int i = 0; i < argc; i++) { char *opt = argv[i]; - g_autofree char **tokens = g_strsplit(opt, "=", 2); + g_auto(GStrv) tokens = g_strsplit(opt, "=", 2); if (g_strcmp0(tokens[0], "haddr") == 0) { if (!qemu_plugin_bool_parse(tokens[0], tokens[1], &do_haddr)) { diff --git a/tests/plugin/syscall.c b/tests/plugin/syscall.c index 96040c578f..72e1a5bf90 100644 --- a/tests/plugin/syscall.c +++ b/tests/plugin/syscall.c @@ -121,7 +121,7 @@ QEMU_PLUGIN_EXPORT int qemu_plugin_install(qemu_plugin_id_t id, for (int i = 0; i < argc; i++) { char *opt = argv[i]; - g_autofree char **tokens = g_strsplit(opt, "=", 2); + g_auto(GStrv) tokens = g_strsplit(opt, "=", 2); if (g_strcmp0(tokens[0], "print") == 0) { if (!qemu_plugin_bool_parse(tokens[0], tokens[1], &do_print)) {