git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: andrzej@ahunt.org
Cc: git@vger.kernel.org, phillip.wood123@gmail.com, newren@gmail.com
Subject: Re: [PATCH v2 08/12] convert: release strbuf to avoid leak
Date: Mon, 26 Jul 2021 13:15:19 -0700	[thread overview]
Message-ID: <xmqq5ywwj0h4.fsf@gitster.g> (raw)
In-Reply-To: <20210725130830.5145-9-andrzej@ahunt.org> (andrzej@ahunt.org's message of "Sun, 25 Jul 2021 15:08:26 +0200")

andrzej@ahunt.org writes:

> From: Andrzej Hunt <ajrhunt@google.com>
>
> apply_multi_file_filter and async_query_available_blobs both query
> subprocess output using subprocess_read_status, which writes data into
> the identically named filter_status strbuf. We add a strbuf_release to
> avoid leaking their contents.
>
> Leak output seen when running t0021 with LSAN:

Obviously correct.  Thanks.

>
> Direct leak of 24 byte(s) in 1 object(s) allocated from:
>     #0 0x49ab49 in realloc ../projects/compiler-rt/lib/asan/asan_malloc_linux.cpp:164:3
>     #1 0xa8c2b5 in xrealloc wrapper.c:126:8
>     #2 0x9ff99d in strbuf_grow strbuf.c:98:2
>     #3 0x9ff99d in strbuf_addbuf strbuf.c:304:2
>     #4 0xa101d6 in subprocess_read_status sub-process.c:45:5
>     #5 0x77793c in apply_multi_file_filter convert.c:886:8
>     #6 0x77793c in apply_filter convert.c:1042:10
>     #7 0x77a0b5 in convert_to_git_filter_fd convert.c:1492:7
>     #8 0x8b48cd in index_stream_convert_blob object-file.c:2156:2
>     #9 0x8b48cd in index_fd object-file.c:2248:9
>     #10 0x597411 in hash_fd builtin/hash-object.c:43:9
>     #11 0x596be1 in hash_object builtin/hash-object.c:59:2
>     #12 0x596be1 in cmd_hash_object builtin/hash-object.c:153:3
>     #13 0x4ce83e in run_builtin git.c:475:11
>     #14 0x4ccafe in handle_builtin git.c:729:3
>     #15 0x4cb01c in run_argv git.c:818:4
>     #16 0x4cb01c in cmd_main git.c:949:19
>     #17 0x6bdc2d in main common-main.c:52:11
>     #18 0x7f42acf79349 in __libc_start_main (/lib64/libc.so.6+0x24349)
>
> SUMMARY: AddressSanitizer: 24 byte(s) leaked in 1 allocation(s).
>
> Direct leak of 120 byte(s) in 5 object(s) allocated from:
>     #0 0x49ab49 in realloc ../projects/compiler-rt/lib/asan/asan_malloc_linux.cpp:164:3
>     #1 0xa8c295 in xrealloc wrapper.c:126:8
>     #2 0x9ff97d in strbuf_grow strbuf.c:98:2
>     #3 0x9ff97d in strbuf_addbuf strbuf.c:304:2
>     #4 0xa101b6 in subprocess_read_status sub-process.c:45:5
>     #5 0x775c73 in async_query_available_blobs convert.c:960:8
>     #6 0x80029d in finish_delayed_checkout entry.c:183:9
>     #7 0xa65d1e in check_updates unpack-trees.c:493:10
>     #8 0xa5f469 in unpack_trees unpack-trees.c:1747:8
>     #9 0x525971 in checkout builtin/clone.c:815:6
>     #10 0x525971 in cmd_clone builtin/clone.c:1409:8
>     #11 0x4ce83e in run_builtin git.c:475:11
>     #12 0x4ccafe in handle_builtin git.c:729:3
>     #13 0x4cb01c in run_argv git.c:818:4
>     #14 0x4cb01c in cmd_main git.c:949:19
>     #15 0x6bdc2d in main common-main.c:52:11
>     #16 0x7fa253fce349 in __libc_start_main (/lib64/libc.so.6+0x24349)
>
> SUMMARY: AddressSanitizer: 120 byte(s) leaked in 5 allocation(s).
>
> Signed-off-by: Andrzej Hunt <andrzej@ahunt.org>
> ---
>  convert.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/convert.c b/convert.c
> index fd9c84b025..0d6fb3410a 100644
> --- a/convert.c
> +++ b/convert.c
> @@ -916,6 +916,7 @@ static int apply_multi_file_filter(const char *path, const char *src, size_t len
>  	else
>  		strbuf_swap(dst, &nbuf);
>  	strbuf_release(&nbuf);
> +	strbuf_release(&filter_status);
>  	return !err;
>  }
>  
> @@ -966,6 +967,7 @@ int async_query_available_blobs(const char *cmd, struct string_list *available_p
>  
>  	if (err)
>  		handle_filter_error(&filter_status, entry, 0);
> +	strbuf_release(&filter_status);
>  	return !err;
>  }

  reply	other threads:[~2021-07-26 20:15 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 [this message]
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   ` [PATCH v2 12/12] reset: clear_unpack_trees_porcelain to plug leak andrzej
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=xmqq5ywwj0h4.fsf@gitster.g \
    --to=gitster@pobox.com \
    --cc=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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).