Skip to content

status: deserialization wait#7

Merged
jeffhostetler merged 1 commit intomicrosoft:gvfs-2.18.0from
jeffhostetler:gvfs-status-serialize-wait
Jul 31, 2018
Merged

status: deserialization wait#7
jeffhostetler merged 1 commit intomicrosoft:gvfs-2.18.0from
jeffhostetler:gvfs-status-serialize-wait

Conversation

@jeffhostetler
Copy link

@jeffhostetler jeffhostetler commented Jul 25, 2018

Extend git status --deserialize to better handle stale status cache files.

Add new --deserialize-wait command line argument and status.deserializeWait config setting.

@jeffhostetler jeffhostetler force-pushed the gvfs-status-serialize-wait branch 2 times, most recently from bc6d742 to 3d6837d Compare July 26, 2018 18:09
@jeffhostetler jeffhostetler force-pushed the gvfs-status-serialize-wait branch from 3d6837d to 068997e Compare July 31, 2018 18:17
@jeffhostetler
Copy link
Author

jeffhostetler commented Jul 31, 2018

@jamill I just did a force push to improve the code a little. It now lstat()'s the file so that while polling it doesn't try to re-ready it until it actually changes. And I added a trace message to summarize how many times it waited on the file. You don't need re-review it if you don't want to. This is more a heads up.

@jeffhostetler
status: deserialization wait
ea5f297 
Teach `git status --deserialize` to either wait indefintely
or immediately fail if the status serialization cache file
is stale.

Signed-off-by: Jeff Hostetler <jeffhost@microsoft.com>
@jeffhostetler jeffhostetler force-pushed the gvfs-status-serialize-wait branch from 068997e to ea5f297 Compare July 31, 2018 19:04
@jeffhostetler jeffhostetler merged commit ab82508 into microsoft:gvfs-2.18.0 Jul 31, 2018
kewillford pushed a commit to kewillford/git that referenced this pull request Sep 4, 2018
@jeffhostetler
find_vs_env.bat: Fix bin dir path used by msys
83972de 
Completely convert the pathname expoted in the %msvc_bin_dir_msys%
variable to MSYS format with forward slashes rather than a mixture
of forward and back slashes.

This solves an obscure problem observed by some developers:

    [...]
    http-push.c
        CC remote-curl.o
    remote-curl.c
        * new script parameters
        GEN git-instaweb
    sed: -e expression microsoft#7, char 155: invalid reference \2 on `s' command's RHS
    make: *** [Makefile:2023: git-instaweb] Error 1

Signed-off-by: Jeff Hostetler <jeffhost@microsoft.com>
kewillford pushed a commit to kewillford/git that referenced this pull request Sep 4, 2018
@jeffhostetler @dscho
find_vs_env.bat: Fix bin dir path used by msys
b75d84e 
Completely convert the pathname expoted in the %msvc_bin_dir_msys%
variable to MSYS format with forward slashes rather than a mixture
of forward and back slashes.

This solves an obscure problem observed by some developers:

    [...]
    http-push.c
        CC remote-curl.o
    remote-curl.c
        * new script parameters
        GEN git-instaweb
    sed: -e expression microsoft#7, char 155: invalid reference \2 on `s' command's RHS
    make: *** [Makefile:2023: git-instaweb] Error 1

Signed-off-by: Jeff Hostetler <jeffhost@microsoft.com>
kewillford pushed a commit to kewillford/git that referenced this pull request Sep 4, 2018
@jeffhostetler @jamill
find_vs_env.bat: Fix bin dir path used by msys
1311c8c 
Completely convert the pathname expoted in the %msvc_bin_dir_msys%
variable to MSYS format with forward slashes rather than a mixture
of forward and back slashes.

This solves an obscure problem observed by some developers:

    [...]
    http-push.c
        CC remote-curl.o
    remote-curl.c
        * new script parameters
        GEN git-instaweb
    sed: -e expression microsoft#7, char 155: invalid reference \2 on `s' command's RHS
    make: *** [Makefile:2023: git-instaweb] Error 1

Signed-off-by: Jeff Hostetler <jeffhost@microsoft.com>
kewillford pushed a commit to kewillford/git that referenced this pull request Sep 4, 2018
@jeffhostetler
Merge pull request microsoft#7 from jeffhostetler/gvfs-status-seriali…
c580952 
…ze-wait

status: deserialization wait
kewillford pushed a commit that referenced this pull request Sep 17, 2018
@jeffhostetler
Merge pull request #7 from jeffhostetler/gvfs-status-serialize-wait
41fc2e3 
status: deserialization wait
kewillford pushed a commit to kewillford/git that referenced this pull request Oct 9, 2018
@jeffhostetler @jamill
find_vs_env.bat: Fix bin dir path used by msys
cfa4938 
Completely convert the pathname expoted in the %msvc_bin_dir_msys%
variable to MSYS format with forward slashes rather than a mixture
of forward and back slashes.

This solves an obscure problem observed by some developers:

    [...]
    http-push.c
        CC remote-curl.o
    remote-curl.c
        * new script parameters
        GEN git-instaweb
    sed: -e expression microsoft#7, char 155: invalid reference \2 on `s' command's RHS
    make: *** [Makefile:2023: git-instaweb] Error 1

Signed-off-by: Jeff Hostetler <jeffhost@microsoft.com>
kewillford pushed a commit to kewillford/git that referenced this pull request Oct 9, 2018
@jeffhostetler @jamill
find_vs_env.bat: Fix bin dir path used by msys
2e41db1 
Completely convert the pathname expoted in the %msvc_bin_dir_msys%
variable to MSYS format with forward slashes rather than a mixture
of forward and back slashes.

This solves an obscure problem observed by some developers:

    [...]
    http-push.c
        CC remote-curl.o
    remote-curl.c
        * new script parameters
        GEN git-instaweb
    sed: -e expression microsoft#7, char 155: invalid reference \2 on `s' command's RHS
    make: *** [Makefile:2023: git-instaweb] Error 1

Signed-off-by: Jeff Hostetler <jeffhost@microsoft.com>
dscho pushed a commit that referenced this pull request Oct 10, 2018
@jeffhostetler @dscho
find_vs_env.bat: Fix bin dir path used by msys
b320842 
Completely convert the pathname expoted in the %msvc_bin_dir_msys%
variable to MSYS format with forward slashes rather than a mixture
of forward and back slashes.

This solves an obscure problem observed by some developers:

    [...]
    http-push.c
        CC remote-curl.o
    remote-curl.c
        * new script parameters
        GEN git-instaweb
    sed: -e expression #7, char 155: invalid reference \2 on `s' command's RHS
    make: *** [Makefile:2023: git-instaweb] Error 1

Signed-off-by: Jeff Hostetler <jeffhost@microsoft.com>
dscho pushed a commit that referenced this pull request Oct 12, 2018
@jeffhostetler @dscho
vcpkg: get MSVC dependencies with vcpkg rather than nuget
4ee8d34 
Dependencies such as cURL and OpenSSL are necessary to build and run
Git. Previously, we obtained those dependencies by fetching NuGet
packages.

However, it is notoriously hard to keep NuGet packages of C/C++
libraries up-to-date, as the toolsets for different Visual Studio
versions are different, and the NuGet packages would have to ship them
all.

That is the reason why the NuGet packages we use are quite old, and even
insecure in the case of cURL and OpenSSL (the versions contain known
security flaws that have been addressed by later versions for which no
NuGet packages are available).

The better way to handle this situation is to use the vcpkg system:
https://github.com/Microsoft/vcpkg

The idea is that a single Git repository contains enough supporting
files to build up-to-date versions of a large number of Open Source
libraries on demand, including cURL and OpenSSL.

We integrate this system via four new .bat files to

1) initialize the vcpkg system,
2) build the packages,
4) set up Git's Makefile system to find the build artifacts, and
3) copy the artifacts into the top-level directory

We now also completely convert the pathname exported in the
%msvc_bin_dir_msys% variable to MSYS format with forward slashes rather
than a mixture of forward and back slashes.

This solves an obscure problem observed by some developers:

    [...]
    http-push.c
        CC remote-curl.o
    remote-curl.c
        * new script parameters
        GEN git-instaweb
    sed: -e expression #7, char 155: invalid reference \2 on `s' command's RHS
    make: *** [Makefile:2023: git-instaweb] Error 1

Signed-off-by: Jeff Hostetler <jeffhost@microsoft.com>
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
dscho pushed a commit that referenced this pull request Oct 15, 2018
@jeffhostetler @dscho
Merge pull request #7 from jeffhostetler/gvfs-status-serialize-wait
5b3c03d 
status: deserialization wait
dscho pushed a commit that referenced this pull request Oct 18, 2018
@jeffhostetler @dscho
Merge pull request #7 from jeffhostetler/gvfs-status-serialize-wait
0c41f25 
status: deserialization wait
dscho pushed a commit that referenced this pull request Nov 21, 2018
@jeffhostetler @dscho
Merge pull request #7 from jeffhostetler/gvfs-status-serialize-wait
198de10 
status: deserialization wait
dscho pushed a commit that referenced this pull request Nov 26, 2018
@jeffhostetler @dscho
Merge pull request #7 from jeffhostetler/gvfs-status-serialize-wait
92735dc 
status: deserialization wait
dscho pushed a commit that referenced this pull request Dec 2, 2018
@jeffhostetler @dscho
Merge pull request #7 from jeffhostetler/gvfs-status-serialize-wait
12eda74 
status: deserialization wait
dscho pushed a commit that referenced this pull request Dec 10, 2018
@jeffhostetler @dscho
Merge pull request #7 from jeffhostetler/gvfs-status-serialize-wait
4bf9218 
status: deserialization wait
dscho pushed a commit that referenced this pull request Jun 3, 2019
@jeffhostetler @dscho
Merge pull request #7 from jeffhostetler/gvfs-status-serialize-wait
cfce4ec 
status: deserialization wait
dscho pushed a commit that referenced this pull request Jun 8, 2019
@jeffhostetler @dscho
Merge pull request #7 from jeffhostetler/gvfs-status-serialize-wait
f48f420 
status: deserialization wait
@jeffhostetler jeffhostetler deleted the gvfs-status-serialize-wait branch July 1, 2019 18:31
dscho pushed a commit that referenced this pull request Aug 17, 2019
@jeffhostetler @dscho
Merge pull request #7 from jeffhostetler/gvfs-status-serialize-wait
d0aa714 
status: deserialization wait
derrickstolee pushed a commit that referenced this pull request Oct 23, 2019
@jeffhostetler @derrickstolee
Merge pull request #7 from jeffhostetler/gvfs-status-serialize-wait
f9410e1 
status: deserialization wait
derrickstolee pushed a commit that referenced this pull request Nov 4, 2019
@jeffhostetler @derrickstolee
Merge pull request #7 from jeffhostetler/gvfs-status-serialize-wait
dee9aa3 
status: deserialization wait
derrickstolee pushed a commit that referenced this pull request Jan 14, 2020
@jeffhostetler @derrickstolee
Merge pull request #7 from jeffhostetler/gvfs-status-serialize-wait
b87f0fa 
status: deserialization wait
derrickstolee pushed a commit that referenced this pull request Feb 21, 2020
@jeffhostetler @derrickstolee
Merge pull request #7 from jeffhostetler/gvfs-status-serialize-wait
5232c36 
status: deserialization wait
derrickstolee pushed a commit that referenced this pull request Mar 17, 2020
@jeffhostetler @derrickstolee
Merge pull request #7 from jeffhostetler/gvfs-status-serialize-wait
d9aa45e 
status: deserialization wait
derrickstolee pushed a commit that referenced this pull request Mar 23, 2020
@jeffhostetler @derrickstolee
Merge pull request #7 from jeffhostetler/gvfs-status-serialize-wait
58f09e4 
status: deserialization wait
dscho pushed a commit that referenced this pull request May 20, 2020
@jeffhostetler @dscho
Merge pull request #7 from jeffhostetler/gvfs-status-serialize-wait
9629ea4 
status: deserialization wait
dscho pushed a commit that referenced this pull request May 20, 2020
@jeffhostetler @dscho
Merge pull request #7 from jeffhostetler/gvfs-status-serialize-wait
496d73c 
status: deserialization wait
derrickstolee pushed a commit that referenced this pull request Mar 29, 2021
@ahunt @gitster
fsmonitor: avoid global-buffer-overflow READ when checking trivial re…
097ea2c 
…sponse

query_result can be be an empty strbuf (STRBUF_INIT) - in that case
trying to read 3 bytes triggers a buffer overflow read (as
query_result.buf = '\0').

Therefore we need to check query_result's length before trying to read 3
bytes.

This overflow was introduced in:
  940b94f (fsmonitor: log invocation of FSMonitor hook to trace2, 2021-02-03)
It was found when running the test-suite against ASAN, and can be most
easily reproduced with the following command:

make GIT_TEST_OPTS="-v" DEFAULT_TEST_TARGET="t7519-status-fsmonitor.sh" \
SANITIZE=address DEVELOPER=1 test

==2235==ERROR: AddressSanitizer: global-buffer-overflow on address 0x0000019e6e5e at pc 0x00000043745c bp 0x7fffd382c520 sp 0x7fffd382bcc8
READ of size 3 at 0x0000019e6e5e thread T0
    #0 0x43745b in MemcmpInterceptorCommon(void*, int (*)(void const*, void const*, unsigned long), void const*, void const*, unsigned long) /home/abuild/rpmbuild/BUILD/llvm-11.0.0.src/build/../projects/compiler-rt/lib/asan/../sanitizer_common/sanitizer_common_interceptors.inc:842:7
    #1 0x43786d in bcmp /home/abuild/rpmbuild/BUILD/llvm-11.0.0.src/build/../projects/compiler-rt/lib/asan/../sanitizer_common/sanitizer_common_interceptors.inc:887:10
    #2 0x80b146 in fsmonitor_is_trivial_response /home/ahunt/oss-fuzz/git/fsmonitor.c:192:10
    #3 0x80b146 in query_fsmonitor /home/ahunt/oss-fuzz/git/fsmonitor.c:175:7
    #4 0x80a749 in refresh_fsmonitor /home/ahunt/oss-fuzz/git/fsmonitor.c:267:21
    #5 0x80bad1 in tweak_fsmonitor /home/ahunt/oss-fuzz/git/fsmonitor.c:429:4
    #6 0x90f040 in read_index_from /home/ahunt/oss-fuzz/git/read-cache.c:2321:3
    #7 0x8e5d08 in repo_read_index_preload /home/ahunt/oss-fuzz/git/preload-index.c:164:15
    #8 0x52dd45 in prepare_index /home/ahunt/oss-fuzz/git/builtin/commit.c:363:6
    #9 0x52a188 in cmd_commit /home/ahunt/oss-fuzz/git/builtin/commit.c:1588:15
    #10 0x4ce77e in run_builtin /home/ahunt/oss-fuzz/git/git.c:453:11
    #11 0x4ccb18 in handle_builtin /home/ahunt/oss-fuzz/git/git.c:704:3
    #12 0x4cb01c in run_argv /home/ahunt/oss-fuzz/git/git.c:771:4
    #13 0x4cb01c in cmd_main /home/ahunt/oss-fuzz/git/git.c:902:19
    #14 0x6aca8d in main /home/ahunt/oss-fuzz/git/common-main.c:52:11
    #15 0x7fb027bf5349 in __libc_start_main (/lib64/libc.so.6+0x24349)
    #16 0x4206b9 in _start /home/abuild/rpmbuild/BUILD/glibc-2.26/csu/../sysdeps/x86_64/start.S:120

0x0000019e6e5e is located 2 bytes to the left of global variable 'strbuf_slopbuf' defined in 'strbuf.c:51:6' (0x19e6e60) of size 1
  'strbuf_slopbuf' is ascii string ''
0x0000019e6e5e is located 126 bytes to the right of global variable 'signals' defined in 'sigchain.c:11:31' (0x19e6be0) of size 512
SUMMARY: AddressSanitizer: global-buffer-overflow /home/abuild/rpmbuild/BUILD/llvm-11.0.0.src/build/../projects/compiler-rt/lib/asan/../sanitizer_common/sanitizer_common_interceptors.inc:842:7 in MemcmpInterceptorCommon(void*, int (*)(void const*, void const*, unsigned long), void const*, void const*, unsigned long)
Shadow bytes around the buggy address:
  0x000080334d70: f9 f9 f9 f9 00 f9 f9 f9 f9 f9 f9 f9 00 00 00 00
  0x000080334d80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x000080334d90: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x000080334da0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x000080334db0: 00 00 00 00 00 00 00 00 00 00 00 00 f9 f9 f9 f9
=>0x000080334dc0: f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9[f9]01 f9 f9 f9
  0x000080334dd0: f9 f9 f9 f9 03 f9 f9 f9 f9 f9 f9 f9 02 f9 f9 f9
  0x000080334de0: f9 f9 f9 f9 00 f9 f9 f9 f9 f9 f9 f9 04 f9 f9 f9
  0x000080334df0: f9 f9 f9 f9 01 f9 f9 f9 f9 f9 f9 f9 00 00 00 00
  0x000080334e00: f9 f9 f9 f9 00 00 00 00 f9 f9 f9 f9 01 f9 f9 f9
  0x000080334e10: f9 f9 f9 f9 04 f9 f9 f9 f9 f9 f9 f9 00 f9 f9 f9
Shadow byte legend (one shadow byte represents 8 application bytes):
  Addressable:           00
  Partially addressable: 01 02 03 04 05 06 07
  Heap left redzone:       fa
  Freed heap region:       fd
  Stack left redzone:      f1
  Stack mid redzone:       f2
  Stack right redzone:     f3
  Stack after return:      f5
  Stack use after scope:   f8
  Global redzone:          f9
  Global init order:       f6
  Poisoned by user:        f7
  Container overflow:      fc
  Array cookie:            ac
  Intra object redzone:    bb
  ASan internal:           fe
  Left alloca redzone:     ca
  Right alloca redzone:    cb
  Shadow gap:              cc

Signed-off-by: Andrzej Hunt <ajrhunt@google.com>
Acked-by: Jeff Hostetler <jeffhost@microsoft.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
ldennington pushed a commit to ldennington/git that referenced this pull request Jun 2, 2021
@ahunt @gitster
symbolic-ref: don't leak shortened refname in check_symref()
f63b888 
shorten_unambiguous_ref() returns an allocated string. We have to
track it separately from the const refname.

This leak has existed since:
9ab55da (git symbolic-ref --delete $symref, 2012-10-21)

This leak was found when running t0001 with LSAN, see also LSAN output
below:

Direct leak of 19 byte(s) in 1 object(s) allocated from:
    #0 0x486514 in strdup /home/abuild/rpmbuild/BUILD/llvm-11.0.0.src/build/../projects/compiler-rt/lib/asan/asan_interceptors.cpp:452:3
    microsoft#1 0x9ab048 in xstrdup /home/ahunt/oss-fuzz/git/wrapper.c:29:14
    microsoft#2 0x8b452f in refs_shorten_unambiguous_ref /home/ahunt/oss-fuzz/git/refs.c
    microsoft#3 0x8b47e8 in shorten_unambiguous_ref /home/ahunt/oss-fuzz/git/refs.c:1287:9
    microsoft#4 0x679fce in check_symref /home/ahunt/oss-fuzz/git/builtin/symbolic-ref.c:28:14
    microsoft#5 0x679ad8 in cmd_symbolic_ref /home/ahunt/oss-fuzz/git/builtin/symbolic-ref.c:70:9
    microsoft#6 0x4cd60d in run_builtin /home/ahunt/oss-fuzz/git/git.c:453:11
    microsoft#7 0x4cb2da in handle_builtin /home/ahunt/oss-fuzz/git/git.c:704:3
    microsoft#8 0x4ccc37 in run_argv /home/ahunt/oss-fuzz/git/git.c:771:4
    microsoft#9 0x4cac29 in cmd_main /home/ahunt/oss-fuzz/git/git.c:902:19
    microsoft#10 0x69cc6e in main /home/ahunt/oss-fuzz/git/common-main.c:52:11
    microsoft#11 0x7f98388a4349 in __libc_start_main (/lib64/libc.so.6+0x24349)

Signed-off-by: Andrzej Hunt <ajrhunt@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
ldennington pushed a commit to ldennington/git that referenced this pull request Jun 2, 2021
@ahunt @gitster
reset: free instead of leaking unneeded ref
e901de6 
dwim_ref() allocs a new string into ref. Instead of setting to NULL to
discard it, we can FREE_AND_NULL.

This leak appears to have been introduced in:
4cf76f6 (builtin/reset: compute checkout metadata for reset, 2020-03-16)

This leak was found when running t0001 with LSAN, see also LSAN output below:

Direct leak of 5 byte(s) in 1 object(s) allocated from:
    #0 0x486514 in strdup /home/abuild/rpmbuild/BUILD/llvm-11.0.0.src/build/../projects/compiler-rt/lib/asan/asan_interceptors.cpp:452:3
    microsoft#1 0x9a7108 in xstrdup /home/ahunt/oss-fuzz/git/wrapper.c:29:14
    microsoft#2 0x8add6b in expand_ref /home/ahunt/oss-fuzz/git/refs.c:670:12
    microsoft#3 0x8ad777 in repo_dwim_ref /home/ahunt/oss-fuzz/git/refs.c:644:22
    microsoft#4 0x6394af in dwim_ref /home/ahunt/oss-fuzz/git/./refs.h:162:9
    microsoft#5 0x637e5c in cmd_reset /home/ahunt/oss-fuzz/git/builtin/reset.c:426:4
    microsoft#6 0x4cd60d in run_builtin /home/ahunt/oss-fuzz/git/git.c:453:11
    microsoft#7 0x4cb2da in handle_builtin /home/ahunt/oss-fuzz/git/git.c:704:3
    microsoft#8 0x4ccc37 in run_argv /home/ahunt/oss-fuzz/git/git.c:771:4
    microsoft#9 0x4cac29 in cmd_main /home/ahunt/oss-fuzz/git/git.c:902:19
    microsoft#10 0x69c5ce in main /home/ahunt/oss-fuzz/git/common-main.c:52:11
    microsoft#11 0x7f57ebb9d349 in __libc_start_main (/lib64/libc.so.6+0x24349)

Signed-off-by: Andrzej Hunt <ajrhunt@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
ldennington pushed a commit to ldennington/git that referenced this pull request Jun 2, 2021
@ahunt @gitster
clone: free or UNLEAK further pointers when finished
0c45427 
Most of these pointers can safely be freed when cmd_clone() completes,
therefore we make sure to free them. The one exception is that we
have to UNLEAK(repo) because it can point either to argv[0], or a
malloc'd string returned by absolute_pathdup().

We also have to free(path) in the middle of cmd_clone(): later during
cmd_clone(), path is unconditionally overwritten with a different path,
triggering a leak. Freeing the first path immediately after use (but
only in the case where it contains data) seems like the cleanest
solution, as opposed to freeing it unconditionally before path is reused
for another path. This leak appears to have been introduced in:
  f38aa83 (use local cloning if insteadOf makes a local URL, 2014-07-17)

These leaks were found when running t0001 with LSAN, see also an excerpt
of the LSAN output below (the full list is omitted because it's far too
long, and mostly consists of indirect leakage of members of the refs we
are freeing).

Direct leak of 178 byte(s) in 1 object(s) allocated from:
    #0 0x49a53d in malloc /home/abuild/rpmbuild/BUILD/llvm-11.0.0.src/build/../projects/compiler-rt/lib/asan/asan_malloc_linux.cpp:145:3
    microsoft#1 0x9a6ff4 in do_xmalloc /home/ahunt/oss-fuzz/git/wrapper.c:41:8
    microsoft#2 0x9a6fca in xmalloc /home/ahunt/oss-fuzz/git/wrapper.c:62:9
    microsoft#3 0x8ce296 in copy_ref /home/ahunt/oss-fuzz/git/remote.c:885:8
    microsoft#4 0x8d2ebd in guess_remote_head /home/ahunt/oss-fuzz/git/remote.c:2215:10
    microsoft#5 0x51d0c5 in cmd_clone /home/ahunt/oss-fuzz/git/builtin/clone.c:1308:4
    microsoft#6 0x4cd60d in run_builtin /home/ahunt/oss-fuzz/git/git.c:453:11
    microsoft#7 0x4cb2da in handle_builtin /home/ahunt/oss-fuzz/git/git.c:704:3
    microsoft#8 0x4ccc37 in run_argv /home/ahunt/oss-fuzz/git/git.c:771:4
    microsoft#9 0x4cac29 in cmd_main /home/ahunt/oss-fuzz/git/git.c:902:19
    microsoft#10 0x69c45e in main /home/ahunt/oss-fuzz/git/common-main.c:52:11
    microsoft#11 0x7f6a459d5349 in __libc_start_main (/lib64/libc.so.6+0x24349)

Direct leak of 165 byte(s) in 1 object(s) allocated from:
    #0 0x49a53d in malloc /home/abuild/rpmbuild/BUILD/llvm-11.0.0.src/build/../projects/compiler-rt/lib/asan/asan_malloc_linux.cpp:145:3
    microsoft#1 0x9a6fc4 in do_xmalloc /home/ahunt/oss-fuzz/git/wrapper.c:41:8
    microsoft#2 0x9a6f9a in xmalloc /home/ahunt/oss-fuzz/git/wrapper.c:62:9
    microsoft#3 0x8ce266 in copy_ref /home/ahunt/oss-fuzz/git/remote.c:885:8
    microsoft#4 0x51e9bd in wanted_peer_refs /home/ahunt/oss-fuzz/git/builtin/clone.c:574:21
    microsoft#5 0x51cfe1 in cmd_clone /home/ahunt/oss-fuzz/git/builtin/clone.c:1284:17
    microsoft#6 0x4cd60d in run_builtin /home/ahunt/oss-fuzz/git/git.c:453:11
    microsoft#7 0x4cb2da in handle_builtin /home/ahunt/oss-fuzz/git/git.c:704:3
    microsoft#8 0x4ccc37 in run_argv /home/ahunt/oss-fuzz/git/git.c:771:4
    microsoft#9 0x4cac29 in cmd_main /home/ahunt/oss-fuzz/git/git.c:902:19
    microsoft#10 0x69c42e in main /home/ahunt/oss-fuzz/git/common-main.c:52:11
    microsoft#11 0x7f8fef0c2349 in __libc_start_main (/lib64/libc.so.6+0x24349)

Direct leak of 178 byte(s) in 1 object(s) allocated from:
    #0 0x49a53d in malloc /home/abuild/rpmbuild/BUILD/llvm-11.0.0.src/build/../projects/compiler-rt/lib/asan/asan_malloc_linux.cpp:145:3
    microsoft#1 0x9a6ff4 in do_xmalloc /home/ahunt/oss-fuzz/git/wrapper.c:41:8
    microsoft#2 0x9a6fca in xmalloc /home/ahunt/oss-fuzz/git/wrapper.c:62:9
    microsoft#3 0x8ce296 in copy_ref /home/ahunt/oss-fuzz/git/remote.c:885:8
    microsoft#4 0x8d2ebd in guess_remote_head /home/ahunt/oss-fuzz/git/remote.c:2215:10
    microsoft#5 0x51d0c5 in cmd_clone /home/ahunt/oss-fuzz/git/builtin/clone.c:1308:4
    microsoft#6 0x4cd60d in run_builtin /home/ahunt/oss-fuzz/git/git.c:453:11
    microsoft#7 0x4cb2da in handle_builtin /home/ahunt/oss-fuzz/git/git.c:704:3
    microsoft#8 0x4ccc37 in run_argv /home/ahunt/oss-fuzz/git/git.c:771:4
    microsoft#9 0x4cac29 in cmd_main /home/ahunt/oss-fuzz/git/git.c:902:19
    microsoft#10 0x69c45e in main /home/ahunt/oss-fuzz/git/common-main.c:52:11
    microsoft#11 0x7f6a459d5349 in __libc_start_main (/lib64/libc.so.6+0x24349)

Direct leak of 165 byte(s) in 1 object(s) allocated from:
    #0 0x49a6b2 in calloc /home/abuild/rpmbuild/BUILD/llvm-11.0.0.src/build/../projects/compiler-rt/lib/asan/asan_malloc_linux.cpp:154:3
    microsoft#1 0x9a72f2 in xcalloc /home/ahunt/oss-fuzz/git/wrapper.c:140:8
    microsoft#2 0x8ce203 in alloc_ref_with_prefix /home/ahunt/oss-fuzz/git/remote.c:867:20
    microsoft#3 0x8ce1a2 in alloc_ref /home/ahunt/oss-fuzz/git/remote.c:875:9
    microsoft#4 0x72f63e in process_ref_v2 /home/ahunt/oss-fuzz/git/connect.c:426:8
    microsoft#5 0x72f21a in get_remote_refs /home/ahunt/oss-fuzz/git/connect.c:525:8
    microsoft#6 0x979ab7 in handshake /home/ahunt/oss-fuzz/git/transport.c:305:4
    microsoft#7 0x97872d in get_refs_via_connect /home/ahunt/oss-fuzz/git/transport.c:339:9
    microsoft#8 0x9774b5 in transport_get_remote_refs /home/ahunt/oss-fuzz/git/transport.c:1388:4
    microsoft#9 0x51cf80 in cmd_clone /home/ahunt/oss-fuzz/git/builtin/clone.c:1271:9
    microsoft#10 0x4cd60d in run_builtin /home/ahunt/oss-fuzz/git/git.c:453:11
    microsoft#11 0x4cb2da in handle_builtin /home/ahunt/oss-fuzz/git/git.c:704:3
    microsoft#12 0x4ccc37 in run_argv /home/ahunt/oss-fuzz/git/git.c:771:4
    microsoft#13 0x4cac29 in cmd_main /home/ahunt/oss-fuzz/git/git.c:902:19
    microsoft#14 0x69c45e in main /home/ahunt/oss-fuzz/git/common-main.c:52:11
    microsoft#15 0x7f6a459d5349 in __libc_start_main (/lib64/libc.so.6+0x24349)

Direct leak of 105 byte(s) in 1 object(s) allocated from:
    #0 0x49a859 in realloc /home/abuild/rpmbuild/BUILD/llvm-11.0.0.src/build/../projects/compiler-rt/lib/asan/asan_malloc_linux.cpp:164:3
    microsoft#1 0x9a71f6 in xrealloc /home/ahunt/oss-fuzz/git/wrapper.c:126:8
    microsoft#2 0x93622d in strbuf_grow /home/ahunt/oss-fuzz/git/strbuf.c:98:2
    microsoft#3 0x937a73 in strbuf_addch /home/ahunt/oss-fuzz/git/./strbuf.h:231:3
    microsoft#4 0x939fcd in strbuf_add_absolute_path /home/ahunt/oss-fuzz/git/strbuf.c:911:4
    microsoft#5 0x69d3ce in absolute_pathdup /home/ahunt/oss-fuzz/git/abspath.c:261:2
    microsoft#6 0x51c688 in cmd_clone /home/ahunt/oss-fuzz/git/builtin/clone.c:1021:10
    microsoft#7 0x4cd60d in run_builtin /home/ahunt/oss-fuzz/git/git.c:453:11
    microsoft#8 0x4cb2da in handle_builtin /home/ahunt/oss-fuzz/git/git.c:704:3
    microsoft#9 0x4ccc37 in run_argv /home/ahunt/oss-fuzz/git/git.c:771:4
    microsoft#10 0x4cac29 in cmd_main /home/ahunt/oss-fuzz/git/git.c:902:19
    microsoft#11 0x69c45e in main /home/ahunt/oss-fuzz/git/common-main.c:52:11
    microsoft#12 0x7f6a459d5349 in __libc_start_main (/lib64/libc.so.6+0x24349)

Signed-off-by: Andrzej Hunt <ajrhunt@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
ldennington pushed a commit to ldennington/git that referenced this pull request Jun 2, 2021
@ahunt @gitster
worktree: fix leak in dwim_branch()
aa1b639 
Make sure that we release the temporary strbuf during dwim_branch() for
all codepaths (and not just for the early return).

This leak appears to have been introduced in:
  f60a7b7 (worktree: teach "add" to check out existing branches, 2018-04-24)

Note that UNLEAK(branchname) is still needed: the returned result is
used in add(), and is stored in a pointer which is used to point at one
of:
  - a string literal ("HEAD")
  - member of argv (whatever the user specified in their invocation)
  - or our newly allocated string returned from dwim_branch()
Fixing the branchname leak isn't impossible, but does not seem
worthwhile given that add() is called directly from cmd_main(), and
cmd_main() returns immediately thereafter - UNLEAK is good enough.

This leak was found when running t0001 with LSAN, see also LSAN output
below:

Direct leak of 60 byte(s) in 1 object(s) allocated from:
    #0 0x49a859 in realloc /home/abuild/rpmbuild/BUILD/llvm-11.0.0.src/build/../projects/compiler-rt/lib/asan/asan_malloc_linux.cpp:164:3
    microsoft#1 0x9ab076 in xrealloc /home/ahunt/oss-fuzz/git/wrapper.c:126:8
    microsoft#2 0x939fcd in strbuf_grow /home/ahunt/oss-fuzz/git/strbuf.c:98:2
    microsoft#3 0x93af53 in strbuf_splice /home/ahunt/oss-fuzz/git/strbuf.c:239:3
    microsoft#4 0x83559a in strbuf_check_branch_ref /home/ahunt/oss-fuzz/git/object-name.c:1593:2
    microsoft#5 0x6988b9 in dwim_branch /home/ahunt/oss-fuzz/git/builtin/worktree.c:454:20
    microsoft#6 0x695f8f in add /home/ahunt/oss-fuzz/git/builtin/worktree.c:525:19
    microsoft#7 0x694a04 in cmd_worktree /home/ahunt/oss-fuzz/git/builtin/worktree.c:1036:10
    microsoft#8 0x4cd60d in run_builtin /home/ahunt/oss-fuzz/git/git.c:453:11
    microsoft#9 0x4cb2da in handle_builtin /home/ahunt/oss-fuzz/git/git.c:704:3
    microsoft#10 0x4ccc37 in run_argv /home/ahunt/oss-fuzz/git/git.c:771:4
    microsoft#11 0x4cac29 in cmd_main /home/ahunt/oss-fuzz/git/git.c:902:19
    microsoft#12 0x69caee in main /home/ahunt/oss-fuzz/git/common-main.c:52:11
    microsoft#13 0x7f7b7dd10349 in __libc_start_main (/lib64/libc.so.6+0x24349)

Signed-off-by: Andrzej Hunt <ajrhunt@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
ldennington pushed a commit to ldennington/git that referenced this pull request Jun 2, 2021
@ahunt @gitster
init: remove git_init_db_config() while fixing leaks
e4de450 
The primary goal of this change is to stop leaking init_db_template_dir.
This leak can happen because:
 1. git_init_db_config() allocates new memory into init_db_template_dir
    without first freeing the existing value.
 2. init_db_template_dir might already contain data, either because:
  2.1 git_config() can be invoked twice with this callback in a single
      process - at least 2 allocations are likely.
  2.2 A single git_config() allocation can invoke the callback multiple
      times for a given key (see further explanation in the function
      docs) - each of those calls will trigger another leak.

The simplest fix for the leak would be to free(init_db_template_dir)
before overwriting it. Instead we choose to convert to fetching
init.templatedir via git_config_get_value() as that is more explicit,
more efficient, and avoids allocations (the returned result is owned by
the config cache, so we aren't responsible for freeing it).

If we remove init_db_template_dir, git_init_db_config() ends up being
responsible only for forwarding core.* config values to
platform_core_config(). However platform_core_config() already ignores
non-core.* config values, so we can safely remove git_init_db_config()
and invoke git_config() directly with platform_core_config() as the
callback.

The platform_core_config forwarding was originally added in:
  2878533 (mingw: respect core.hidedotfiles = false in git-init again, 2019-03-11
And I suspect the potential for a leak existed since the original
implementation of git_init_db_config in:
  90b4518 (Add `init.templatedir` configuration variable., 2010-02-17)

LSAN output from t0001:

Direct leak of 73 byte(s) in 1 object(s) allocated from:
    #0 0x49a859 in realloc /home/abuild/rpmbuild/BUILD/llvm-11.0.0.src/build/../projects/compiler-rt/lib/asan/asan_malloc_linux.cpp:164:3
    microsoft#1 0x9a7276 in xrealloc /home/ahunt/oss-fuzz/git/wrapper.c:126:8
    microsoft#2 0x9362ad in strbuf_grow /home/ahunt/oss-fuzz/git/strbuf.c:98:2
    microsoft#3 0x936eaa in strbuf_add /home/ahunt/oss-fuzz/git/strbuf.c:295:2
    microsoft#4 0x868112 in strbuf_addstr /home/ahunt/oss-fuzz/git/./strbuf.h:304:2
    microsoft#5 0x86a8ad in expand_user_path /home/ahunt/oss-fuzz/git/path.c:758:2
    microsoft#6 0x720bb1 in git_config_pathname /home/ahunt/oss-fuzz/git/config.c:1287:10
    microsoft#7 0x5960e2 in git_init_db_config /home/ahunt/oss-fuzz/git/builtin/init-db.c:161:11
    microsoft#8 0x7255b8 in configset_iter /home/ahunt/oss-fuzz/git/config.c:1982:7
    microsoft#9 0x7253fc in repo_config /home/ahunt/oss-fuzz/git/config.c:2311:2
    microsoft#10 0x725ca7 in git_config /home/ahunt/oss-fuzz/git/config.c:2399:2
    microsoft#11 0x593e8d in create_default_files /home/ahunt/oss-fuzz/git/builtin/init-db.c:225:2
    microsoft#12 0x5935c6 in init_db /home/ahunt/oss-fuzz/git/builtin/init-db.c:449:11
    microsoft#13 0x59588e in cmd_init_db /home/ahunt/oss-fuzz/git/builtin/init-db.c:714:9
    microsoft#14 0x4cd60d in run_builtin /home/ahunt/oss-fuzz/git/git.c:453:11
    microsoft#15 0x4cb2da in handle_builtin /home/ahunt/oss-fuzz/git/git.c:704:3
    microsoft#16 0x4ccc37 in run_argv /home/ahunt/oss-fuzz/git/git.c:771:4
    microsoft#17 0x4cac29 in cmd_main /home/ahunt/oss-fuzz/git/git.c:902:19
    microsoft#18 0x69c4de in main /home/ahunt/oss-fuzz/git/common-main.c:52:11
    microsoft#19 0x7f23552d6349 in __libc_start_main (/lib64/libc.so.6+0x24349)

Signed-off-by: Andrzej Hunt <ajrhunt@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
ldennington pushed a commit to ldennington/git that referenced this pull request Jun 2, 2021
@ahunt @gitster
parse-options: don't leak alias help messages
64cc539 
preprocess_options() allocates new strings for help messages for
OPTION_ALIAS. Therefore we also need to clean those help messages up
when freeing the returned options.

First introduced in:
  7c28058 (parse-options: teach "git cmd -h" to show alias as alias, 2020-03-16)

The preprocessed options themselves no longer contain any indication
that a given option is/was an alias - therefore we add a new flag to
indicate former aliases. (An alternative approach would be to look back
at the original options to determine which options are aliases - but
that seems like a fragile approach. Or we could even look at the
alias_groups list - which might be less fragile, but would be slower
as it requires nested looping.)

As far as I can tell, parse_options() is only ever used once per
command, and the help messages are small - hence this leak has very
little impact.

This leak was found while running t0001. LSAN output can be found below:

Direct leak of 65 byte(s) in 1 object(s) allocated from:
    #0 0x49a859 in realloc /home/abuild/rpmbuild/BUILD/llvm-11.0.0.src/build/../projects/compiler-rt/lib/asan/asan_malloc_linux.cpp:164:3
    microsoft#1 0x9aae36 in xrealloc /home/ahunt/oss-fuzz/git/wrapper.c:126:8
    microsoft#2 0x939d8d in strbuf_grow /home/ahunt/oss-fuzz/git/strbuf.c:98:2
    microsoft#3 0x93b936 in strbuf_vaddf /home/ahunt/oss-fuzz/git/strbuf.c:392:3
    microsoft#4 0x93b7ff in strbuf_addf /home/ahunt/oss-fuzz/git/strbuf.c:333:2
    microsoft#5 0x86747e in preprocess_options /home/ahunt/oss-fuzz/git/parse-options.c:666:3
    microsoft#6 0x866ed2 in parse_options /home/ahunt/oss-fuzz/git/parse-options.c:847:17
    microsoft#7 0x51c4a7 in cmd_clone /home/ahunt/oss-fuzz/git/builtin/clone.c:989:9
    microsoft#8 0x4cd60d in run_builtin /home/ahunt/oss-fuzz/git/git.c:453:11
    microsoft#9 0x4cb2da in handle_builtin /home/ahunt/oss-fuzz/git/git.c:704:3
    microsoft#10 0x4ccc37 in run_argv /home/ahunt/oss-fuzz/git/git.c:771:4
    microsoft#11 0x4cac29 in cmd_main /home/ahunt/oss-fuzz/git/git.c:902:19
    microsoft#12 0x69c9fe in main /home/ahunt/oss-fuzz/git/common-main.c:52:11
    microsoft#13 0x7fdac42d4349 in __libc_start_main (/lib64/libc.so.6+0x24349)

Signed-off-by: Andrzej Hunt <ajrhunt@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
ldennington pushed a commit to ldennington/git that referenced this pull request Jun 2, 2021
@ahunt @gitster
transport: also free remote_refs in transport_disconnect()
68ffe09 
transport_get_remote_refs() can populate the transport struct's
remote_refs. transport_disconnect() is already responsible for most of
transport's cleanup - therefore we also take care of freeing remote_refs
there.

There are 2 locations where transport_disconnect() is called before
we're done using the returned remote_refs. This patch changes those
callsites to only call transport_disconnect() after the returned refs
are no longer being used - which is necessary to safely be able to
free remote_refs during transport_disconnect().

This commit fixes the following leak which was found while running
t0000, but is expected to also fix the same pattern of leak in all
locations that use transport_get_remote_refs():

Direct leak of 165 byte(s) in 1 object(s) allocated from:
    #0 0x49a6b2 in calloc /home/abuild/rpmbuild/BUILD/llvm-11.0.0.src/build/../projects/compiler-rt/lib/asan/asan_malloc_linux.cpp:154:3
    microsoft#1 0x9a72f2 in xcalloc /home/ahunt/oss-fuzz/git/wrapper.c:140:8
    microsoft#2 0x8ce203 in alloc_ref_with_prefix /home/ahunt/oss-fuzz/git/remote.c:867:20
    microsoft#3 0x8ce1a2 in alloc_ref /home/ahunt/oss-fuzz/git/remote.c:875:9
    microsoft#4 0x72f63e in process_ref_v2 /home/ahunt/oss-fuzz/git/connect.c:426:8
    microsoft#5 0x72f21a in get_remote_refs /home/ahunt/oss-fuzz/git/connect.c:525:8
    microsoft#6 0x979ab7 in handshake /home/ahunt/oss-fuzz/git/transport.c:305:4
    microsoft#7 0x97872d in get_refs_via_connect /home/ahunt/oss-fuzz/git/transport.c:339:9
    microsoft#8 0x9774b5 in transport_get_remote_refs /home/ahunt/oss-fuzz/git/transport.c:1388:4
    microsoft#9 0x51cf80 in cmd_clone /home/ahunt/oss-fuzz/git/builtin/clone.c:1271:9
    microsoft#10 0x4cd60d in run_builtin /home/ahunt/oss-fuzz/git/git.c:453:11
    microsoft#11 0x4cb2da in handle_builtin /home/ahunt/oss-fuzz/git/git.c:704:3
    microsoft#12 0x4ccc37 in run_argv /home/ahunt/oss-fuzz/git/git.c:771:4
    microsoft#13 0x4cac29 in cmd_main /home/ahunt/oss-fuzz/git/git.c:902:19
    microsoft#14 0x69c45e in main /home/ahunt/oss-fuzz/git/common-main.c:52:11
    microsoft#15 0x7f6a459d5349 in __libc_start_main (/lib64/libc.so.6+0x24349)

Signed-off-by: Andrzej Hunt <ajrhunt@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@jamill jamill jamill approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@jamill jamill

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@jeffhostetler @jamill