From: Abhishek Kumar <abhishekkumar8222@gmail.com>
To: gitgitgadget@gmail.com
Cc: ajrhunt@google.com, andrzej@ahunt.org, git@vger.kernel.org
Subject: Re: [PATCH] commit-graph: avoid leaking topo_levels slab in write_commit_graph()
Date: Thu, 25 Feb 2021 11:56:16 +0530 [thread overview]
Message-ID: <YDdDCMMf4Csumeci@Abhishek-Arch> (raw)
"Andrzej Hunt via GitGitGadget" <gitgitgadget@gmail.com> writes:
> From: Andrzej Hunt <ajrhunt@google.com>
>
> write_commit_graph initialises topo_levels using init_topo_level_slab(),
> next it calls compute_topological_levels() which can cause the slab to
> grow, we therefore need to clear the slab again using
> clear_topo_level_slab() when we're done.
>
> First introduced in 72a2bfcaf01860ce8dd6921490d903dc0ad59c89 - which
> is currently only in master and not on maint.
>
Thanks for identifying and fixing this memory leak!
- Abhishek
> LeakSanitizer output:
> ==1026==ERROR: LeakSanitizer: detected memory leaks
> Direct leak of 8 byte(s) in 1 object(s) allocated from:
> #0 0x498ae9 in realloc /src/llvm-project/compiler-rt/lib/asan/asan_malloc_linux.cpp:164:3
> #1 0xafbed8 in xrealloc /src/git/wrapper.c:126:8
> #2 0x7966d1 in topo_level_slab_at_peek /src/git/commit-graph.c:71:1
> #3 0x7965e0 in topo_level_slab_at /src/git/commit-graph.c:71:1
> #4 0x78fbf5 in compute_topological_levels /src/git/commit-graph.c:1472:12
> #5 0x78c5c3 in write_commit_graph /src/git/commit-graph.c:2456:2
> #6 0x535c5f in graph_write /src/git/builtin/commit-graph.c:299:6
> #7 0x5350ca in cmd_commit_graph /src/git/builtin/commit-graph.c:337:11
> #8 0x4cddb1 in run_builtin /src/git/git.c:453:11
> #9 0x4cabe2 in handle_builtin /src/git/git.c:704:3
> #10 0x4cd084 in run_argv /src/git/git.c:771:4
> #11 0x4ca424 in cmd_main /src/git/git.c:902:19
> #12 0x707fb6 in main /src/git/common-main.c:52:11
> #13 0x7fee4249383f in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2083f)
> Indirect leak of 524256 byte(s) in 1 object(s) allocated from:
> #0 0x498942 in calloc /src/llvm-project/compiler-rt/lib/asan/asan_malloc_linux.cpp:154:3
> #1 0xafc088 in xcalloc /src/git/wrapper.c:140:8
> #2 0x796870 in topo_level_slab_at_peek /src/git/commit-graph.c:71:1
> #3 0x7965e0 in topo_level_slab_at /src/git/commit-graph.c:71:1
> #4 0x78fbf5 in compute_topological_levels /src/git/commit-graph.c:1472:12
> #5 0x78c5c3 in write_commit_graph /src/git/commit-graph.c:2456:2
> #6 0x535c5f in graph_write /src/git/builtin/commit-graph.c:299:6
> #7 0x5350ca in cmd_commit_graph /src/git/builtin/commit-graph.c:337:11
> #8 0x4cddb1 in run_builtin /src/git/git.c:453:11
> #9 0x4cabe2 in handle_builtin /src/git/git.c:704:3
> #10 0x4cd084 in run_argv /src/git/git.c:771:4
> #11 0x4ca424 in cmd_main /src/git/git.c:902:19
> #12 0x707fb6 in main /src/git/common-main.c:52:11
> #13 0x7fee4249383f in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2083f)
> SUMMARY: AddressSanitizer: 524264 byte(s) leaked in 2 allocation(s).
> Signed-off-by: Andrzej Hunt <ajrhunt@google.com>
> ---
> commit-graph: avoid leaking topo_levels slab in write_commit_graph()
>
> write_commit_graph initialises topo_levels using init_topo_level_slab(),
> next it calls compute_topological_levels() which can cause the slab to
> grow, we therefore need to clear the slab again using
> clear_topo_level_slab() when we're done.
>
> First introduced in 72a2bfcaf01860ce8dd6921490d903dc0ad59c89 - which is
> currently only in master and not on maint.
>
> LeakSanitizer output:
>
> ==1026==ERROR: LeakSanitizer: detected memory leaks
>
> Direct leak of 8 byte(s) in 1 object(s) allocated from: #0 0x498ae9 in
> realloc
> /src/llvm-project/compiler-rt/lib/asan/asan_malloc_linux.cpp:164:3 #1
> 0xafbed8 in xrealloc /src/git/wrapper.c:126:8 #2 0x7966d1 in
> topo_level_slab_at_peek /src/git/commit-graph.c:71:1 #3 0x7965e0 in
> topo_level_slab_at /src/git/commit-graph.c:71:1 #4 0x78fbf5 in
> compute_topological_levels /src/git/commit-graph.c:1472:12 #5 0x78c5c3
> in write_commit_graph /src/git/commit-graph.c:2456:2 #6 0x535c5f in
> graph_write /src/git/builtin/commit-graph.c:299:6 #7 0x5350ca in
> cmd_commit_graph /src/git/builtin/commit-graph.c:337:11 #8 0x4cddb1 in
> run_builtin /src/git/git.c:453:11 #9 0x4cabe2 in handle_builtin
> /src/git/git.c:704:3 #10 0x4cd084 in run_argv /src/git/git.c:771:4 #11
> 0x4ca424 in cmd_main /src/git/git.c:902:19 #12 0x707fb6 in main
> /src/git/common-main.c:52:11 #13 0x7fee4249383f in __libc_start_main
> (/lib/x86_64-linux-gnu/libc.so.6+0x2083f)
>
> Indirect leak of 524256 byte(s) in 1 object(s) allocated from: #0
> 0x498942 in calloc
> /src/llvm-project/compiler-rt/lib/asan/asan_malloc_linux.cpp:154:3 #1
> 0xafc088 in xcalloc /src/git/wrapper.c:140:8 #2 0x796870 in
> topo_level_slab_at_peek /src/git/commit-graph.c:71:1 #3 0x7965e0 in
> topo_level_slab_at /src/git/commit-graph.c:71:1 #4 0x78fbf5 in
> compute_topological_levels /src/git/commit-graph.c:1472:12 #5 0x78c5c3
> in write_commit_graph /src/git/commit-graph.c:2456:2 #6 0x535c5f in
> graph_write /src/git/builtin/commit-graph.c:299:6 #7 0x5350ca in
> cmd_commit_graph /src/git/builtin/commit-graph.c:337:11 #8 0x4cddb1 in
> run_builtin /src/git/git.c:453:11 #9 0x4cabe2 in handle_builtin
> /src/git/git.c:704:3 #10 0x4cd084 in run_argv /src/git/git.c:771:4 #11
> 0x4ca424 in cmd_main /src/git/git.c:902:19 #12 0x707fb6 in main
> /src/git/common-main.c:52:11 #13 0x7fee4249383f in __libc_start_main
> (/lib/x86_64-linux-gnu/libc.so.6+0x2083f)
>
> SUMMARY: AddressSanitizer: 524264 byte(s) leaked in 2 allocation(s).
> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-881%2Fahunt%2Fcommit-graph-leak-v1
> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-881/ahunt/commit-graph-leak-v1
> Pull-Request: https://github.com/gitgitgadget/git/pull/881
> commit-graph.c | 1 +
> 1 file changed, 1 insertion(+)
> diff --git a/commit-graph.c b/commit-graph.c
> index ed31843fa522..9529ec552139 100644
> --- a/commit-graph.c
> +++ b/commit-graph.c
> @@ -2471,6 +2471,7 @@ int write_commit_graph(struct object_directory *odb,
> free(ctx->graph_name);
> free(ctx->commits.list);
> oid_array_clear(&ctx->oids);
> + clear_topo_level_slab(&topo_levels);
>
> if (ctx->commit_graph_filenames_after) {
> for (i = 0; i < ctx->num_commit_graphs_after; i++) {
> base-commit: 2283e0e9af55689215afa39c03beb2315ce18e83
> --
> gitgitgadget
next reply other threads:[~2021-02-25 6:28 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-02-25 6:26 Abhishek Kumar [this message]
-- strict thread matches above, loose matches on Subject: below --
2021-02-19 20:13 [PATCH] commit-graph: avoid leaking topo_levels slab in write_commit_graph() Andrzej Hunt via GitGitGadget
2021-02-20 3:36 ` Junio C Hamano
2021-02-22 14:15 ` Derrick Stolee
2021-02-22 19:14 ` Andrzej Hunt
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
List information: http://vger.kernel.org/majordomo-info.html
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=YDdDCMMf4Csumeci@Abhishek-Arch \
--to=abhishekkumar8222@gmail.com \
--cc=ajrhunt@google.com \
--cc=andrzej@ahunt.org \
--cc=git@vger.kernel.org \
--cc=gitgitgadget@gmail.com \
--cc=pull.881.git.1613765590412.gitgitgadget@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.
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
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 read-only IMAP folder(s) and NNTP newsgroup(s).