diff mbox series

[08/12] convert: release strbuf to avoid leak

Message ID 20210620151204.19260-9-andrzej@ahunt.org (mailing list archive)
State New, archived
Headers show
Series Fix all leaks in tests t0002-t0099: Part 2 | expand

Commit Message

Andrzej Hunt June 20, 2021, 3:12 p.m. UTC
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:

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(+)

Comments

Elijah Newren June 21, 2021, 8:31 p.m. UTC | #1
On Sun, Jun 20, 2021 at 8:15 AM <andrzej@ahunt.org> wrote:
>
> 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:
>
> 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;
>  }
>
> --
> 2.26.2

Makes sense; a `git grep -e STRBUF_INIT -e strbuf_release -e
strbuf_init convert.c` does a fairly good job of highlighting that
these appear to be the only two strbuf leaks in this file.
diff mbox series

Patch

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;
 }