git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
* Possible memory leak bug reports for the latest "git" repository.
@ 2021-04-09 13:59 ‍우승훈[ 대학원석·박사통합과정수료연구(재학) / 컴퓨터학과 ]
  2021-04-09 16:49 ` Andrzej Hunt
  0 siblings, 1 reply; 3+ messages in thread
From: ‍우승훈[ 대학원석·박사통합과정수료연구(재학) / 컴퓨터학과 ] @ 2021-04-09 13:59 UTC (permalink / raw)
  To: git; +Cc: hanjiyeon0

Dear Git Development team.

Hi.
Recently, we discovered that the vulnerable code of CVE-2019-9169 is
included in the latest version (v2.31.1) of your source repository
(https://github.com/git/git).

Vulnerable file: git/compat/regex/regexec.c

Ref: CVE-2019-9169 (https://nvd.nist.gov/vuln/detail/CVE-2019-9169)

Original patch link:
https://sourceware.org/git/?p=glibc.git;a=blobdiff;f=posix/regexec.c;h=084b1222d95b62eb2930166060174ef78cb74b02;hp=91d5a797b82e2679ceab74238416de06693e46ea;hb=583dd860d5b833037175247230a328f0050dbfe9;hpb=2bac7daa58da1a313bd452369b0508b31e146637


Although the original vulnerability caused a heap-based buffer
over-read, the vulnerable code seems to cause a memory leak in your
repository.

Could you review whether this is a real vulnerability?

The below information shows our testing environments and results:

*****************************************************************************************
First test:

OS: Ubuntu 16.04.07 LTS
Git version: v2.31.1
Makefile config:

  1184 # Set CFLAGS, LDFLAGS and other *FLAGS variables. These might be
  1185 # tweaked by config.* below as well as the command-line, both of
  1186 # which'll override these defaults.
  1187 CFLAGS = -g -O2 -Wall -fsanitize=address
  1188 LDFLAGS =

Result:

$ ./git-log -i '\(\(\)*.\)*\(\)\(\)\1'
fatal: your current branch 'master' does not have any commits yet
=================================================================
==27253==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 65 byte(s) in 1 object(s) allocated from:
    #0 0x7f6ff01de961 in realloc
(/usr/lib/x86_64-linux-gnu/libasan.so.2+0x98961)
    #1 0x843ee5 in xrealloc  /HDD/POC/CVE-2019-9169/git-2.31.1/wrapper.c:126

SUMMARY: AddressSanitizer: 65 byte(s) leaked in 1 allocation(s).
*****************************************************************************************


*****************************************************************************************
Second test:

OS: Ubuntu 18.04.5 LTS
Git version: v2.31.1
Makefile config: Default

Result:

$ printf xxxxxxxxxxxxxx |valgrind --leak-check=full ./git-log -i '\(\(\)*.\)*\1'
==63105== Memcheck, a memory error detector
==63105== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
==63105== Using Valgrind-3.13.0 and LibVEX; rerun with -h for copyright info
==63105== Command: ./git-log -i \\(\\(\\)*.\\)*\\1
==63105==
==63105==
==63105== HEAP SUMMARY:
==63105==     in use at exit: 118,368,930 bytes in 76,720 blocks
==63105==   total heap usage: 463,994 allocs, 387,274 frees,
2,273,364,625 bytes allocated
==63105==
==63105== 40 bytes in 1 blocks are possibly lost in loss record 38 of 142
==63105==    at 0x4C31B0F: malloc (in
/usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==63105==    by 0x34013A: do_xmalloc (wrapper.c:41)
==63105==    by 0x355BB5: chdir_notify_register (chdir-notify.c:18)
==63105==    by 0x355BB5: chdir_notify_reparent (chdir-notify.c:48)
==63105==    by 0x2D5DC6: packed_ref_store_create (packed-backend.c:207)
==63105==    by 0x2D02BA: files_ref_store_create (files-backend.c:96)
==63105==    by 0x2CC295: get_main_ref_store (refs.c:1900)
==63105==    by 0x2E969D: handle_revision_pseudo_opt (revision.c:2578)
==63105==    by 0x2E969D: setup_revisions (revision.c:2730)
==63105==    by 0x180562: cmd_log_init_finish (log.c:206)
==63105==    by 0x18241F: cmd_log_init (log.c:275)
==63105==    by 0x18241F: cmd_log (log.c:754)
==63105==    by 0x12781D: run_builtin (git.c:453)
==63105==    by 0x12781D: handle_builtin (git.c:704)
==63105==    by 0x1289A4: cmd_main (git.c:872)
==63105==    by 0x127467: main (common-main.c:52)
==63105==
==63105== 40 bytes in 1 blocks are possibly lost in loss record 39 of 142
==63105==    at 0x4C31B0F: malloc (in
/usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==63105==    by 0x34013A: do_xmalloc (wrapper.c:41)
==63105==    by 0x355BB5: chdir_notify_register (chdir-notify.c:18)
==63105==    by 0x355BB5: chdir_notify_reparent (chdir-notify.c:48)
==63105==    by 0x2D02D6: files_ref_store_create (files-backend.c:99)
==63105==    by 0x2CC295: get_main_ref_store (refs.c:1900)
==63105==    by 0x2E969D: handle_revision_pseudo_opt (revision.c:2578)
==63105==    by 0x2E969D: setup_revisions (revision.c:2730)
==63105==    by 0x180562: cmd_log_init_finish (log.c:206)
==63105==    by 0x18241F: cmd_log_init (log.c:275)
==63105==    by 0x18241F: cmd_log (log.c:754)
==63105==    by 0x12781D: run_builtin (git.c:453)
==63105==    by 0x12781D: handle_builtin (git.c:704)
==63105==    by 0x1289A4: cmd_main (git.c:872)
==63105==    by 0x127467: main (common-main.c:52)
==63105==
==63105== 40 bytes in 1 blocks are possibly lost in loss record 40 of 142
==63105==    at 0x4C31B0F: malloc (in
/usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==63105==    by 0x34013A: do_xmalloc (wrapper.c:41)
==63105==    by 0x355BB5: chdir_notify_register (chdir-notify.c:18)
==63105==    by 0x355BB5: chdir_notify_reparent (chdir-notify.c:48)
==63105==    by 0x2D02E6: files_ref_store_create (files-backend.c:100)
==63105==    by 0x2CC295: get_main_ref_store (refs.c:1900)
==63105==    by 0x2E969D: handle_revision_pseudo_opt (revision.c:2578)
==63105==    by 0x2E969D: setup_revisions (revision.c:2730)
==63105==    by 0x180562: cmd_log_init_finish (log.c:206)
==63105==    by 0x18241F: cmd_log_init (log.c:275)
==63105==    by 0x18241F: cmd_log (log.c:754)
==63105==    by 0x12781D: run_builtin (git.c:453)
==63105==    by 0x12781D: handle_builtin (git.c:704)
==63105==    by 0x1289A4: cmd_main (git.c:872)
==63105==    by 0x127467: main (common-main.c:52)
==63105==
==63105== 65 bytes in 1 blocks are definitely lost in loss record 59 of 142
==63105==    at 0x4C31A3F: malloc (in
/usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==63105==    by 0x4C33D84: realloc (in
/usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==63105==    by 0x340345: xrealloc (wrapper.c:126)
==63105==    by 0x30B93A: strbuf_grow (strbuf.c:98)
==63105==    by 0x30C8EC: strbuf_vaddf (strbuf.c:392)
==63105==    by 0x30CA33: strbuf_addf (strbuf.c:333)
==63105==    by 0x2A9700: preprocess_options (parse-options.c:666)
==63105==    by 0x2A9700: parse_options (parse-options.c:847)
==63105==    by 0x18053E: cmd_log_init_finish (log.c:199)
==63105==    by 0x18241F: cmd_log_init (log.c:275)
==63105==    by 0x18241F: cmd_log (log.c:754)
==63105==    by 0x12781D: run_builtin (git.c:453)
==63105==    by 0x12781D: handle_builtin (git.c:704)
==63105==    by 0x1289A4: cmd_main (git.c:872)
==63105==    by 0x127467: main (common-main.c:52)
==63105==
==63105== 84 (56 direct, 28 indirect) bytes in 1 blocks are definitely
lost in loss record 61 of 142
==63105==    at 0x4C31B0F: malloc (in
/usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==63105==    by 0x34013A: do_xmalloc (wrapper.c:41)
==63105==    by 0x2AE9C0: copy_pathspec (pathspec.c:664)
==63105==    by 0x2E993A: setup_revisions (revision.c:2844)
==63105==    by 0x180562: cmd_log_init_finish (log.c:206)
==63105==    by 0x18241F: cmd_log_init (log.c:275)
==63105==    by 0x18241F: cmd_log (log.c:754)
==63105==    by 0x12781D: run_builtin (git.c:453)
==63105==    by 0x12781D: handle_builtin (git.c:704)
==63105==    by 0x1289A4: cmd_main (git.c:872)
==63105==    by 0x127467: main (common-main.c:52)
==63105==
==63105== 84 (56 direct, 28 indirect) bytes in 1 blocks are definitely
lost in loss record 62 of 142
==63105==    at 0x4C31B0F: malloc (in
/usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==63105==    by 0x34013A: do_xmalloc (wrapper.c:41)
==63105==    by 0x2AE9C0: copy_pathspec (pathspec.c:664)
==63105==    by 0x2E996A: setup_revisions (revision.c:2849)
==63105==    by 0x180562: cmd_log_init_finish (log.c:206)
==63105==    by 0x18241F: cmd_log_init (log.c:275)
==63105==    by 0x18241F: cmd_log (log.c:754)
==63105==    by 0x12781D: run_builtin (git.c:453)
==63105==    by 0x12781D: handle_builtin (git.c:704)
==63105==    by 0x1289A4: cmd_main (git.c:872)
==63105==    by 0x127467: main (common-main.c:52)
==63105==
==63105== 191 (112 direct, 79 indirect) bytes in 1 blocks are
definitely lost in loss record 76 of 142
==63105==    at 0x4C31B0F: malloc (in
/usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==63105==    by 0x34013A: do_xmalloc (wrapper.c:41)
==63105==    by 0x2AE381: parse_pathspec (pathspec.c:582)
==63105==    by 0x2E9164: setup_revisions (revision.c:2800)
==63105==    by 0x180562: cmd_log_init_finish (log.c:206)
==63105==    by 0x18241F: cmd_log_init (log.c:275)
==63105==    by 0x18241F: cmd_log (log.c:754)
==63105==    by 0x12781D: run_builtin (git.c:453)
==63105==    by 0x12781D: handle_builtin (git.c:704)
==63105==    by 0x1289A4: cmd_main (git.c:872)
==63105==    by 0x127467: main (common-main.c:52)
==63105==
==63105== 464 (448 direct, 16 indirect) bytes in 28 blocks are
definitely lost in loss record 98 of 142
==63105==    at 0x4C31B0F: malloc (in
/usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==63105==    by 0x34013A: do_xmalloc (wrapper.c:41)
==63105==    by 0x217A3D: commit_list_insert (commit.c:554)
==63105==    by 0x217A3D: parse_commit_buffer (commit.c:447)
==63105==    by 0x296F37: parse_object_buffer (object.c:217)
==63105==    by 0x297070: parse_object (object.c:282)
==63105==    by 0x274781: add_ref_decoration (log-tree.c:179)
==63105==    by 0x2D5C53: do_for_each_repo_ref_iterator (iterator.c:418)
==63105==    by 0x2CB9F8: do_for_each_ref (refs.c:1492)
==63105==    by 0x274A65: load_ref_decorations (log-tree.c:211)
==63105==    by 0x1807C9: cmd_log_init_finish (log.c:262)
==63105==    by 0x18241F: cmd_log_init (log.c:275)
==63105==    by 0x18241F: cmd_log (log.c:754)
==63105==    by 0x12781D: run_builtin (git.c:453)
==63105==    by 0x12781D: handle_builtin (git.c:704)
==63105==
==63105== 37,468 (32 direct, 37,436 indirect) bytes in 1 blocks are
definitely lost in loss record 120 of 142
==63105==    at 0x4C33B25: calloc (in
/usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==63105==    by 0x3403AD: xcalloc (wrapper.c:140)
==63105==    by 0x1806F4: cmd_log_init_finish (log.c:233)
==63105==    by 0x18241F: cmd_log_init (log.c:275)
==63105==    by 0x18241F: cmd_log (log.c:754)
==63105==    by 0x12781D: run_builtin (git.c:453)
==63105==    by 0x12781D: handle_builtin (git.c:704)
==63105==    by 0x1289A4: cmd_main (git.c:872)
==63105==    by 0x127467: main (common-main.c:52)
==63105==
==63105== 184,176 (183,792 direct, 384 indirect) bytes in 11,487
blocks are definitely lost in loss record 130 of 142
==63105==    at 0x4C31B0F: malloc (in
/usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==63105==    by 0x34013A: do_xmalloc (wrapper.c:41)
==63105==    by 0x217A3D: commit_list_insert (commit.c:554)
==63105==    by 0x217A3D: parse_commit_buffer (commit.c:447)
==63105==    by 0x217CEC: repo_parse_commit_internal (commit.c:498)
==63105==    by 0x2E4C3D: repo_parse_commit (commit.h:89)
==63105==    by 0x2E4C3D: try_to_simplify_commit (revision.c:1007)
==63105==    by 0x2E84AA: process_parents (revision.c:1140)
==63105==    by 0x2EBFED: get_revision_1 (revision.c:3999)
==63105==    by 0x2EC1BF: get_revision_internal (revision.c:4113)
==63105==    by 0x2EC344: get_revision (revision.c:4187)
==63105==    by 0x181538: cmd_log_walk (log.c:422)
==63105==    by 0x182427: cmd_log (log.c:755)
==63105==    by 0x12781D: run_builtin (git.c:453)
==63105==    by 0x12781D: handle_builtin (git.c:704)
==63105==
==63105== LEAK SUMMARY:
==63105==    definitely lost: 184,561 bytes in 11,520 blocks
==63105==    indirectly lost: 37,971 bytes in 1,139 blocks
==63105==      possibly lost: 120 bytes in 3 blocks
==63105==    still reachable: 118,146,278 bytes in 64,058 blocks
==63105==         suppressed: 0 bytes in 0 blocks
==63105== Reachable blocks (those to which a pointer was found) are not shown.
==63105== To see them, rerun with: --leak-check=full --show-leak-kinds=all
==63105==
==63105== For counts of detected and suppressed errors, rerun with: -v
==63105== ERROR SUMMARY: 10 errors from 10 contexts (suppressed: 0 from 0)
*****************************************************************************************

Thank you.
Best regards,
Seunghoon Woo, Jiyeon Han

-- 

Best regards,

Seunghoon Woo
Korea University Dept. of Computer Science and Engineering
Computer & Communication Security Lab.
seunghoonwoo@korea.ac.kr
(+82)10-8147-9308

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: Possible memory leak bug reports for the latest "git" repository.
  2021-04-09 13:59 Possible memory leak bug reports for the latest "git" repository ‍우승훈[ 대학원석·박사통합과정수료연구(재학) / 컴퓨터학과 ]
@ 2021-04-09 16:49 ` Andrzej Hunt
  2021-04-09 18:46   ` Jeff King
  0 siblings, 1 reply; 3+ messages in thread
From: Andrzej Hunt @ 2021-04-09 16:49 UTC (permalink / raw)
  To: 우승훈[ 대학원석·박사통합과정수료연구(재학) / 컴퓨터학과 ],
	git
  Cc: hanjiyeon0

On 09/04/2021 15:59, 우승훈[ 대학원석·박사통합과정수료연구(재학) / 컴퓨 
터학과 ] wrote:
> Dear Git Development team.
> 
> Hi.
> Recently, we discovered that the vulnerable code of CVE-2019-9169 is
> included in the latest version (v2.31.1) of your source repository
> (https://github.com/git/git).
> 
> Vulnerable file: git/compat/regex/regexec.c
> 
> Ref: CVE-2019-9169 (https://nvd.nist.gov/vuln/detail/CVE-2019-9169)
> 
> Original patch link:
> https://sourceware.org/git/?p=glibc.git;a=blobdiff;f=posix/regexec.c;h=084b1222d95b62eb2930166060174ef78cb74b02;hp=91d5a797b82e2679ceab74238416de06693e46ea;hb=583dd860d5b833037175247230a328f0050dbfe9;hpb=2bac7daa58da1a313bd452369b0508b31e146637
> 
> 
> Although the original vulnerability caused a heap-based buffer
> over-read, the vulnerable code seems to cause a memory leak in your
> repository.

I don't feel qualified to comment on the vulnerability itself (except 
that by inspection the code looks the same) - hopefully someone with 
more expertise can chime in. That said, I ran your repro example through 
a debugger and at least /on my system/, the problematic code is not 
being called when using /the specific testcases you supplied/.

Regarding the memory leaks you have found: they are not of particular 
concern - git has a lot of small memory leaks, they're ugly but not 
impactful (that said - I've been working on fixing some of those leaks). 
In fact I don't think any of the leaks you found are in any way related 
to the regex code - see analysis inline:

> $ ./git-log -i '\(\(\)*.\)*\(\)\(\)\1'
> fatal: your current branch 'master' does not have any commits yet
> =================================================================
> ==27253==ERROR: LeakSanitizer: detected memory leaks
> 
> Direct leak of 65 byte(s) in 1 object(s) allocated from:
>      #0 0x7f6ff01de961 in realloc
> (/usr/lib/x86_64-linux-gnu/libasan.so.2+0x98961)
>      #1 0x843ee5 in xrealloc  /HDD/POC/CVE-2019-9169/git-2.31.1/wrapper.c:126
> 
> SUMMARY: AddressSanitizer: 65 byte(s) leaked in 1 allocation(s).
> *****************************************************************************************

Unfortunately this log is a bit useless: LSAN should print the call 
stack down to realloc, but it sometimes misses everything above the 
wrapper calls. I haven't managed to figure out why this is happening. 
That said, I can't reproduce this leak locally (clang-11 + ASAN + LSAN), 
so it's plausible that it was fixed recently.

> 
> 
> *****************************************************************************************
> Second test:
> 
> OS: Ubuntu 18.04.5 LTS
> Git version: v2.31.1
> Makefile config: Default
> 
> Result:
> 
> $ printf xxxxxxxxxxxxxx |valgrind --leak-check=full ./git-log -i '\(\(\)*.\)*\1'
> ==63105== Memcheck, a memory error detector
> ==63105== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
> ==63105== Using Valgrind-3.13.0 and LibVEX; rerun with -h for copyright info
> ==63105== Command: ./git-log -i \\(\\(\\)*.\\)*\\1
> ==63105==
> ==63105==
> ==63105== HEAP SUMMARY:
> ==63105==     in use at exit: 118,368,930 bytes in 76,720 blocks
> ==63105==   total heap usage: 463,994 allocs, 387,274 frees,
> 2,273,364,625 bytes allocated
> ==63105==
> ==63105== 40 bytes in 1 blocks are possibly lost in loss record 38 of 142
> ==63105==    at 0x4C31B0F: malloc (in
> /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
> ==63105==    by 0x34013A: do_xmalloc (wrapper.c:41)
> ==63105==    by 0x355BB5: chdir_notify_register (chdir-notify.c:18)
> ==63105==    by 0x355BB5: chdir_notify_reparent (chdir-notify.c:48)
> ==63105==    by 0x2D5DC6: packed_ref_store_create (packed-backend.c:207)
> ==63105==    by 0x2D02BA: files_ref_store_create (files-backend.c:96)
> ==63105==    by 0x2CC295: get_main_ref_store (refs.c:1900)
> ==63105==    by 0x2E969D: handle_revision_pseudo_opt (revision.c:2578)
> ==63105==    by 0x2E969D: setup_revisions (revision.c:2730)
> ==63105==    by 0x180562: cmd_log_init_finish (log.c:206)
> ==63105==    by 0x18241F: cmd_log_init (log.c:275)
> ==63105==    by 0x18241F: cmd_log (log.c:754)
> ==63105==    by 0x12781D: run_builtin (git.c:453)
> ==63105==    by 0x12781D: handle_builtin (git.c:704)
> ==63105==    by 0x1289A4: cmd_main (git.c:872)
> ==63105==    by 0x127467: main (common-main.c:52)
> ==63105==
> ==63105== 40 bytes in 1 blocks are possibly lost in loss record 39 of 142
> ==63105==    at 0x4C31B0F: malloc (in
> /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
> ==63105==    by 0x34013A: do_xmalloc (wrapper.c:41)
> ==63105==    by 0x355BB5: chdir_notify_register (chdir-notify.c:18)
> ==63105==    by 0x355BB5: chdir_notify_reparent (chdir-notify.c:48)
> ==63105==    by 0x2D02D6: files_ref_store_create (files-backend.c:99)
> ==63105==    by 0x2CC295: get_main_ref_store (refs.c:1900)
> ==63105==    by 0x2E969D: handle_revision_pseudo_opt (revision.c:2578)
> ==63105==    by 0x2E969D: setup_revisions (revision.c:2730)
> ==63105==    by 0x180562: cmd_log_init_finish (log.c:206)
> ==63105==    by 0x18241F: cmd_log_init (log.c:275)
> ==63105==    by 0x18241F: cmd_log (log.c:754)
> ==63105==    by 0x12781D: run_builtin (git.c:453)
> ==63105==    by 0x12781D: handle_builtin (git.c:704)
> ==63105==    by 0x1289A4: cmd_main (git.c:872)
> ==63105==    by 0x127467: main (common-main.c:52)
> ==63105==
> ==63105== 40 bytes in 1 blocks are possibly lost in loss record 40 of 142
> ==63105==    at 0x4C31B0F: malloc (in
> /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
> ==63105==    by 0x34013A: do_xmalloc (wrapper.c:41)
> ==63105==    by 0x355BB5: chdir_notify_register (chdir-notify.c:18)
> ==63105==    by 0x355BB5: chdir_notify_reparent (chdir-notify.c:48)
> ==63105==    by 0x2D02E6: files_ref_store_create (files-backend.c:100)
> ==63105==    by 0x2CC295: get_main_ref_store (refs.c:1900)
> ==63105==    by 0x2E969D: handle_revision_pseudo_opt (revision.c:2578)
> ==63105==    by 0x2E969D: setup_revisions (revision.c:2730)
> ==63105==    by 0x180562: cmd_log_init_finish (log.c:206)
> ==63105==    by 0x18241F: cmd_log_init (log.c:275)
> ==63105==    by 0x18241F: cmd_log (log.c:754)
> ==63105==    by 0x12781D: run_builtin (git.c:453)
> ==63105==    by 0x12781D: handle_builtin (git.c:704)
> ==63105==    by 0x1289A4: cmd_main (git.c:872)
> ==63105==    by 0x127467: main (common-main.c:52)
> ==63105==
> ==63105== 65 bytes in 1 blocks are definitely lost in loss record 59 of 142

These are a bit odd: chdir_notify_register() is attaching the newly 
allocated memory to a static list (chdir_notify_entries), and I can't 
see any obvious reason why this would leak (but maybe valgrind doesn't 
like static pointers)? Either way, this doesn't seem to be anywhere near 
the regex code.

> ==63105==    at 0x4C31A3F: malloc (in
> /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
> ==63105==    by 0x4C33D84: realloc (in
> /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
> ==63105==    by 0x340345: xrealloc (wrapper.c:126)
> ==63105==    by 0x30B93A: strbuf_grow (strbuf.c:98)
> ==63105==    by 0x30C8EC: strbuf_vaddf (strbuf.c:392)
> ==63105==    by 0x30CA33: strbuf_addf (strbuf.c:333)
> ==63105==    by 0x2A9700: preprocess_options (parse-options.c:666)
> ==63105==    by 0x2A9700: parse_options (parse-options.c:847)
> ==63105==    by 0x18053E: cmd_log_init_finish (log.c:199)
> ==63105==    by 0x18241F: cmd_log_init (log.c:275)
> ==63105==    by 0x18241F: cmd_log (log.c:754)
> ==63105==    by 0x12781D: run_builtin (git.c:453)
> ==63105==    by 0x12781D: handle_builtin (git.c:704)
> ==63105==    by 0x1289A4: cmd_main (git.c:872)
> ==63105==    by 0x127467: main (common-main.c:52)
> ==63105==

This was fixed in:
https://github.com/git/git/commit/64cc539fd2f8e02f39cfae21e9523da2532c0467#diff-1f0cbd43fdb6fcca38e0bcd390901118a948a9fb3a4f9b2e2edb2783d343f0b8

> ==63105== 84 (56 direct, 28 indirect) bytes in 1 blocks are definitely
> lost in loss record 61 of 142
> ==63105==    at 0x4C31B0F: malloc (in
> /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
> ==63105==    by 0x34013A: do_xmalloc (wrapper.c:41)
> ==63105==    by 0x2AE9C0: copy_pathspec (pathspec.c:664)
> ==63105==    by 0x2E993A: setup_revisions (revision.c:2844)
> ==63105==    by 0x180562: cmd_log_init_finish (log.c:206)
> ==63105==    by 0x18241F: cmd_log_init (log.c:275)
> ==63105==    by 0x18241F: cmd_log (log.c:754)
> ==63105==    by 0x12781D: run_builtin (git.c:453)
> ==63105==    by 0x12781D: handle_builtin (git.c:704)
> ==63105==    by 0x1289A4: cmd_main (git.c:872)
> ==63105==    by 0x127467: main (common-main.c:52 > ==63105==
> ==63105== 84 (56 direct, 28 indirect) bytes in 1 blocks are definitely
> lost in loss record 62 of 142
> ==63105==    at 0x4C31B0F: malloc (in
> /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
> ==63105==    by 0x34013A: do_xmalloc (wrapper.c:41)
> ==63105==    by 0x2AE9C0: copy_pathspec (pathspec.c:664)
> ==63105==    by 0x2E996A: setup_revisions (revision.c:2849)
> ==63105==    by 0x180562: cmd_log_init_finish (log.c:206)
> ==63105==    by 0x18241F: cmd_log_init (log.c:275)
> ==63105==    by 0x18241F: cmd_log (log.c:754)
> ==63105==    by 0x12781D: run_builtin (git.c:453)
> ==63105==    by 0x12781D: handle_builtin (git.c:704)
> ==63105==    by 0x1289A4: cmd_main (git.c:872)
> ==63105==    by 0x127467: main (common-main.c:52)
> ==63105==
> ==63105== 191 (112 direct, 79 indirect) bytes in 1 blocks are
> definitely lost in loss record 76 of 142
> ==63105==    at 0x4C31B0F: malloc (in
> /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
> ==63105==    by 0x34013A: do_xmalloc (wrapper.c:41)
> ==63105==    by 0x2AE381: parse_pathspec (pathspec.c:582)
> ==63105==    by 0x2E9164: setup_revisions (revision.c:2800)
> ==63105==    by 0x180562: cmd_log_init_finish (log.c:206)
> ==63105==    by 0x18241F: cmd_log_init (log.c:275)
> ==63105==    by 0x18241F: cmd_log (log.c:754)
> ==63105==    by 0x12781D: run_builtin (git.c:453)
> ==63105==    by 0x12781D: handle_builtin (git.c:704)
> ==63105==    by 0x1289A4: cmd_main (git.c:872)
> ==63105==    by 0x127467: main (common-main.c:52)
> ==63105==
> ==63105== 464 (448 direct, 16 indirect) bytes in 28 blocks are
> definitely lost in loss record 98 of 14 > ==63105==    at 0x4C31B0F: malloc (in
> /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
> ==63105==    by 0x34013A: do_xmalloc (wrapper.c:41)
> ==63105==    by 0x217A3D: commit_list_insert (commit.c:554)
> ==63105==    by 0x217A3D: parse_commit_buffer (commit.c:447)
> ==63105==    by 0x296F37: parse_object_buffer (object.c:217)
> ==63105==    by 0x297070: parse_object (object.c:282)
> ==63105==    by 0x274781: add_ref_decoration (log-tree.c:179)
> ==63105==    by 0x2D5C53: do_for_each_repo_ref_iterator (iterator.c:418)
> ==63105==    by 0x2CB9F8: do_for_each_ref (refs.c:1492)
> ==63105==    by 0x274A65: load_ref_decorations (log-tree.c:211)
> ==63105==    by 0x1807C9: cmd_log_init_finish (log.c:262)
> ==63105==    by 0x18241F: cmd_log_init (log.c:275)
> ==63105==    by 0x18241F: cmd_log (log.c:754)
> ==63105==    by 0x12781D: run_builtin (git.c:453)
> ==63105==    by 0x12781D: handle_builtin (git.c:704)
> ==63105==
> ==63105== 37,468 (32 direct, 37,436 indirect) bytes in 1 blocks are
> definitely lost in loss record 120 of 142
> ==63105==    at 0x4C33B25: calloc (in
> /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
> ==63105==    by 0x3403AD: xcalloc (wrapper.c:140)
> ==63105==    by 0x1806F4: cmd_log_init_finish (log.c:233)
> ==63105==    by 0x18241F: cmd_log_init (log.c:275)
> ==63105==    by 0x18241F: cmd_log (log.c:754)
> ==63105==    by 0x12781D: run_builtin (git.c:453)
> ==63105==    by 0x12781D: handle_builtin (git.c:704)
> ==63105==    by 0x1289A4: cmd_main (git.c:872)
> ==63105==    by 0x127467: main (common-main.c:52)
> ==63105==

These aren't really "real" leaks because they're populating a rev_info 
that is owned by cmd_log. In other words: the data isn't leaked until 
cmd_log() returns, and by that point we don't care anymore.

That said, I have a patch prepared to UNLEAK the rev_info in question 
which will suppress these leaks when running with 
-DSUPPRESS_ANNOTATED_LEAKS, which I'll probably be sending out soon.

> ==63105== 184,176 (183,792 direct, 384 indirect) bytes in 11,487
> blocks are definitely lost in loss record 130 of 142
> ==63105==    at 0x4C31B0F: malloc (in
> /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
> ==63105==    by 0x34013A: do_xmalloc (wrapper.c:41)
> ==63105==    by 0x217A3D: commit_list_insert (commit.c:554)
> ==63105==    by 0x217A3D: parse_commit_buffer (commit.c:447)
> ==63105==    by 0x217CEC: repo_parse_commit_internal (commit.c:498)
> ==63105==    by 0x2E4C3D: repo_parse_commit (commit.h:89)
> ==63105==    by 0x2E4C3D: try_to_simplify_commit (revision.c:1007)
> ==63105==    by 0x2E84AA: process_parents (revision.c:1140)
> ==63105==    by 0x2EBFED: get_revision_1 (revision.c:3999)
> ==63105==    by 0x2EC1BF: get_revision_internal (revision.c:4113)
> ==63105==    by 0x2EC344: get_revision (revision.c:4187)
> ==63105==    by 0x181538: cmd_log_walk (log.c:422)
> ==63105==    by 0x182427: cmd_log (log.c:755)
> ==63105==    by 0x12781D: run_builtin (git.c:453)
> ==63105==    by 0x12781D: handle_builtin (git.c:704)

I suspect this is similar to the last bunch: we end up leaking anything 
that's added to rev_info when cmd_log() returns. Not a big deal, but it 
will also be supressed with an UNLEAK in future.

ATB,
     Andrzej

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: Possible memory leak bug reports for the latest "git" repository.
  2021-04-09 16:49 ` Andrzej Hunt
@ 2021-04-09 18:46   ` Jeff King
  0 siblings, 0 replies; 3+ messages in thread
From: Jeff King @ 2021-04-09 18:46 UTC (permalink / raw)
  To: Andrzej Hunt
  Cc: 우승훈[ 대학원석·박사통합과정수료연구(재학) / 컴퓨터학과 ],
	git, hanjiyeon0

On Fri, Apr 09, 2021 at 06:49:39PM +0200, Andrzej Hunt wrote:

> > ==63105== 40 bytes in 1 blocks are possibly lost in loss record 38 of 142
> > ==63105==    at 0x4C31B0F: malloc (in
> > /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
> > ==63105==    by 0x34013A: do_xmalloc (wrapper.c:41)
> > ==63105==    by 0x355BB5: chdir_notify_register (chdir-notify.c:18)
> [...]
> 
> These are a bit odd: chdir_notify_register() is attaching the newly
> allocated memory to a static list (chdir_notify_entries), and I can't see
> any obvious reason why this would leak (but maybe valgrind doesn't like
> static pointers)? Either way, this doesn't seem to be anywhere near the
> regex code.

These "possibly lost" are false positives; there's more discussion in
this thread:

  https://lore.kernel.org/git/20201117002435.GA13516@coredump.intra.peff.net/

-Peff

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2021-04-09 18:46 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-04-09 13:59 Possible memory leak bug reports for the latest "git" repository ‍우승훈[ 대학원석·박사통합과정수료연구(재학) / 컴퓨터학과 ]
2021-04-09 16:49 ` Andrzej Hunt
2021-04-09 18:46   ` Jeff King

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).