All of lore.kernel.org
 help / color / mirror / Atom feed
From: andrzej@ahunt.org
To: git@vger.kernel.org
Cc: andrzej@ahunt.org, phillip.wood123@gmail.com, newren@gmail.com
Subject: [PATCH v2 12/12] reset: clear_unpack_trees_porcelain to plug leak
Date: Sun, 25 Jul 2021 15:08:30 +0200	[thread overview]
Message-ID: <20210725130830.5145-13-andrzej@ahunt.org> (raw)
In-Reply-To: <20210725130830.5145-1-andrzej@ahunt.org>

From: Andrzej Hunt <ajrhunt@google.com>

setup_unpack_trees_porcelain() populates various fields on
unpack_tree_opts, we need to call clear_unpack_trees_porcelain() to
avoid leaking them. Specifically, we used to leak
unpack_tree_opts.msgs_to_free.

We have to do this in leave_reset_head because there are multiple
scenarios where unpack_tree_opts has already been configured, followed
by a 'goto leave_reset_head'. But we can also 'goto leave_reset_head'
prior to having initialised unpack_tree_opts via memset(..., 0, ...).
Therefore we also move unpack_tree_opts initialisation to the start of
reset_head(), and convert it to use brace initialisation - which
guarantees that we can never clear an uninitialised unpack_tree_opts.
clear_unpack_tree_opts() is always safe to call as long as
unpack_tree_opts is at least zero-initialised, i.e. it does not depend
on a previous call to setup_unpack_trees_porcelain().

LSAN output from t0021:

Direct leak of 192 byte(s) in 1 object(s) allocated from:
    #0 0x49ab49 in realloc ../projects/compiler-rt/lib/asan/asan_malloc_linux.cpp:164:3
    #1 0xa721e5 in xrealloc wrapper.c:126:8
    #2 0x9f7861 in strvec_push_nodup strvec.c:19:2
    #3 0x9f7861 in strvec_pushf strvec.c:39:2
    #4 0xa43e14 in setup_unpack_trees_porcelain unpack-trees.c:129:3
    #5 0x97e011 in reset_head reset.c:53:2
    #6 0x61dfa5 in cmd_rebase builtin/rebase.c:1991:9
    #7 0x4ce83e in run_builtin git.c:475:11
    #8 0x4ccafe in handle_builtin git.c:729:3
    #9 0x4cb01c in run_argv git.c:818:4
    #10 0x4cb01c in cmd_main git.c:949:19
    #11 0x6b3f3d in main common-main.c:52:11
    #12 0x7fa8addf3349 in __libc_start_main (/lib64/libc.so.6+0x24349)

Indirect leak of 147 byte(s) in 1 object(s) allocated from:
    #0 0x49ab49 in realloc ../projects/compiler-rt/lib/asan/asan_malloc_linux.cpp:164:3
    #1 0xa721e5 in xrealloc wrapper.c:126:8
    #2 0x9e8d54 in strbuf_grow strbuf.c:98:2
    #3 0x9e8d54 in strbuf_vaddf strbuf.c:401:3
    #4 0x9f7774 in strvec_pushf strvec.c:36:2
    #5 0xa43e14 in setup_unpack_trees_porcelain unpack-trees.c:129:3
    #6 0x97e011 in reset_head reset.c:53:2
    #7 0x61dfa5 in cmd_rebase builtin/rebase.c:1991:9
    #8 0x4ce83e in run_builtin git.c:475:11
    #9 0x4ccafe in handle_builtin git.c:729:3
    #10 0x4cb01c in run_argv git.c:818:4
    #11 0x4cb01c in cmd_main git.c:949:19
    #12 0x6b3f3d in main common-main.c:52:11
    #13 0x7fa8addf3349 in __libc_start_main (/lib64/libc.so.6+0x24349)

Indirect leak of 134 byte(s) in 1 object(s) allocated from:
    #0 0x49ab49 in realloc ../projects/compiler-rt/lib/asan/asan_malloc_linux.cpp:164:3
    #1 0xa721e5 in xrealloc wrapper.c:126:8
    #2 0x9e8d54 in strbuf_grow strbuf.c:98:2
    #3 0x9e8d54 in strbuf_vaddf strbuf.c:401:3
    #4 0x9f7774 in strvec_pushf strvec.c:36:2
    #5 0xa43fe4 in setup_unpack_trees_porcelain unpack-trees.c:168:3
    #6 0x97e011 in reset_head reset.c:53:2
    #7 0x61dfa5 in cmd_rebase builtin/rebase.c:1991:9
    #8 0x4ce83e in run_builtin git.c:475:11
    #9 0x4ccafe in handle_builtin git.c:729:3
    #10 0x4cb01c in run_argv git.c:818:4
    #11 0x4cb01c in cmd_main git.c:949:19
    #12 0x6b3f3d in main common-main.c:52:11
    #13 0x7fa8addf3349 in __libc_start_main (/lib64/libc.so.6+0x24349)

Indirect leak of 130 byte(s) in 1 object(s) allocated from:
    #0 0x49ab49 in realloc ../projects/compiler-rt/lib/asan/asan_malloc_linux.cpp:164:3
    #1 0xa721e5 in xrealloc wrapper.c:126:8
    #2 0x9e8d54 in strbuf_grow strbuf.c:98:2
    #3 0x9e8d54 in strbuf_vaddf strbuf.c:401:3
    #4 0x9f7774 in strvec_pushf strvec.c:36:2
    #5 0xa43f20 in setup_unpack_trees_porcelain unpack-trees.c:150:3
    #6 0x97e011 in reset_head reset.c:53:2
    #7 0x61dfa5 in cmd_rebase builtin/rebase.c:1991:9
    #8 0x4ce83e in run_builtin git.c:475:11
    #9 0x4ccafe in handle_builtin git.c:729:3
    #10 0x4cb01c in run_argv git.c:818:4
    #11 0x4cb01c in cmd_main git.c:949:19
    #12 0x6b3f3d in main common-main.c:52:11
    #13 0x7fa8addf3349 in __libc_start_main (/lib64/libc.so.6+0x24349)

SUMMARY: AddressSanitizer: 603 byte(s) leaked in 4 allocation(s).

Signed-off-by: Andrzej Hunt <andrzej@ahunt.org>
---
 reset.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/reset.c b/reset.c
index 4bea758053..79310ae071 100644
--- a/reset.c
+++ b/reset.c
@@ -21,7 +21,7 @@ int reset_head(struct repository *r, struct object_id *oid, const char *action,
 	struct object_id head_oid;
 	struct tree_desc desc[2] = { { NULL }, { NULL } };
 	struct lock_file lock = LOCK_INIT;
-	struct unpack_trees_options unpack_tree_opts;
+	struct unpack_trees_options unpack_tree_opts = { 0 };
 	struct tree *tree;
 	const char *reflog_action;
 	struct strbuf msg = STRBUF_INIT;
@@ -49,7 +49,6 @@ int reset_head(struct repository *r, struct object_id *oid, const char *action,
 	if (refs_only)
 		goto reset_head_refs;
 
-	memset(&unpack_tree_opts, 0, sizeof(unpack_tree_opts));
 	setup_unpack_trees_porcelain(&unpack_tree_opts, action);
 	unpack_tree_opts.head_idx = 1;
 	unpack_tree_opts.src_index = r->index;
@@ -134,6 +133,7 @@ int reset_head(struct repository *r, struct object_id *oid, const char *action,
 leave_reset_head:
 	strbuf_release(&msg);
 	rollback_lock_file(&lock);
+	clear_unpack_trees_porcelain(&unpack_tree_opts);
 	while (nr)
 		free((void *)desc[--nr].buffer);
 	return ret;
-- 
2.26.2


  parent reply	other threads:[~2021-07-25 13:10 UTC|newest]

Thread overview: 51+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-06-20 15:11 [PATCH 00/12] Fix all leaks in tests t0002-t0099: Part 2 andrzej
2021-06-20 15:11 ` [PATCH 01/12] fmt-merge-msg: free newly allocated temporary strings when done andrzej
2021-06-21 20:34   ` Elijah Newren
2021-06-20 15:11 ` [PATCH 02/12] environment: move strbuf into block to plug leak andrzej
2021-06-21 20:49   ` Elijah Newren
2021-06-26  8:27     ` René Scharfe
2021-06-20 15:11 ` [PATCH 03/12] builtin/submodule--helper: release unused strbuf to avoid leak andrzej
2021-06-20 15:11 ` [PATCH 04/12] builtin/for-each-repo: remove unnecessary argv copy to plug leak andrzej
2021-06-21 20:55   ` Elijah Newren
2021-06-20 15:11 ` [PATCH 05/12] diffcore-rename: move old_dir/new_dir definition " andrzej
2021-06-21 14:01   ` Elijah Newren
2021-06-20 15:11 ` [PATCH 06/12] ref-filter: also free head for ATOM_HEAD to avoid leak andrzej
2021-06-21 21:10   ` Elijah Newren
2021-06-20 15:11 ` [PATCH 07/12] read-cache: call diff_setup_done " andrzej
2021-06-21 21:17   ` Elijah Newren
2021-06-20 15:12 ` [PATCH 08/12] convert: release strbuf " andrzej
2021-06-21 20:31   ` Elijah Newren
2021-06-20 15:12 ` [PATCH 09/12] builtin/mv: free or UNLEAK multiple pointers at end of cmd_mv andrzej
2021-06-20 15:12 ` [PATCH 10/12] builtin/merge: free found_ref when done andrzej
2021-06-21 21:27   ` Elijah Newren
2021-06-20 15:12 ` [PATCH 11/12] builtin/rebase: fix options.strategy memory lifecycle andrzej
2021-06-20 18:14   ` Phillip Wood
2021-06-21 21:39     ` Elijah Newren
2021-06-22  9:02       ` Phillip Wood
2021-07-25 13:03         ` Andrzej Hunt
2021-07-27 19:34           ` Phillip Wood
2021-06-20 15:12 ` [PATCH 12/12] reset: clear_unpack_trees_porcelain to plug leak andrzej
2021-06-21 21:44   ` Elijah Newren
2021-06-21 21:54 ` [PATCH 00/12] Fix all leaks in tests t0002-t0099: Part 2 Elijah Newren
2021-07-25 13:05   ` Andrzej Hunt
2021-07-26  8:01   ` Christian Couder
2021-07-25 13:08 ` [PATCH v2 " andrzej
2021-07-25 13:08   ` [PATCH v2 01/12] fmt-merge-msg: free newly allocated temporary strings when done andrzej
2021-07-26 19:20     ` Junio C Hamano
2021-07-25 13:08   ` [PATCH v2 02/12] environment: move strbuf into block to plug leak andrzej
2021-07-25 13:08   ` [PATCH v2 03/12] builtin/submodule--helper: release unused strbuf to avoid leak andrzej
2021-07-25 13:08   ` [PATCH v2 04/12] builtin/for-each-repo: remove unnecessary argv copy to plug leak andrzej
2021-07-26 20:02     ` Junio C Hamano
2021-07-25 13:08   ` [PATCH v2 05/12] diffcore-rename: move old_dir/new_dir definition " andrzej
2021-07-26 20:02     ` Junio C Hamano
2021-07-25 13:08   ` [PATCH v2 06/12] ref-filter: also free head for ATOM_HEAD to avoid leak andrzej
2021-07-26 20:04     ` Junio C Hamano
2021-07-25 13:08   ` [PATCH v2 07/12] read-cache: call diff_setup_done " andrzej
2021-07-26 20:10     ` Junio C Hamano
2021-07-25 13:08   ` [PATCH v2 08/12] convert: release strbuf " andrzej
2021-07-26 20:15     ` Junio C Hamano
2021-07-25 13:08   ` [PATCH v2 09/12] builtin/mv: free or UNLEAK multiple pointers at end of cmd_mv andrzej
2021-07-25 13:08   ` [PATCH v2 10/12] builtin/merge: free found_ref when done andrzej
2021-07-25 13:08   ` [PATCH v2 11/12] builtin/rebase: fix options.strategy memory lifecycle andrzej
2021-07-25 13:08   ` andrzej [this message]
2021-07-26 20:20   ` [PATCH v2 00/12] Fix all leaks in tests t0002-t0099: Part 2 Junio C Hamano

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=20210725130830.5145-13-andrzej@ahunt.org \
    --to=andrzej@ahunt.org \
    --cc=git@vger.kernel.org \
    --cc=newren@gmail.com \
    --cc=phillip.wood123@gmail.com \
    /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.