From patchwork Wed Dec 1 12:27:23 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Mark Wielaard X-Patchwork-Id: 48352 Return-Path: X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 034513858006 for ; Wed, 1 Dec 2021 12:27:33 +0000 (GMT) X-Original-To: elfutils-devel@sourceware.org Delivered-To: elfutils-devel@sourceware.org Received: from gnu.wildebeest.org (gnu.wildebeest.org [45.83.234.184]) by sourceware.org (Postfix) with ESMTPS id 36C933858401 for ; Wed, 1 Dec 2021 12:27:27 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 36C933858401 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=klomp.org Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=klomp.org Received: from tarox.wildebeest.org (83-87-18-245.cable.dynamic.v4.ziggo.nl [83.87.18.245]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by gnu.wildebeest.org (Postfix) with ESMTPSA id E5938300070C; Wed, 1 Dec 2021 13:27:25 +0100 (CET) Received: by tarox.wildebeest.org (Postfix, from userid 1000) id 9687A425A456; Wed, 1 Dec 2021 13:27:25 +0100 (CET) From: Mark Wielaard To: elfutils-devel@sourceware.org Subject: [PATCH] debuginfod: Fix some memory leaks on debuginfod-client error paths. Date: Wed, 1 Dec 2021 13:27:23 +0100 Message-Id: <20211201122723.3767-1-mark@klomp.org> X-Mailer: git-send-email 2.18.4 X-Spam-Status: No, score=-10.1 required=5.0 tests=BAYES_00, GIT_PATCH_0, KAM_DMARC_STATUS, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on server2.sourceware.org X-BeenThere: elfutils-devel@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Elfutils-devel mailing list List-Unsubscribe: , List-Archive: List-Help: List-Subscribe: , Cc: Mark Wielaard Errors-To: elfutils-devel-bounces+patchwork=sourceware.org@sourceware.org Sender: "Elfutils-devel" In a couple of places we might leak some memory when we encounter an error. tmp_url might leak if realloc failed. escaped_string might leak when setting up the data handle fails and we don't use it. And one of the goto out1 should have been goto out2 to make sure we release all allocated resources on exit (also updated a wrong comment about that). Signed-off-by: Mark Wielaard --- debuginfod/ChangeLog | 7 +++++++ debuginfod/debuginfod-client.c | 16 +++++++++++----- 2 files changed, 18 insertions(+), 5 deletions(-) diff --git a/debuginfod/ChangeLog b/debuginfod/ChangeLog index 7054fdad..ae584b9b 100644 --- a/debuginfod/ChangeLog +++ b/debuginfod/ChangeLog @@ -1,3 +1,10 @@ +2021-12-01 Mark Wielaard + + * debuginfod-client.c (debuginfod_query_server): Free tmp_url on + realloc error. curl_free escaped_string on error. Fix error out + goto on curl_easy_init failure. Only cleanup data[i] handle and + response_data if it was initialized. + 2021-12-01 Mark Wielaard * debuginfod.cxx (reportable_exception::mhd_send_response): Check diff --git a/debuginfod/debuginfod-client.c b/debuginfod/debuginfod-client.c index c875ee62..75d3e301 100644 --- a/debuginfod/debuginfod-client.c +++ b/debuginfod/debuginfod-client.c @@ -1,5 +1,6 @@ /* Retrieve ELF / DWARF / source files from the debuginfod. Copyright (C) 2019-2021 Red Hat, Inc. + Copyright (C) 2021 Mark J. Wielaard This file is part of elfutils. This file is free software; you can redistribute it and/or modify @@ -882,6 +883,7 @@ debuginfod_query_server (debuginfod_client *c, sizeof(char*)); if (realloc_ptr == NULL) { + free (tmp_url); rc = -ENOMEM; goto out1; } @@ -909,7 +911,7 @@ debuginfod_query_server (debuginfod_client *c, goto out1; } - /* thereafter, goto out1 on error. */ + /* thereafter, goto out2 on error. */ /*The beginning of goto block query_in_parallel.*/ query_in_parallel: @@ -962,8 +964,9 @@ debuginfod_query_server (debuginfod_client *c, data[i].handle = curl_easy_init(); if (data[i].handle == NULL) { + if (filename) curl_free (escaped_string); rc = -ENETUNREACH; - goto out1; + goto out2; } data[i].client = c; @@ -1384,9 +1387,12 @@ debuginfod_query_server (debuginfod_client *c, /* remove all handles from multi */ for (int i = 0; i < num_urls; i++) { - curl_multi_remove_handle(curlm, data[i].handle); /* ok to repeat */ - curl_easy_cleanup (data[i].handle); - free (data[i].response_data); + if (data[i].handle != NULL) + { + curl_multi_remove_handle(curlm, data[i].handle); /* ok to repeat */ + curl_easy_cleanup (data[i].handle); + free (data[i].response_data); + } } unlink (target_cache_tmppath);