git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH] commit-graph: avoid leaking topo_levels slab in write_commit_graph()

From
Junio C Hamano <gitster@pobox.com>
Date
Feb 20, 2021, 03:36 UTC
Message-ID
<xmqqa6rz9zrx.fsf@gitster.g>
In-Reply-To
<pull.881.git.1613765590412.gitgitgadget@gmail.com>
"Andrzej Hunt via GitGitGadget" <gitgitgadget@gmail.com> writes:
Show 9 quoted lines
> 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.

Forwarding to those who were involved in the said commit for insights.

Show 109 quoted lines
> 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
Previous: Andrzej Hunt via GitGitGadgetNext: Derrick Stolee
Message 2 of 4 in “commit-graph: avoid leaking topo_levels slab in write_commit_graph()”
  1. commit-graph: avoid leaking topo_levels slab in write_commit_graph()Andrzej Hunt via GitGitGadget, Feb 19, 2021
  2. Junio C HamanoFeb 20, 2021
  3. Derrick StoleeFeb 22, 2021
  4. Andrzej HuntFeb 22, 2021

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.