diff mbox series

[v2,07/12] read-cache: call diff_setup_done to avoid leak

Message ID 20210725130830.5145-8-andrzej@ahunt.org (mailing list archive)
State Accepted
Commit 8d833e933719cd4badb7ec7d042d6f7cc206247d
Headers show
Series [v2,01/12] fmt-merge-msg: free newly allocated temporary strings when done | expand

Commit Message

Andrzej Hunt July 25, 2021, 1:08 p.m. UTC
From: Andrzej Hunt <ajrhunt@google.com>

repo_diff_setup() calls through to diff.c's static prep_parse_options(),
which in  turn allocates a new array into diff_opts.parseopts.
diff_setup_done() is responsible for freeing that array, and has the
benefit of verifying diff_opts too - hence we add a call to
diff_setup_done() to avoid leaking parseopts.

Output from the leak as found while running t0090 with LSAN:

Direct leak of 7120 byte(s) in 1 object(s) allocated from:
    #0 0x49a82d in malloc ../projects/compiler-rt/lib/asan/asan_malloc_linux.cpp:145:3
    #1 0xa8bf89 in do_xmalloc wrapper.c:41:8
    #2 0x7a7bae in prep_parse_options diff.c:5636:2
    #3 0x7a7bae in repo_diff_setup diff.c:4611:2
    #4 0x93716c in repo_index_has_changes read-cache.c:2518:3
    #5 0x872233 in unclean merge-ort-wrappers.c:12:14
    #6 0x872233 in merge_ort_recursive merge-ort-wrappers.c:53:6
    #7 0x5d5b11 in try_merge_strategy builtin/merge.c:752:12
    #8 0x5d0b6b in cmd_merge builtin/merge.c:1666:9
    #9 0x4ce83e in run_builtin git.c:475:11
    #10 0x4ccafe in handle_builtin git.c:729:3
    #11 0x4cb01c in run_argv git.c:818:4
    #12 0x4cb01c in cmd_main git.c:949:19
    #13 0x6bdc2d in main common-main.c:52:11
    #14 0x7f551eb51349 in __libc_start_main (/lib64/libc.so.6+0x24349)

SUMMARY: AddressSanitizer: 7120 byte(s) leaked in 1 allocation(s)

Signed-off-by: Andrzej Hunt <andrzej@ahunt.org>
---
 read-cache.c | 1 +
 1 file changed, 1 insertion(+)

Comments

Junio C Hamano July 26, 2021, 8:10 p.m. UTC | #1
andrzej@ahunt.org writes:

> From: Andrzej Hunt <ajrhunt@google.com>
>
> repo_diff_setup() calls through to diff.c's static prep_parse_options(),
> which in  turn allocates a new array into diff_opts.parseopts.
> diff_setup_done() is responsible for freeing that array, and has the
> benefit of verifying diff_opts too - hence we add a call to
> diff_setup_done() to avoid leaking parseopts.

Right.  Thanks.

>
> Output from the leak as found while running t0090 with LSAN:
>
> Direct leak of 7120 byte(s) in 1 object(s) allocated from:
>     #0 0x49a82d in malloc ../projects/compiler-rt/lib/asan/asan_malloc_linux.cpp:145:3
>     #1 0xa8bf89 in do_xmalloc wrapper.c:41:8
>     #2 0x7a7bae in prep_parse_options diff.c:5636:2
>     #3 0x7a7bae in repo_diff_setup diff.c:4611:2
>     #4 0x93716c in repo_index_has_changes read-cache.c:2518:3
>     #5 0x872233 in unclean merge-ort-wrappers.c:12:14
>     #6 0x872233 in merge_ort_recursive merge-ort-wrappers.c:53:6
>     #7 0x5d5b11 in try_merge_strategy builtin/merge.c:752:12
>     #8 0x5d0b6b in cmd_merge builtin/merge.c:1666:9
>     #9 0x4ce83e in run_builtin git.c:475:11
>     #10 0x4ccafe in handle_builtin git.c:729:3
>     #11 0x4cb01c in run_argv git.c:818:4
>     #12 0x4cb01c in cmd_main git.c:949:19
>     #13 0x6bdc2d in main common-main.c:52:11
>     #14 0x7f551eb51349 in __libc_start_main (/lib64/libc.so.6+0x24349)
>
> SUMMARY: AddressSanitizer: 7120 byte(s) leaked in 1 allocation(s)
>
> Signed-off-by: Andrzej Hunt <andrzej@ahunt.org>
> ---
>  read-cache.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/read-cache.c b/read-cache.c
> index 46ccd66f34..83d1817ad0 100644
> --- a/read-cache.c
> +++ b/read-cache.c
> @@ -2505,6 +2505,7 @@ int repo_index_has_changes(struct repository *repo,
>  		opt.flags.exit_with_status = 1;
>  		if (!sb)
>  			opt.flags.quick = 1;
> +		diff_setup_done(&opt);
>  		do_diff_cache(&cmp, &opt);
>  		diffcore_std(&opt);
>  		for (i = 0; sb && i < diff_queued_diff.nr; i++) {
diff mbox series

Patch

diff --git a/read-cache.c b/read-cache.c
index 46ccd66f34..83d1817ad0 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -2505,6 +2505,7 @@  int repo_index_has_changes(struct repository *repo,
 		opt.flags.exit_with_status = 1;
 		if (!sb)
 			opt.flags.quick = 1;
+		diff_setup_done(&opt);
 		do_diff_cache(&cmp, &opt);
 		diffcore_std(&opt);
 		for (i = 0; sb && i < diff_queued_diff.nr; i++) {