All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
To: git@vger.kernel.org
Cc: "Junio C Hamano" <gitster@pobox.com>,
	"Elijah Newren" <newren@gmail.com>,
	"Nguyễn Thái Ngọc Duy" <pclouds@gmail.com>,
	"Martin Ågren" <martin.agren@gmail.com>,
	"Andrzej Hunt" <ajrhunt@google.com>, "Jeff King" <peff@peff.net>,
	"Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
Subject: [PATCH 08/10] unpack-trees: don't leak memory in verify_clean_subdirectory()
Date: Mon,  4 Oct 2021 02:46:09 +0200	[thread overview]
Message-ID: <patch-08.10-32acc5e5c9e-20211004T002226Z-avarab@gmail.com> (raw)
In-Reply-To: <cover-00.10-00000000000-20211004T002226Z-avarab@gmail.com>

Fix two different but related memory leaks in
verify_clean_subdirectory(). We leaked both the "pathbuf" if
read_directory() returned non-zero, and we never cleaned up our own
"struct dir_struct" either.

 * "pathbuf": When the read_directory() call followed by the
   free(pathbuf) was added in c81935348be (Fix switching to a branch
   with D/F when current branch has file D., 2007-03-15) we didn't
   bother to free() before we called die().

   But when this code was later libified in 203a2fe1170 (Allow callers
   of unpack_trees() to handle failure, 2008-02-07) we started to leak
   as we returned data to the caller. This fixes that memory leak,
   which can be observed under SANITIZE=leak with e.g. the
   "t1001-read-tree-m-2way.sh" test.

 * "struct dir_struct": We've leaked the dir_struct ever since this
   code was added back in c81935348be.

   When that commit was written there wasn't an equivalent of
   dir_clear(). Since it was added in 270be816049 (dir.c: provide
   clear_directory() for reclaiming dir_struct memory, 2013-01-06)
   we've omitted freeing the memory allocated here.

   This memory leak could also be observed under SANITIZE=leak and the
   "t1001-read-tree-m-2way.sh" test.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 unpack-trees.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/unpack-trees.c b/unpack-trees.c
index bea598c9ece..260e7ec5bb4 100644
--- a/unpack-trees.c
+++ b/unpack-trees.c
@@ -2138,9 +2138,10 @@ static int verify_clean_subdirectory(const struct cache_entry *ce,
 
 	d.exclude_per_dir = o->dir.exclude_per_dir;
 	i = read_directory(&d, o->src_index, pathbuf, namelen+1, NULL);
+	dir_clear(&d);
+	free(pathbuf);
 	if (i)
 		return add_rejected_path(o, ERROR_NOT_UPTODATE_DIR, ce->name);
-	free(pathbuf);
 	return cnt;
 }
 
-- 
2.33.0.1404.g83021034c5d


  parent reply	other threads:[~2021-10-04  0:47 UTC|newest]

Thread overview: 31+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-10-04  0:46 [PATCH 00/10] unpack-trees & dir APIs: fix memory leaks Ævar Arnfjörð Bjarmason
2021-10-04  0:46 ` [PATCH 01/10] unpack-trees.[ch]: define and use a UNPACK_TREES_OPTIONS_INIT Ævar Arnfjörð Bjarmason
2021-10-04  0:46 ` [PATCH 02/10] merge-recursive.c: call a new unpack_trees_options_init() function Ævar Arnfjörð Bjarmason
2021-10-04 13:45   ` Elijah Newren
2021-10-04 14:41     ` Ævar Arnfjörð Bjarmason
2021-10-04 15:04       ` Elijah Newren
2021-10-04  0:46 ` [PATCH 03/10] unpack-trees.[ch]: embed "dir" in "struct unpack_trees_options" Ævar Arnfjörð Bjarmason
2021-10-04 13:45   ` Elijah Newren
2021-10-04  0:46 ` [PATCH 04/10] unpack-trees API: don't have clear_unpack_trees_porcelain() reset Ævar Arnfjörð Bjarmason
2021-10-04  9:31   ` Phillip Wood
2021-10-04 11:12     ` Ævar Arnfjörð Bjarmason
2021-10-04 13:45   ` Elijah Newren
2021-10-04 15:20     ` Ævar Arnfjörð Bjarmason
2021-10-04 16:28       ` Elijah Newren
2021-10-04  0:46 ` [PATCH 05/10] dir.[ch]: make DIR_INIT mandatory Ævar Arnfjörð Bjarmason
2021-10-04 13:45   ` Elijah Newren
2021-10-04  0:46 ` [PATCH 06/10] dir.c: get rid of lazy initialization Ævar Arnfjörð Bjarmason
2021-10-04 13:45   ` Elijah Newren
2021-10-04  0:46 ` [PATCH 07/10] unpack-trees API: rename clear_unpack_trees_porcelain() Ævar Arnfjörð Bjarmason
2021-10-04  9:38   ` Phillip Wood
2021-10-04 11:10     ` Ævar Arnfjörð Bjarmason
2021-10-04 13:45   ` Elijah Newren
2021-10-04  0:46 ` Ævar Arnfjörð Bjarmason [this message]
2021-10-04 13:45   ` [PATCH 08/10] unpack-trees: don't leak memory in verify_clean_subdirectory() Elijah Newren
2021-10-04  0:46 ` [PATCH 09/10] merge.c: avoid duplicate unpack_trees_options_release() code Ævar Arnfjörð Bjarmason
2021-10-04 13:45   ` Elijah Newren
2021-10-04 14:50     ` Ævar Arnfjörð Bjarmason
2021-10-04  0:46 ` [PATCH 10/10] built-ins: plug memory leaks with unpack_trees_options_release() Ævar Arnfjörð Bjarmason
2021-10-04 13:45   ` Elijah Newren
2021-10-04 14:54     ` Ævar Arnfjörð Bjarmason
2021-10-04 13:45 ` [PATCH 00/10] unpack-trees & dir APIs: fix memory leaks Elijah Newren

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=patch-08.10-32acc5e5c9e-20211004T002226Z-avarab@gmail.com \
    --to=avarab@gmail.com \
    --cc=ajrhunt@google.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=martin.agren@gmail.com \
    --cc=newren@gmail.com \
    --cc=pclouds@gmail.com \
    --cc=peff@peff.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.