Skip to content

Update contrib/completion/git-completion.bash#17

Closed
mathieug wants to merge 1 commit intogit:masterfrom
mathieug:patch-1
Closed

Update contrib/completion/git-completion.bash#17
mathieug wants to merge 1 commit intogit:masterfrom
mathieug:patch-1

Conversation

@mathieug
Copy link

No description provided.

@vladimir-vg
Copy link

well, at top of this repo written: "This is a publish-only repository and all pull requests are ignored. Please follow Documentation/SubmittingPatches procedure for any of your improvements."

So I think your try to push your pull-request are hopeless.

@peff peff closed this May 11, 2012
ahunt added a commit to ahunt/git that referenced this pull request Mar 5, 2021
init_db_config_path could be leaked because:
1. git_init_db_config() allocates new data into init_db_config_path on every
   invocation without freeing preexisting data.
2. git_init_db_config() can be called multiple times for a single git_config()
   invocation (see docs on git_config() for context).
Furthermore, until very recently, git_config(git_init_db_config(), ...) could
have been invoked twice in a single process as git_init_db_config() used to
be used to handle core.* config settings. This was changed in a previous
patch in this series.

Freeing the existing value in git_init_db_config() would be the least intrusive
fix, however switching to git_config_get_value() simplifies the code further by
letting us remove the static pointer (furthermore, the returned data is owned by
the config cache, saving us from having to worry about freeing it later).

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
    #1 0x9a7276 in xrealloc /home/ahunt/oss-fuzz/git/wrapper.c:126:8
    #2 0x9362ad in strbuf_grow /home/ahunt/oss-fuzz/git/strbuf.c:98:2
    #3 0x936eaa in strbuf_add /home/ahunt/oss-fuzz/git/strbuf.c:295:2
    #4 0x868112 in strbuf_addstr /home/ahunt/oss-fuzz/git/./strbuf.h:304:2
    #5 0x86a8ad in expand_user_path /home/ahunt/oss-fuzz/git/path.c:758:2
    #6 0x720bb1 in git_config_pathname /home/ahunt/oss-fuzz/git/config.c:1287:10
    #7 0x5960e2 in git_init_db_config /home/ahunt/oss-fuzz/git/builtin/init-db.c:161:11
    git#8 0x7255b8 in configset_iter /home/ahunt/oss-fuzz/git/config.c:1982:7
    git#9 0x7253fc in repo_config /home/ahunt/oss-fuzz/git/config.c:2311:2
    git#10 0x725ca7 in git_config /home/ahunt/oss-fuzz/git/config.c:2399:2
    git#11 0x593e8d in create_default_files /home/ahunt/oss-fuzz/git/builtin/init-db.c:225:2
    git#12 0x5935c6 in init_db /home/ahunt/oss-fuzz/git/builtin/init-db.c:449:11
    git#13 0x59588e in cmd_init_db /home/ahunt/oss-fuzz/git/builtin/init-db.c:714:9
    git#14 0x4cd60d in run_builtin /home/ahunt/oss-fuzz/git/git.c:453:11
    git#15 0x4cb2da in handle_builtin /home/ahunt/oss-fuzz/git/git.c:704:3
    git#16 0x4ccc37 in run_argv /home/ahunt/oss-fuzz/git/git.c:771:4
    git#17 0x4cac29 in cmd_main /home/ahunt/oss-fuzz/git/git.c:902:19
    git#18 0x69c4de in main /home/ahunt/oss-fuzz/git/common-main.c:52:11
    git#19 0x7f23552d6349 in __libc_start_main (/lib64/libc.so.6+0x24349)
ahunt added a commit to ahunt/git that referenced this pull request Mar 5, 2021
init_db_config_path could be leaked because:
1. git_init_db_config() allocates new data into init_db_config_path on every
   invocation without freeing preexisting data.
2. git_init_db_config() can be called multiple times for a single git_config()
   invocation (see docs on git_config() for context).
Furthermore, until very recently, git_config(git_init_db_config(), ...) could
have been invoked twice in a single process as git_init_db_config() used to
be used to handle core.* config settings. This was changed in a previous
patch in this series.

Freeing the existing value in git_init_db_config() would be the least intrusive
fix, however switching to git_config_get_value() simplifies the code further by
letting us remove the static pointer (furthermore, the returned data is owned by
the config cache, saving us from having to worry about freeing it later).

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
    #1 0x9a7276 in xrealloc /home/ahunt/oss-fuzz/git/wrapper.c:126:8
    #2 0x9362ad in strbuf_grow /home/ahunt/oss-fuzz/git/strbuf.c:98:2
    #3 0x936eaa in strbuf_add /home/ahunt/oss-fuzz/git/strbuf.c:295:2
    #4 0x868112 in strbuf_addstr /home/ahunt/oss-fuzz/git/./strbuf.h:304:2
    #5 0x86a8ad in expand_user_path /home/ahunt/oss-fuzz/git/path.c:758:2
    #6 0x720bb1 in git_config_pathname /home/ahunt/oss-fuzz/git/config.c:1287:10
    #7 0x5960e2 in git_init_db_config /home/ahunt/oss-fuzz/git/builtin/init-db.c:161:11
    git#8 0x7255b8 in configset_iter /home/ahunt/oss-fuzz/git/config.c:1982:7
    git#9 0x7253fc in repo_config /home/ahunt/oss-fuzz/git/config.c:2311:2
    git#10 0x725ca7 in git_config /home/ahunt/oss-fuzz/git/config.c:2399:2
    git#11 0x593e8d in create_default_files /home/ahunt/oss-fuzz/git/builtin/init-db.c:225:2
    git#12 0x5935c6 in init_db /home/ahunt/oss-fuzz/git/builtin/init-db.c:449:11
    git#13 0x59588e in cmd_init_db /home/ahunt/oss-fuzz/git/builtin/init-db.c:714:9
    git#14 0x4cd60d in run_builtin /home/ahunt/oss-fuzz/git/git.c:453:11
    git#15 0x4cb2da in handle_builtin /home/ahunt/oss-fuzz/git/git.c:704:3
    git#16 0x4ccc37 in run_argv /home/ahunt/oss-fuzz/git/git.c:771:4
    git#17 0x4cac29 in cmd_main /home/ahunt/oss-fuzz/git/git.c:902:19
    git#18 0x69c4de in main /home/ahunt/oss-fuzz/git/common-main.c:52:11
    git#19 0x7f23552d6349 in __libc_start_main (/lib64/libc.so.6+0x24349)
ahunt added a commit to ahunt/git that referenced this pull request Mar 5, 2021
init_db_config_path could be leaked because:
1. git_init_db_config() allocates new data into init_db_config_path on every
   invocation without freeing preexisting data.
2. git_init_db_config() can be called multiple times for a single git_config()
   invocation (see docs on git_config() for context).
Furthermore, until very recently, git_config(git_init_db_config(), ...) could
have been invoked twice in a single process as git_init_db_config() used to
be used to handle core.* config settings. This was changed in a previous
patch in this series.

Freeing the existing value in git_init_db_config() would be the least intrusive
fix, however switching to git_config_get_value() simplifies the code further by
letting us remove the static pointer (furthermore, the returned data is owned by
the config cache, saving us from having to worry about freeing it later).

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
    #1 0x9a7276 in xrealloc /home/ahunt/oss-fuzz/git/wrapper.c:126:8
    #2 0x9362ad in strbuf_grow /home/ahunt/oss-fuzz/git/strbuf.c:98:2
    #3 0x936eaa in strbuf_add /home/ahunt/oss-fuzz/git/strbuf.c:295:2
    #4 0x868112 in strbuf_addstr /home/ahunt/oss-fuzz/git/./strbuf.h:304:2
    #5 0x86a8ad in expand_user_path /home/ahunt/oss-fuzz/git/path.c:758:2
    #6 0x720bb1 in git_config_pathname /home/ahunt/oss-fuzz/git/config.c:1287:10
    #7 0x5960e2 in git_init_db_config /home/ahunt/oss-fuzz/git/builtin/init-db.c:161:11
    git#8 0x7255b8 in configset_iter /home/ahunt/oss-fuzz/git/config.c:1982:7
    git#9 0x7253fc in repo_config /home/ahunt/oss-fuzz/git/config.c:2311:2
    git#10 0x725ca7 in git_config /home/ahunt/oss-fuzz/git/config.c:2399:2
    git#11 0x593e8d in create_default_files /home/ahunt/oss-fuzz/git/builtin/init-db.c:225:2
    git#12 0x5935c6 in init_db /home/ahunt/oss-fuzz/git/builtin/init-db.c:449:11
    git#13 0x59588e in cmd_init_db /home/ahunt/oss-fuzz/git/builtin/init-db.c:714:9
    git#14 0x4cd60d in run_builtin /home/ahunt/oss-fuzz/git/git.c:453:11
    git#15 0x4cb2da in handle_builtin /home/ahunt/oss-fuzz/git/git.c:704:3
    git#16 0x4ccc37 in run_argv /home/ahunt/oss-fuzz/git/git.c:771:4
    git#17 0x4cac29 in cmd_main /home/ahunt/oss-fuzz/git/git.c:902:19
    git#18 0x69c4de in main /home/ahunt/oss-fuzz/git/common-main.c:52:11
    git#19 0x7f23552d6349 in __libc_start_main (/lib64/libc.so.6+0x24349)
ahunt added a commit to ahunt/git that referenced this pull request Mar 6, 2021
init_db_config_path could be leaked because:
1. git_init_db_config() allocates new data into init_db_config_path on every
   invocation without freeing preexisting data.
2. git_init_db_config() can be called multiple times for a single git_config()
   invocation (see docs on git_config() for context).
Furthermore, until very recently, git_config(git_init_db_config(), ...) could
have been invoked twice in a single process as git_init_db_config() used to
be used to handle core.* config settings. This was changed in a previous
patch in this series - that change alone might fix the leak in some scenarios,
but is not enough to guarantee that we never leak.

Freeing the existing value in git_init_db_config() would be the least intrusive
fix, however switching to git_config_get_value() simplifies the code further by
letting us remove the static pointer (furthermore, the returned data is owned by
the config cache, saving us from having to worry about freeing it later).

The following commit is when we first started using git_init_db_config() for 2
different purposes:
  2878533 (mingw: respect core.hidedotfiles = false in git-init again, 2019-03-11)
Although I suspect the potential for a leak existed since the original
implementation 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
    #1 0x9a7276 in xrealloc /home/ahunt/oss-fuzz/git/wrapper.c:126:8
    #2 0x9362ad in strbuf_grow /home/ahunt/oss-fuzz/git/strbuf.c:98:2
    #3 0x936eaa in strbuf_add /home/ahunt/oss-fuzz/git/strbuf.c:295:2
    #4 0x868112 in strbuf_addstr /home/ahunt/oss-fuzz/git/./strbuf.h:304:2
    #5 0x86a8ad in expand_user_path /home/ahunt/oss-fuzz/git/path.c:758:2
    #6 0x720bb1 in git_config_pathname /home/ahunt/oss-fuzz/git/config.c:1287:10
    #7 0x5960e2 in git_init_db_config /home/ahunt/oss-fuzz/git/builtin/init-db.c:161:11
    git#8 0x7255b8 in configset_iter /home/ahunt/oss-fuzz/git/config.c:1982:7
    git#9 0x7253fc in repo_config /home/ahunt/oss-fuzz/git/config.c:2311:2
    git#10 0x725ca7 in git_config /home/ahunt/oss-fuzz/git/config.c:2399:2
    git#11 0x593e8d in create_default_files /home/ahunt/oss-fuzz/git/builtin/init-db.c:225:2
    git#12 0x5935c6 in init_db /home/ahunt/oss-fuzz/git/builtin/init-db.c:449:11
    git#13 0x59588e in cmd_init_db /home/ahunt/oss-fuzz/git/builtin/init-db.c:714:9
    git#14 0x4cd60d in run_builtin /home/ahunt/oss-fuzz/git/git.c:453:11
    git#15 0x4cb2da in handle_builtin /home/ahunt/oss-fuzz/git/git.c:704:3
    git#16 0x4ccc37 in run_argv /home/ahunt/oss-fuzz/git/git.c:771:4
    git#17 0x4cac29 in cmd_main /home/ahunt/oss-fuzz/git/git.c:902:19
    git#18 0x69c4de in main /home/ahunt/oss-fuzz/git/common-main.c:52:11
    git#19 0x7f23552d6349 in __libc_start_main (/lib64/libc.so.6+0x24349)
ahunt added a commit to ahunt/git that referenced this pull request Mar 6, 2021
init_db_config_path could be leaked because:
1. git_init_db_config() allocates new data into init_db_config_path on every
   invocation without freeing preexisting data.
2. git_init_db_config() can be called multiple times for a single git_config()
   invocation (see docs on git_config() for context).
Furthermore, until very recently, git_config(git_init_db_config(), ...) could
have been invoked twice in a single process as git_init_db_config() used to
be used to handle core.* config settings. This was changed in a previous
patch in this series - that change alone might fix the leak in some scenarios,
but is not enough to guarantee that we never leak.

Freeing the existing value in git_init_db_config() would be the least intrusive
fix, however switching to git_config_get_value() simplifies the code further by
letting us remove the static pointer (furthermore, the returned data is owned by
the config cache, saving us from having to worry about freeing it later).

The following commit is when we first started using git_init_db_config() for 2
different purposes:
  2878533 (mingw: respect core.hidedotfiles = false in git-init again, 2019-03-11)
Although I suspect the potential for a leak existed since the original
implementation 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
    #1 0x9a7276 in xrealloc /home/ahunt/oss-fuzz/git/wrapper.c:126:8
    #2 0x9362ad in strbuf_grow /home/ahunt/oss-fuzz/git/strbuf.c:98:2
    #3 0x936eaa in strbuf_add /home/ahunt/oss-fuzz/git/strbuf.c:295:2
    #4 0x868112 in strbuf_addstr /home/ahunt/oss-fuzz/git/./strbuf.h:304:2
    #5 0x86a8ad in expand_user_path /home/ahunt/oss-fuzz/git/path.c:758:2
    #6 0x720bb1 in git_config_pathname /home/ahunt/oss-fuzz/git/config.c:1287:10
    #7 0x5960e2 in git_init_db_config /home/ahunt/oss-fuzz/git/builtin/init-db.c:161:11
    git#8 0x7255b8 in configset_iter /home/ahunt/oss-fuzz/git/config.c:1982:7
    git#9 0x7253fc in repo_config /home/ahunt/oss-fuzz/git/config.c:2311:2
    git#10 0x725ca7 in git_config /home/ahunt/oss-fuzz/git/config.c:2399:2
    git#11 0x593e8d in create_default_files /home/ahunt/oss-fuzz/git/builtin/init-db.c:225:2
    git#12 0x5935c6 in init_db /home/ahunt/oss-fuzz/git/builtin/init-db.c:449:11
    git#13 0x59588e in cmd_init_db /home/ahunt/oss-fuzz/git/builtin/init-db.c:714:9
    git#14 0x4cd60d in run_builtin /home/ahunt/oss-fuzz/git/git.c:453:11
    git#15 0x4cb2da in handle_builtin /home/ahunt/oss-fuzz/git/git.c:704:3
    git#16 0x4ccc37 in run_argv /home/ahunt/oss-fuzz/git/git.c:771:4
    git#17 0x4cac29 in cmd_main /home/ahunt/oss-fuzz/git/git.c:902:19
    git#18 0x69c4de in main /home/ahunt/oss-fuzz/git/common-main.c:52:11
    git#19 0x7f23552d6349 in __libc_start_main (/lib64/libc.so.6+0x24349)
ahunt added a commit to ahunt/git that referenced this pull request Mar 6, 2021
Also UNLEAK template_dir in cmd_init_db(), as we are passing it into init_db()
and cannot easily free it without making more intrusive changes to execution
flow.

init_db_config_path could be leaked because:
1. git_init_db_config() allocates new data into init_db_config_path on every
   invocation without freeing preexisting data.
2. git_init_db_config() can be called multiple times for a single git_config()
   invocation (see docs on git_config() for context).
Furthermore, until very recently, git_config(git_init_db_config(), ...) could
have been invoked twice in a single process as git_init_db_config() used to
be used to handle core.* config settings. This was changed in a previous
patch in this series - that change alone might fix the leak in some scenarios,
but is not enough to guarantee that we never leak.

Freeing the existing value in git_init_db_config() would be the least intrusive
fix, however switching to git_config_get_value() simplifies the code further by
letting us remove the static pointer (furthermore, the returned data is owned by
the config cache, saving us from having to worry about freeing it later).

The following commit is when we first started using git_init_db_config() for 2
different purposes:
  2878533 (mingw: respect core.hidedotfiles = false in git-init again, 2019-03-11)
Although I suspect the potential for a leak existed since the original
implementation 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
    #1 0x9a7276 in xrealloc /home/ahunt/oss-fuzz/git/wrapper.c:126:8
    #2 0x9362ad in strbuf_grow /home/ahunt/oss-fuzz/git/strbuf.c:98:2
    #3 0x936eaa in strbuf_add /home/ahunt/oss-fuzz/git/strbuf.c:295:2
    #4 0x868112 in strbuf_addstr /home/ahunt/oss-fuzz/git/./strbuf.h:304:2
    #5 0x86a8ad in expand_user_path /home/ahunt/oss-fuzz/git/path.c:758:2
    #6 0x720bb1 in git_config_pathname /home/ahunt/oss-fuzz/git/config.c:1287:10
    #7 0x5960e2 in git_init_db_config /home/ahunt/oss-fuzz/git/builtin/init-db.c:161:11
    git#8 0x7255b8 in configset_iter /home/ahunt/oss-fuzz/git/config.c:1982:7
    git#9 0x7253fc in repo_config /home/ahunt/oss-fuzz/git/config.c:2311:2
    git#10 0x725ca7 in git_config /home/ahunt/oss-fuzz/git/config.c:2399:2
    git#11 0x593e8d in create_default_files /home/ahunt/oss-fuzz/git/builtin/init-db.c:225:2
    git#12 0x5935c6 in init_db /home/ahunt/oss-fuzz/git/builtin/init-db.c:449:11
    git#13 0x59588e in cmd_init_db /home/ahunt/oss-fuzz/git/builtin/init-db.c:714:9
    git#14 0x4cd60d in run_builtin /home/ahunt/oss-fuzz/git/git.c:453:11
    git#15 0x4cb2da in handle_builtin /home/ahunt/oss-fuzz/git/git.c:704:3
    git#16 0x4ccc37 in run_argv /home/ahunt/oss-fuzz/git/git.c:771:4
    git#17 0x4cac29 in cmd_main /home/ahunt/oss-fuzz/git/git.c:902:19
    git#18 0x69c4de in main /home/ahunt/oss-fuzz/git/common-main.c:52:11
    git#19 0x7f23552d6349 in __libc_start_main (/lib64/libc.so.6+0x24349)
ahunt added a commit to ahunt/git that referenced this pull request Mar 6, 2021
Also UNLEAK template_dir in cmd_init_db(), as we are passing it into init_db()
and cannot easily free it without making more intrusive changes to execution
flow.

init_db_config_path could be leaked because:
1. git_init_db_config() allocates new data into init_db_config_path on every
   invocation without freeing preexisting data.
2. git_init_db_config() can be called multiple times for a single git_config()
   invocation (see docs on git_config() for context).
Furthermore, until very recently, git_config(git_init_db_config(), ...) could
have been invoked twice in a single process as git_init_db_config() used to
be used to handle core.* config settings. This was changed in a previous
patch in this series - that change alone might fix the leak in some scenarios,
but is not enough to guarantee that we never leak.

Freeing the existing value in git_init_db_config() would be the least intrusive
fix, however switching to git_config_get_value() simplifies the code further by
letting us remove the static pointer (furthermore, the returned data is owned by
the config cache, saving us from having to worry about freeing it later).

The following commit is when we first started using git_init_db_config() for 2
different purposes:
  2878533 (mingw: respect core.hidedotfiles = false in git-init again, 2019-03-11)
Although I suspect the potential for a leak existed since the original
implementation 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
    #1 0x9a7276 in xrealloc /home/ahunt/oss-fuzz/git/wrapper.c:126:8
    #2 0x9362ad in strbuf_grow /home/ahunt/oss-fuzz/git/strbuf.c:98:2
    #3 0x936eaa in strbuf_add /home/ahunt/oss-fuzz/git/strbuf.c:295:2
    #4 0x868112 in strbuf_addstr /home/ahunt/oss-fuzz/git/./strbuf.h:304:2
    #5 0x86a8ad in expand_user_path /home/ahunt/oss-fuzz/git/path.c:758:2
    #6 0x720bb1 in git_config_pathname /home/ahunt/oss-fuzz/git/config.c:1287:10
    #7 0x5960e2 in git_init_db_config /home/ahunt/oss-fuzz/git/builtin/init-db.c:161:11
    git#8 0x7255b8 in configset_iter /home/ahunt/oss-fuzz/git/config.c:1982:7
    git#9 0x7253fc in repo_config /home/ahunt/oss-fuzz/git/config.c:2311:2
    git#10 0x725ca7 in git_config /home/ahunt/oss-fuzz/git/config.c:2399:2
    git#11 0x593e8d in create_default_files /home/ahunt/oss-fuzz/git/builtin/init-db.c:225:2
    git#12 0x5935c6 in init_db /home/ahunt/oss-fuzz/git/builtin/init-db.c:449:11
    git#13 0x59588e in cmd_init_db /home/ahunt/oss-fuzz/git/builtin/init-db.c:714:9
    git#14 0x4cd60d in run_builtin /home/ahunt/oss-fuzz/git/git.c:453:11
    git#15 0x4cb2da in handle_builtin /home/ahunt/oss-fuzz/git/git.c:704:3
    git#16 0x4ccc37 in run_argv /home/ahunt/oss-fuzz/git/git.c:771:4
    git#17 0x4cac29 in cmd_main /home/ahunt/oss-fuzz/git/git.c:902:19
    git#18 0x69c4de in main /home/ahunt/oss-fuzz/git/common-main.c:52:11
    git#19 0x7f23552d6349 in __libc_start_main (/lib64/libc.so.6+0x24349)

Signed-off-by: Andrzej Hunt <ajrhunt@google.com>
ahunt added a commit to ahunt/git that referenced this pull request Mar 8, 2021
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.
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 convert to git_config_get_value() as that is more
explicit, seemingly more efficient, AND avoids allocations (the returned result
is owned by the config cache, so we aren't responsible for freeing it).

By removing init_db_template_dir, git_init_db_config() ends up only being
responsible 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 on
platform_core_config().

In addition to those changes, we squash another leak by UNLEAK'ing template_dir
in cmd_init_db(). We are already passing it into init_db() and cannot easily
free it without making more intrusive changes to execution flow.

The platform_core_config forwarding was originally added in:
  2878533 (mingw: respect core.hidedotfiles = false in git-init again, 2019-03-11
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
    #1 0x9a7276 in xrealloc /home/ahunt/oss-fuzz/git/wrapper.c:126:8
    #2 0x9362ad in strbuf_grow /home/ahunt/oss-fuzz/git/strbuf.c:98:2
    #3 0x936eaa in strbuf_add /home/ahunt/oss-fuzz/git/strbuf.c:295:2
    #4 0x868112 in strbuf_addstr /home/ahunt/oss-fuzz/git/./strbuf.h:304:2
    #5 0x86a8ad in expand_user_path /home/ahunt/oss-fuzz/git/path.c:758:2
    #6 0x720bb1 in git_config_pathname /home/ahunt/oss-fuzz/git/config.c:1287:10
    #7 0x5960e2 in git_init_db_config /home/ahunt/oss-fuzz/git/builtin/init-db.c:161:11
    git#8 0x7255b8 in configset_iter /home/ahunt/oss-fuzz/git/config.c:1982:7
    git#9 0x7253fc in repo_config /home/ahunt/oss-fuzz/git/config.c:2311:2
    git#10 0x725ca7 in git_config /home/ahunt/oss-fuzz/git/config.c:2399:2
    git#11 0x593e8d in create_default_files /home/ahunt/oss-fuzz/git/builtin/init-db.c:225:2
    git#12 0x5935c6 in init_db /home/ahunt/oss-fuzz/git/builtin/init-db.c:449:11
    git#13 0x59588e in cmd_init_db /home/ahunt/oss-fuzz/git/builtin/init-db.c:714:9
    git#14 0x4cd60d in run_builtin /home/ahunt/oss-fuzz/git/git.c:453:11
    git#15 0x4cb2da in handle_builtin /home/ahunt/oss-fuzz/git/git.c:704:3
    git#16 0x4ccc37 in run_argv /home/ahunt/oss-fuzz/git/git.c:771:4
    git#17 0x4cac29 in cmd_main /home/ahunt/oss-fuzz/git/git.c:902:19
    git#18 0x69c4de in main /home/ahunt/oss-fuzz/git/common-main.c:52:11
    git#19 0x7f23552d6349 in __libc_start_main (/lib64/libc.so.6+0x24349)

Signed-off-by: Andrzej Hunt <ajrhunt@google.com>
ahunt added a commit to ahunt/git that referenced this pull request Mar 8, 2021
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.
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 convert to git_config_get_value() as that is more
explicit, seemingly more efficient, AND avoids allocations (the returned result
is owned by the config cache, so we aren't responsible for freeing it).

By removing init_db_template_dir, git_init_db_config() ends up only being
responsible 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 on
platform_core_config().

The platform_core_config forwarding was originally added in:
  2878533 (mingw: respect core.hidedotfiles = false in git-init again, 2019-03-11
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
    #1 0x9a7276 in xrealloc /home/ahunt/oss-fuzz/git/wrapper.c:126:8
    #2 0x9362ad in strbuf_grow /home/ahunt/oss-fuzz/git/strbuf.c:98:2
    #3 0x936eaa in strbuf_add /home/ahunt/oss-fuzz/git/strbuf.c:295:2
    #4 0x868112 in strbuf_addstr /home/ahunt/oss-fuzz/git/./strbuf.h:304:2
    #5 0x86a8ad in expand_user_path /home/ahunt/oss-fuzz/git/path.c:758:2
    #6 0x720bb1 in git_config_pathname /home/ahunt/oss-fuzz/git/config.c:1287:10
    #7 0x5960e2 in git_init_db_config /home/ahunt/oss-fuzz/git/builtin/init-db.c:161:11
    git#8 0x7255b8 in configset_iter /home/ahunt/oss-fuzz/git/config.c:1982:7
    git#9 0x7253fc in repo_config /home/ahunt/oss-fuzz/git/config.c:2311:2
    git#10 0x725ca7 in git_config /home/ahunt/oss-fuzz/git/config.c:2399:2
    git#11 0x593e8d in create_default_files /home/ahunt/oss-fuzz/git/builtin/init-db.c:225:2
    git#12 0x5935c6 in init_db /home/ahunt/oss-fuzz/git/builtin/init-db.c:449:11
    git#13 0x59588e in cmd_init_db /home/ahunt/oss-fuzz/git/builtin/init-db.c:714:9
    git#14 0x4cd60d in run_builtin /home/ahunt/oss-fuzz/git/git.c:453:11
    git#15 0x4cb2da in handle_builtin /home/ahunt/oss-fuzz/git/git.c:704:3
    git#16 0x4ccc37 in run_argv /home/ahunt/oss-fuzz/git/git.c:771:4
    git#17 0x4cac29 in cmd_main /home/ahunt/oss-fuzz/git/git.c:902:19
    git#18 0x69c4de in main /home/ahunt/oss-fuzz/git/common-main.c:52:11
    git#19 0x7f23552d6349 in __libc_start_main (/lib64/libc.so.6+0x24349)

Signed-off-by: Andrzej Hunt <ajrhunt@google.com>
ahunt added a commit to ahunt/git that referenced this pull request Mar 8, 2021
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.
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 convert to git_config_get_value() as that is more
explicit, seemingly more efficient, AND avoids allocations (the returned result
is owned by the config cache, so we aren't responsible for freeing it).

By removing init_db_template_dir, git_init_db_config() ends up only being
responsible 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 on
platform_core_config().

The platform_core_config forwarding was originally added in:
  2878533 (mingw: respect core.hidedotfiles = false in git-init again, 2019-03-11
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
    #1 0x9a7276 in xrealloc /home/ahunt/oss-fuzz/git/wrapper.c:126:8
    #2 0x9362ad in strbuf_grow /home/ahunt/oss-fuzz/git/strbuf.c:98:2
    #3 0x936eaa in strbuf_add /home/ahunt/oss-fuzz/git/strbuf.c:295:2
    #4 0x868112 in strbuf_addstr /home/ahunt/oss-fuzz/git/./strbuf.h:304:2
    #5 0x86a8ad in expand_user_path /home/ahunt/oss-fuzz/git/path.c:758:2
    #6 0x720bb1 in git_config_pathname /home/ahunt/oss-fuzz/git/config.c:1287:10
    #7 0x5960e2 in git_init_db_config /home/ahunt/oss-fuzz/git/builtin/init-db.c:161:11
    git#8 0x7255b8 in configset_iter /home/ahunt/oss-fuzz/git/config.c:1982:7
    git#9 0x7253fc in repo_config /home/ahunt/oss-fuzz/git/config.c:2311:2
    git#10 0x725ca7 in git_config /home/ahunt/oss-fuzz/git/config.c:2399:2
    git#11 0x593e8d in create_default_files /home/ahunt/oss-fuzz/git/builtin/init-db.c:225:2
    git#12 0x5935c6 in init_db /home/ahunt/oss-fuzz/git/builtin/init-db.c:449:11
    git#13 0x59588e in cmd_init_db /home/ahunt/oss-fuzz/git/builtin/init-db.c:714:9
    git#14 0x4cd60d in run_builtin /home/ahunt/oss-fuzz/git/git.c:453:11
    git#15 0x4cb2da in handle_builtin /home/ahunt/oss-fuzz/git/git.c:704:3
    git#16 0x4ccc37 in run_argv /home/ahunt/oss-fuzz/git/git.c:771:4
    git#17 0x4cac29 in cmd_main /home/ahunt/oss-fuzz/git/git.c:902:19
    git#18 0x69c4de in main /home/ahunt/oss-fuzz/git/common-main.c:52:11
    git#19 0x7f23552d6349 in __libc_start_main (/lib64/libc.so.6+0x24349)

Signed-off-by: Andrzej Hunt <ajrhunt@google.com>
ahunt added a commit to ahunt/git that referenced this pull request Mar 8, 2021
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.
 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
    #1 0x9a7276 in xrealloc /home/ahunt/oss-fuzz/git/wrapper.c:126:8
    #2 0x9362ad in strbuf_grow /home/ahunt/oss-fuzz/git/strbuf.c:98:2
    #3 0x936eaa in strbuf_add /home/ahunt/oss-fuzz/git/strbuf.c:295:2
    #4 0x868112 in strbuf_addstr /home/ahunt/oss-fuzz/git/./strbuf.h:304:2
    #5 0x86a8ad in expand_user_path /home/ahunt/oss-fuzz/git/path.c:758:2
    #6 0x720bb1 in git_config_pathname /home/ahunt/oss-fuzz/git/config.c:1287:10
    #7 0x5960e2 in git_init_db_config /home/ahunt/oss-fuzz/git/builtin/init-db.c:161:11
    git#8 0x7255b8 in configset_iter /home/ahunt/oss-fuzz/git/config.c:1982:7
    git#9 0x7253fc in repo_config /home/ahunt/oss-fuzz/git/config.c:2311:2
    git#10 0x725ca7 in git_config /home/ahunt/oss-fuzz/git/config.c:2399:2
    git#11 0x593e8d in create_default_files /home/ahunt/oss-fuzz/git/builtin/init-db.c:225:2
    git#12 0x5935c6 in init_db /home/ahunt/oss-fuzz/git/builtin/init-db.c:449:11
    git#13 0x59588e in cmd_init_db /home/ahunt/oss-fuzz/git/builtin/init-db.c:714:9
    git#14 0x4cd60d in run_builtin /home/ahunt/oss-fuzz/git/git.c:453:11
    git#15 0x4cb2da in handle_builtin /home/ahunt/oss-fuzz/git/git.c:704:3
    git#16 0x4ccc37 in run_argv /home/ahunt/oss-fuzz/git/git.c:771:4
    git#17 0x4cac29 in cmd_main /home/ahunt/oss-fuzz/git/git.c:902:19
    git#18 0x69c4de in main /home/ahunt/oss-fuzz/git/common-main.c:52:11
    git#19 0x7f23552d6349 in __libc_start_main (/lib64/libc.so.6+0x24349)

Signed-off-by: Andrzej Hunt <ajrhunt@google.com>
ahunt added a commit to ahunt/git that referenced this pull request Mar 8, 2021
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
    #1 0x9a7276 in xrealloc /home/ahunt/oss-fuzz/git/wrapper.c:126:8
    #2 0x9362ad in strbuf_grow /home/ahunt/oss-fuzz/git/strbuf.c:98:2
    #3 0x936eaa in strbuf_add /home/ahunt/oss-fuzz/git/strbuf.c:295:2
    #4 0x868112 in strbuf_addstr /home/ahunt/oss-fuzz/git/./strbuf.h:304:2
    #5 0x86a8ad in expand_user_path /home/ahunt/oss-fuzz/git/path.c:758:2
    #6 0x720bb1 in git_config_pathname /home/ahunt/oss-fuzz/git/config.c:1287:10
    #7 0x5960e2 in git_init_db_config /home/ahunt/oss-fuzz/git/builtin/init-db.c:161:11
    git#8 0x7255b8 in configset_iter /home/ahunt/oss-fuzz/git/config.c:1982:7
    git#9 0x7253fc in repo_config /home/ahunt/oss-fuzz/git/config.c:2311:2
    git#10 0x725ca7 in git_config /home/ahunt/oss-fuzz/git/config.c:2399:2
    git#11 0x593e8d in create_default_files /home/ahunt/oss-fuzz/git/builtin/init-db.c:225:2
    git#12 0x5935c6 in init_db /home/ahunt/oss-fuzz/git/builtin/init-db.c:449:11
    git#13 0x59588e in cmd_init_db /home/ahunt/oss-fuzz/git/builtin/init-db.c:714:9
    git#14 0x4cd60d in run_builtin /home/ahunt/oss-fuzz/git/git.c:453:11
    git#15 0x4cb2da in handle_builtin /home/ahunt/oss-fuzz/git/git.c:704:3
    git#16 0x4ccc37 in run_argv /home/ahunt/oss-fuzz/git/git.c:771:4
    git#17 0x4cac29 in cmd_main /home/ahunt/oss-fuzz/git/git.c:902:19
    git#18 0x69c4de in main /home/ahunt/oss-fuzz/git/common-main.c:52:11
    git#19 0x7f23552d6349 in __libc_start_main (/lib64/libc.so.6+0x24349)

Signed-off-by: Andrzej Hunt <ajrhunt@google.com>
gitster pushed a commit that referenced this pull request Mar 8, 2021
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
    #1 0x9a7276 in xrealloc /home/ahunt/oss-fuzz/git/wrapper.c:126:8
    #2 0x9362ad in strbuf_grow /home/ahunt/oss-fuzz/git/strbuf.c:98:2
    #3 0x936eaa in strbuf_add /home/ahunt/oss-fuzz/git/strbuf.c:295:2
    #4 0x868112 in strbuf_addstr /home/ahunt/oss-fuzz/git/./strbuf.h:304:2
    #5 0x86a8ad in expand_user_path /home/ahunt/oss-fuzz/git/path.c:758:2
    #6 0x720bb1 in git_config_pathname /home/ahunt/oss-fuzz/git/config.c:1287:10
    #7 0x5960e2 in git_init_db_config /home/ahunt/oss-fuzz/git/builtin/init-db.c:161:11
    #8 0x7255b8 in configset_iter /home/ahunt/oss-fuzz/git/config.c:1982:7
    #9 0x7253fc in repo_config /home/ahunt/oss-fuzz/git/config.c:2311:2
    #10 0x725ca7 in git_config /home/ahunt/oss-fuzz/git/config.c:2399:2
    #11 0x593e8d in create_default_files /home/ahunt/oss-fuzz/git/builtin/init-db.c:225:2
    #12 0x5935c6 in init_db /home/ahunt/oss-fuzz/git/builtin/init-db.c:449:11
    #13 0x59588e in cmd_init_db /home/ahunt/oss-fuzz/git/builtin/init-db.c:714:9
    #14 0x4cd60d in run_builtin /home/ahunt/oss-fuzz/git/git.c:453:11
    #15 0x4cb2da in handle_builtin /home/ahunt/oss-fuzz/git/git.c:704:3
    #16 0x4ccc37 in run_argv /home/ahunt/oss-fuzz/git/git.c:771:4
    #17 0x4cac29 in cmd_main /home/ahunt/oss-fuzz/git/git.c:902:19
    #18 0x69c4de in main /home/ahunt/oss-fuzz/git/common-main.c:52:11
    #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>
ahunt added a commit to ahunt/git that referenced this pull request Mar 14, 2021
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
    #1 0x9a7276 in xrealloc /home/ahunt/oss-fuzz/git/wrapper.c:126:8
    #2 0x9362ad in strbuf_grow /home/ahunt/oss-fuzz/git/strbuf.c:98:2
    #3 0x936eaa in strbuf_add /home/ahunt/oss-fuzz/git/strbuf.c:295:2
    #4 0x868112 in strbuf_addstr /home/ahunt/oss-fuzz/git/./strbuf.h:304:2
    #5 0x86a8ad in expand_user_path /home/ahunt/oss-fuzz/git/path.c:758:2
    #6 0x720bb1 in git_config_pathname /home/ahunt/oss-fuzz/git/config.c:1287:10
    #7 0x5960e2 in git_init_db_config /home/ahunt/oss-fuzz/git/builtin/init-db.c:161:11
    git#8 0x7255b8 in configset_iter /home/ahunt/oss-fuzz/git/config.c:1982:7
    git#9 0x7253fc in repo_config /home/ahunt/oss-fuzz/git/config.c:2311:2
    git#10 0x725ca7 in git_config /home/ahunt/oss-fuzz/git/config.c:2399:2
    git#11 0x593e8d in create_default_files /home/ahunt/oss-fuzz/git/builtin/init-db.c:225:2
    git#12 0x5935c6 in init_db /home/ahunt/oss-fuzz/git/builtin/init-db.c:449:11
    git#13 0x59588e in cmd_init_db /home/ahunt/oss-fuzz/git/builtin/init-db.c:714:9
    git#14 0x4cd60d in run_builtin /home/ahunt/oss-fuzz/git/git.c:453:11
    git#15 0x4cb2da in handle_builtin /home/ahunt/oss-fuzz/git/git.c:704:3
    git#16 0x4ccc37 in run_argv /home/ahunt/oss-fuzz/git/git.c:771:4
    git#17 0x4cac29 in cmd_main /home/ahunt/oss-fuzz/git/git.c:902:19
    git#18 0x69c4de in main /home/ahunt/oss-fuzz/git/common-main.c:52:11
    git#19 0x7f23552d6349 in __libc_start_main (/lib64/libc.so.6+0x24349)

Signed-off-by: Andrzej Hunt <ajrhunt@google.com>
ahunt added a commit to ahunt/git that referenced this pull request Mar 14, 2021
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
    #1 0x9a7276 in xrealloc /home/ahunt/oss-fuzz/git/wrapper.c:126:8
    #2 0x9362ad in strbuf_grow /home/ahunt/oss-fuzz/git/strbuf.c:98:2
    #3 0x936eaa in strbuf_add /home/ahunt/oss-fuzz/git/strbuf.c:295:2
    #4 0x868112 in strbuf_addstr /home/ahunt/oss-fuzz/git/./strbuf.h:304:2
    #5 0x86a8ad in expand_user_path /home/ahunt/oss-fuzz/git/path.c:758:2
    #6 0x720bb1 in git_config_pathname /home/ahunt/oss-fuzz/git/config.c:1287:10
    #7 0x5960e2 in git_init_db_config /home/ahunt/oss-fuzz/git/builtin/init-db.c:161:11
    git#8 0x7255b8 in configset_iter /home/ahunt/oss-fuzz/git/config.c:1982:7
    git#9 0x7253fc in repo_config /home/ahunt/oss-fuzz/git/config.c:2311:2
    git#10 0x725ca7 in git_config /home/ahunt/oss-fuzz/git/config.c:2399:2
    git#11 0x593e8d in create_default_files /home/ahunt/oss-fuzz/git/builtin/init-db.c:225:2
    git#12 0x5935c6 in init_db /home/ahunt/oss-fuzz/git/builtin/init-db.c:449:11
    git#13 0x59588e in cmd_init_db /home/ahunt/oss-fuzz/git/builtin/init-db.c:714:9
    git#14 0x4cd60d in run_builtin /home/ahunt/oss-fuzz/git/git.c:453:11
    git#15 0x4cb2da in handle_builtin /home/ahunt/oss-fuzz/git/git.c:704:3
    git#16 0x4ccc37 in run_argv /home/ahunt/oss-fuzz/git/git.c:771:4
    git#17 0x4cac29 in cmd_main /home/ahunt/oss-fuzz/git/git.c:902:19
    git#18 0x69c4de in main /home/ahunt/oss-fuzz/git/common-main.c:52:11
    git#19 0x7f23552d6349 in __libc_start_main (/lib64/libc.so.6+0x24349)

Signed-off-by: Andrzej Hunt <ajrhunt@google.com>
ahunt added a commit to ahunt/git that referenced this pull request Mar 14, 2021
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
    #1 0x9a7276 in xrealloc /home/ahunt/oss-fuzz/git/wrapper.c:126:8
    #2 0x9362ad in strbuf_grow /home/ahunt/oss-fuzz/git/strbuf.c:98:2
    #3 0x936eaa in strbuf_add /home/ahunt/oss-fuzz/git/strbuf.c:295:2
    #4 0x868112 in strbuf_addstr /home/ahunt/oss-fuzz/git/./strbuf.h:304:2
    #5 0x86a8ad in expand_user_path /home/ahunt/oss-fuzz/git/path.c:758:2
    #6 0x720bb1 in git_config_pathname /home/ahunt/oss-fuzz/git/config.c:1287:10
    #7 0x5960e2 in git_init_db_config /home/ahunt/oss-fuzz/git/builtin/init-db.c:161:11
    git#8 0x7255b8 in configset_iter /home/ahunt/oss-fuzz/git/config.c:1982:7
    git#9 0x7253fc in repo_config /home/ahunt/oss-fuzz/git/config.c:2311:2
    git#10 0x725ca7 in git_config /home/ahunt/oss-fuzz/git/config.c:2399:2
    git#11 0x593e8d in create_default_files /home/ahunt/oss-fuzz/git/builtin/init-db.c:225:2
    git#12 0x5935c6 in init_db /home/ahunt/oss-fuzz/git/builtin/init-db.c:449:11
    git#13 0x59588e in cmd_init_db /home/ahunt/oss-fuzz/git/builtin/init-db.c:714:9
    git#14 0x4cd60d in run_builtin /home/ahunt/oss-fuzz/git/git.c:453:11
    git#15 0x4cb2da in handle_builtin /home/ahunt/oss-fuzz/git/git.c:704:3
    git#16 0x4ccc37 in run_argv /home/ahunt/oss-fuzz/git/git.c:771:4
    git#17 0x4cac29 in cmd_main /home/ahunt/oss-fuzz/git/git.c:902:19
    git#18 0x69c4de in main /home/ahunt/oss-fuzz/git/common-main.c:52:11
    git#19 0x7f23552d6349 in __libc_start_main (/lib64/libc.so.6+0x24349)

Signed-off-by: Andrzej Hunt <ajrhunt@google.com>
gitster pushed a commit that referenced this pull request Mar 15, 2021
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
    #1 0x9a7276 in xrealloc /home/ahunt/oss-fuzz/git/wrapper.c:126:8
    #2 0x9362ad in strbuf_grow /home/ahunt/oss-fuzz/git/strbuf.c:98:2
    #3 0x936eaa in strbuf_add /home/ahunt/oss-fuzz/git/strbuf.c:295:2
    #4 0x868112 in strbuf_addstr /home/ahunt/oss-fuzz/git/./strbuf.h:304:2
    #5 0x86a8ad in expand_user_path /home/ahunt/oss-fuzz/git/path.c:758:2
    #6 0x720bb1 in git_config_pathname /home/ahunt/oss-fuzz/git/config.c:1287:10
    #7 0x5960e2 in git_init_db_config /home/ahunt/oss-fuzz/git/builtin/init-db.c:161:11
    #8 0x7255b8 in configset_iter /home/ahunt/oss-fuzz/git/config.c:1982:7
    #9 0x7253fc in repo_config /home/ahunt/oss-fuzz/git/config.c:2311:2
    #10 0x725ca7 in git_config /home/ahunt/oss-fuzz/git/config.c:2399:2
    #11 0x593e8d in create_default_files /home/ahunt/oss-fuzz/git/builtin/init-db.c:225:2
    #12 0x5935c6 in init_db /home/ahunt/oss-fuzz/git/builtin/init-db.c:449:11
    #13 0x59588e in cmd_init_db /home/ahunt/oss-fuzz/git/builtin/init-db.c:714:9
    #14 0x4cd60d in run_builtin /home/ahunt/oss-fuzz/git/git.c:453:11
    #15 0x4cb2da in handle_builtin /home/ahunt/oss-fuzz/git/git.c:704:3
    #16 0x4ccc37 in run_argv /home/ahunt/oss-fuzz/git/git.c:771:4
    #17 0x4cac29 in cmd_main /home/ahunt/oss-fuzz/git/git.c:902:19
    #18 0x69c4de in main /home/ahunt/oss-fuzz/git/common-main.c:52:11
    #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>
ahunt added a commit to ahunt/git that referenced this pull request Mar 16, 2021
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
    #1 0x9a7276 in xrealloc /home/ahunt/oss-fuzz/git/wrapper.c:126:8
    #2 0x9362ad in strbuf_grow /home/ahunt/oss-fuzz/git/strbuf.c:98:2
    #3 0x936eaa in strbuf_add /home/ahunt/oss-fuzz/git/strbuf.c:295:2
    #4 0x868112 in strbuf_addstr /home/ahunt/oss-fuzz/git/./strbuf.h:304:2
    #5 0x86a8ad in expand_user_path /home/ahunt/oss-fuzz/git/path.c:758:2
    #6 0x720bb1 in git_config_pathname /home/ahunt/oss-fuzz/git/config.c:1287:10
    #7 0x5960e2 in git_init_db_config /home/ahunt/oss-fuzz/git/builtin/init-db.c:161:11
    git#8 0x7255b8 in configset_iter /home/ahunt/oss-fuzz/git/config.c:1982:7
    git#9 0x7253fc in repo_config /home/ahunt/oss-fuzz/git/config.c:2311:2
    git#10 0x725ca7 in git_config /home/ahunt/oss-fuzz/git/config.c:2399:2
    git#11 0x593e8d in create_default_files /home/ahunt/oss-fuzz/git/builtin/init-db.c:225:2
    git#12 0x5935c6 in init_db /home/ahunt/oss-fuzz/git/builtin/init-db.c:449:11
    git#13 0x59588e in cmd_init_db /home/ahunt/oss-fuzz/git/builtin/init-db.c:714:9
    git#14 0x4cd60d in run_builtin /home/ahunt/oss-fuzz/git/git.c:453:11
    git#15 0x4cb2da in handle_builtin /home/ahunt/oss-fuzz/git/git.c:704:3
    git#16 0x4ccc37 in run_argv /home/ahunt/oss-fuzz/git/git.c:771:4
    git#17 0x4cac29 in cmd_main /home/ahunt/oss-fuzz/git/git.c:902:19
    git#18 0x69c4de in main /home/ahunt/oss-fuzz/git/common-main.c:52:11
    git#19 0x7f23552d6349 in __libc_start_main (/lib64/libc.so.6+0x24349)

Signed-off-by: Andrzej Hunt <ajrhunt@google.com>
ahunt added a commit to ahunt/git that referenced this pull request Mar 21, 2021
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
    #1 0x9a7276 in xrealloc /home/ahunt/oss-fuzz/git/wrapper.c:126:8
    #2 0x9362ad in strbuf_grow /home/ahunt/oss-fuzz/git/strbuf.c:98:2
    #3 0x936eaa in strbuf_add /home/ahunt/oss-fuzz/git/strbuf.c:295:2
    #4 0x868112 in strbuf_addstr /home/ahunt/oss-fuzz/git/./strbuf.h:304:2
    #5 0x86a8ad in expand_user_path /home/ahunt/oss-fuzz/git/path.c:758:2
    #6 0x720bb1 in git_config_pathname /home/ahunt/oss-fuzz/git/config.c:1287:10
    #7 0x5960e2 in git_init_db_config /home/ahunt/oss-fuzz/git/builtin/init-db.c:161:11
    git#8 0x7255b8 in configset_iter /home/ahunt/oss-fuzz/git/config.c:1982:7
    git#9 0x7253fc in repo_config /home/ahunt/oss-fuzz/git/config.c:2311:2
    git#10 0x725ca7 in git_config /home/ahunt/oss-fuzz/git/config.c:2399:2
    git#11 0x593e8d in create_default_files /home/ahunt/oss-fuzz/git/builtin/init-db.c:225:2
    git#12 0x5935c6 in init_db /home/ahunt/oss-fuzz/git/builtin/init-db.c:449:11
    git#13 0x59588e in cmd_init_db /home/ahunt/oss-fuzz/git/builtin/init-db.c:714:9
    git#14 0x4cd60d in run_builtin /home/ahunt/oss-fuzz/git/git.c:453:11
    git#15 0x4cb2da in handle_builtin /home/ahunt/oss-fuzz/git/git.c:704:3
    git#16 0x4ccc37 in run_argv /home/ahunt/oss-fuzz/git/git.c:771:4
    git#17 0x4cac29 in cmd_main /home/ahunt/oss-fuzz/git/git.c:902:19
    git#18 0x69c4de in main /home/ahunt/oss-fuzz/git/common-main.c:52:11
    git#19 0x7f23552d6349 in __libc_start_main (/lib64/libc.so.6+0x24349)

Signed-off-by: Andrzej Hunt <ajrhunt@google.com>
ahunt added a commit to ahunt/git that referenced this pull request Apr 9, 2021
limit_list() iterates over the original revs->commits list, and consumes
many of its entries via pop_commit. However we might stop iterating over
the list early (e.g. if we realise that the rest of the list is
uninteresting). If we do stop iterating early, list will be pointing to
the unconsumed portion of revs->commits - and we need to free this list
to avoid a leak. (revs->commits itself will be an invalid pointer: it
will have been free'd during the first pop_commit.)

This leak was found while running t0090. It's not likely to be very
impactful, but it can happen quite early during some checkout
invocations, and hence seems to be worth fixing:

Direct leak of 16 byte(s) in 1 object(s) allocated from:
    #0 0x49a85d in malloc ../projects/compiler-rt/lib/asan/asan_malloc_linux.cpp:145:3
    #1 0x9ac084 in do_xmalloc wrapper.c:41:8
    #2 0x9ac05a in xmalloc wrapper.c:62:9
    #3 0x7175d6 in commit_list_insert commit.c:540:33
    #4 0x71800f in commit_list_insert_by_date commit.c:604:9
    #5 0x8f8d2e in process_parents revision.c:1128:5
    #6 0x8f2f2c in limit_list revision.c:1418:7
    #7 0x8f210e in prepare_revision_walk revision.c:3577:7
    git#8 0x514170 in orphaned_commit_warning builtin/checkout.c:1185:6
    git#9 0x512f05 in switch_branches builtin/checkout.c:1250:3
    git#10 0x50f8de in checkout_branch builtin/checkout.c:1646:9
    git#11 0x50ba12 in checkout_main builtin/checkout.c:2003:9
    git#12 0x5086c0 in cmd_checkout builtin/checkout.c:2055:8
    git#13 0x4cd91d in run_builtin git.c:467:11
    git#14 0x4cb5f3 in handle_builtin git.c:719:3
    git#15 0x4ccf47 in run_argv git.c:808:4
    git#16 0x4caf49 in cmd_main git.c:939:19
    git#17 0x69dc0e in main common-main.c:52:11
    git#18 0x7faaabd0e349 in __libc_start_main (/lib64/libc.so.6+0x24349)

Indirect leak of 48 byte(s) in 3 object(s) allocated from:
    #0 0x49a85d in malloc ../projects/compiler-rt/lib/asan/asan_malloc_linux.cpp:145:3
    #1 0x9ac084 in do_xmalloc wrapper.c:41:8
    #2 0x9ac05a in xmalloc wrapper.c:62:9
    #3 0x717de6 in commit_list_append commit.c:1609:35
    #4 0x8f1f9b in prepare_revision_walk revision.c:3554:12
    #5 0x514170 in orphaned_commit_warning builtin/checkout.c:1185:6
    #6 0x512f05 in switch_branches builtin/checkout.c:1250:3
    #7 0x50f8de in checkout_branch builtin/checkout.c:1646:9
    git#8 0x50ba12 in checkout_main builtin/checkout.c:2003:9
    git#9 0x5086c0 in cmd_checkout builtin/checkout.c:2055:8
    git#10 0x4cd91d in run_builtin git.c:467:11
    git#11 0x4cb5f3 in handle_builtin git.c:719:3
    git#12 0x4ccf47 in run_argv git.c:808:4
    git#13 0x4caf49 in cmd_main git.c:939:19
    git#14 0x69dc0e in main common-main.c:52:11
    git#15 0x7faaabd0e349 in __libc_start_main (/lib64/libc.so.6+0x24349)

Signed-off-by: Andrzej Hunt <ajrhunt@google.com>
ahunt added a commit to ahunt/git that referenced this pull request Apr 9, 2021
add_pending_object() populates rev.pending, we need to take care of
clearing it once we're done.

This code is run close to the end of a checkout, therefore this leak
seems like it would have very little impact. See also LSAN output
from t0020 below:

Direct leak of 2048 byte(s) in 1 object(s) allocated from:
    #0 0x49ab79 in realloc ../projects/compiler-rt/lib/asan/asan_malloc_linux.cpp:164:3
    #1 0x9acc46 in xrealloc wrapper.c:126:8
    #2 0x83e3a3 in add_object_array_with_path object.c:337:3
    #3 0x8f672a in add_pending_object_with_path revision.c:329:2
    #4 0x8eaeab in add_pending_object_with_mode revision.c:336:2
    #5 0x8eae9d in add_pending_object revision.c:342:2
    #6 0x5154a0 in show_local_changes builtin/checkout.c:602:2
    #7 0x513b00 in merge_working_tree builtin/checkout.c:979:3
    git#8 0x512cb3 in switch_branches builtin/checkout.c:1242:9
    git#9 0x50f8de in checkout_branch builtin/checkout.c:1646:9
    git#10 0x50ba12 in checkout_main builtin/checkout.c:2003:9
    git#11 0x5086c0 in cmd_checkout builtin/checkout.c:2055:8
    git#12 0x4cd91d in run_builtin git.c:467:11
    git#13 0x4cb5f3 in handle_builtin git.c:719:3
    git#14 0x4ccf47 in run_argv git.c:808:4
    git#15 0x4caf49 in cmd_main git.c:939:19
    git#16 0x69e43e in main common-main.c:52:11
    git#17 0x7f5dd1d50349 in __libc_start_main (/lib64/libc.so.6+0x24349)

SUMMARY: AddressSanitizer: 2048 byte(s) leaked in 1 allocation(s).
Signed-off-by: Andrzej Hunt <ajrhunt@google.com>
ahunt added a commit to ahunt/git that referenced this pull request Apr 9, 2021
add_pending_object() populates rev.pending, we need to take care of
clearing it once we're done.

This code is run close to the end of a checkout, therefore this leak
seems like it would have very little impact. See also LSAN output
from t0020 below:

Direct leak of 2048 byte(s) in 1 object(s) allocated from:
    #0 0x49ab79 in realloc ../projects/compiler-rt/lib/asan/asan_malloc_linux.cpp:164:3
    #1 0x9acc46 in xrealloc wrapper.c:126:8
    #2 0x83e3a3 in add_object_array_with_path object.c:337:3
    #3 0x8f672a in add_pending_object_with_path revision.c:329:2
    #4 0x8eaeab in add_pending_object_with_mode revision.c:336:2
    #5 0x8eae9d in add_pending_object revision.c:342:2
    #6 0x5154a0 in show_local_changes builtin/checkout.c:602:2
    #7 0x513b00 in merge_working_tree builtin/checkout.c:979:3
    git#8 0x512cb3 in switch_branches builtin/checkout.c:1242:9
    git#9 0x50f8de in checkout_branch builtin/checkout.c:1646:9
    git#10 0x50ba12 in checkout_main builtin/checkout.c:2003:9
    git#11 0x5086c0 in cmd_checkout builtin/checkout.c:2055:8
    git#12 0x4cd91d in run_builtin git.c:467:11
    git#13 0x4cb5f3 in handle_builtin git.c:719:3
    git#14 0x4ccf47 in run_argv git.c:808:4
    git#15 0x4caf49 in cmd_main git.c:939:19
    git#16 0x69e43e in main common-main.c:52:11
    git#17 0x7f5dd1d50349 in __libc_start_main (/lib64/libc.so.6+0x24349)

SUMMARY: AddressSanitizer: 2048 byte(s) leaked in 1 allocation(s).
Signed-off-by: Andrzej Hunt <ajrhunt@google.com>
ahunt added a commit to ahunt/git that referenced this pull request Apr 9, 2021
add_pending_object() populates rev.pending, we need to take care of
clearing it once we're done.

This code is run close to the end of a checkout, therefore this leak
seems like it would have very little impact. See also LSAN output
from t0020 below:

Direct leak of 2048 byte(s) in 1 object(s) allocated from:
    #0 0x49ab79 in realloc ../projects/compiler-rt/lib/asan/asan_malloc_linux.cpp:164:3
    #1 0x9acc46 in xrealloc wrapper.c:126:8
    #2 0x83e3a3 in add_object_array_with_path object.c:337:3
    #3 0x8f672a in add_pending_object_with_path revision.c:329:2
    #4 0x8eaeab in add_pending_object_with_mode revision.c:336:2
    #5 0x8eae9d in add_pending_object revision.c:342:2
    #6 0x5154a0 in show_local_changes builtin/checkout.c:602:2
    #7 0x513b00 in merge_working_tree builtin/checkout.c:979:3
    git#8 0x512cb3 in switch_branches builtin/checkout.c:1242:9
    git#9 0x50f8de in checkout_branch builtin/checkout.c:1646:9
    git#10 0x50ba12 in checkout_main builtin/checkout.c:2003:9
    git#11 0x5086c0 in cmd_checkout builtin/checkout.c:2055:8
    git#12 0x4cd91d in run_builtin git.c:467:11
    git#13 0x4cb5f3 in handle_builtin git.c:719:3
    git#14 0x4ccf47 in run_argv git.c:808:4
    git#15 0x4caf49 in cmd_main git.c:939:19
    git#16 0x69e43e in main common-main.c:52:11
    git#17 0x7f5dd1d50349 in __libc_start_main (/lib64/libc.so.6+0x24349)

SUMMARY: AddressSanitizer: 2048 byte(s) leaked in 1 allocation(s).
Signed-off-by: Andrzej Hunt <ajrhunt@google.com>
ahunt added a commit to ahunt/git that referenced this pull request Apr 9, 2021
add_pending_object() populates rev.pending, we need to take care of
clearing it once we're done.

This code is run close to the end of a checkout, therefore this leak
seems like it would have very little impact. See also LSAN output
from t0020 below:

Direct leak of 2048 byte(s) in 1 object(s) allocated from:
    #0 0x49ab79 in realloc ../projects/compiler-rt/lib/asan/asan_malloc_linux.cpp:164:3
    #1 0x9acc46 in xrealloc wrapper.c:126:8
    #2 0x83e3a3 in add_object_array_with_path object.c:337:3
    #3 0x8f672a in add_pending_object_with_path revision.c:329:2
    #4 0x8eaeab in add_pending_object_with_mode revision.c:336:2
    #5 0x8eae9d in add_pending_object revision.c:342:2
    #6 0x5154a0 in show_local_changes builtin/checkout.c:602:2
    #7 0x513b00 in merge_working_tree builtin/checkout.c:979:3
    git#8 0x512cb3 in switch_branches builtin/checkout.c:1242:9
    git#9 0x50f8de in checkout_branch builtin/checkout.c:1646:9
    git#10 0x50ba12 in checkout_main builtin/checkout.c:2003:9
    git#11 0x5086c0 in cmd_checkout builtin/checkout.c:2055:8
    git#12 0x4cd91d in run_builtin git.c:467:11
    git#13 0x4cb5f3 in handle_builtin git.c:719:3
    git#14 0x4ccf47 in run_argv git.c:808:4
    git#15 0x4caf49 in cmd_main git.c:939:19
    git#16 0x69e43e in main common-main.c:52:11
    git#17 0x7f5dd1d50349 in __libc_start_main (/lib64/libc.so.6+0x24349)

SUMMARY: AddressSanitizer: 2048 byte(s) leaked in 1 allocation(s).
Signed-off-by: Andrzej Hunt <ajrhunt@google.com>
gitster pushed a commit that referenced this pull request Apr 11, 2021
add_pending_object() populates rev.pending, we need to take care of
clearing it once we're done.

This code is run close to the end of a checkout, therefore this leak
seems like it would have very little impact. See also LSAN output
from t0020 below:

Direct leak of 2048 byte(s) in 1 object(s) allocated from:
    #0 0x49ab79 in realloc ../projects/compiler-rt/lib/asan/asan_malloc_linux.cpp:164:3
    #1 0x9acc46 in xrealloc wrapper.c:126:8
    #2 0x83e3a3 in add_object_array_with_path object.c:337:3
    #3 0x8f672a in add_pending_object_with_path revision.c:329:2
    #4 0x8eaeab in add_pending_object_with_mode revision.c:336:2
    #5 0x8eae9d in add_pending_object revision.c:342:2
    #6 0x5154a0 in show_local_changes builtin/checkout.c:602:2
    #7 0x513b00 in merge_working_tree builtin/checkout.c:979:3
    #8 0x512cb3 in switch_branches builtin/checkout.c:1242:9
    #9 0x50f8de in checkout_branch builtin/checkout.c:1646:9
    #10 0x50ba12 in checkout_main builtin/checkout.c:2003:9
    #11 0x5086c0 in cmd_checkout builtin/checkout.c:2055:8
    #12 0x4cd91d in run_builtin git.c:467:11
    #13 0x4cb5f3 in handle_builtin git.c:719:3
    #14 0x4ccf47 in run_argv git.c:808:4
    #15 0x4caf49 in cmd_main git.c:939:19
    #16 0x69e43e in main common-main.c:52:11
    #17 0x7f5dd1d50349 in __libc_start_main (/lib64/libc.so.6+0x24349)

SUMMARY: AddressSanitizer: 2048 byte(s) leaked in 1 allocation(s).
Signed-off-by: Andrzej Hunt <ajrhunt@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
ahunt added a commit to ahunt/git that referenced this pull request Apr 25, 2021
limit_list() iterates over the original revs->commits list, and consumes
many of its entries via pop_commit. However we might stop iterating over
the list early (e.g. if we realise that the rest of the list is
uninteresting). If we do stop iterating early, list will be pointing to
the unconsumed portion of revs->commits - and we need to free this list
to avoid a leak. (revs->commits itself will be an invalid pointer: it
will have been free'd during the first pop_commit.)

This leak was found while running t0090. It's not likely to be very
impactful, but it can happen quite early during some checkout
invocations, and hence seems to be worth fixing:

Direct leak of 16 byte(s) in 1 object(s) allocated from:
    #0 0x49a85d in malloc ../projects/compiler-rt/lib/asan/asan_malloc_linux.cpp:145:3
    #1 0x9ac084 in do_xmalloc wrapper.c:41:8
    #2 0x9ac05a in xmalloc wrapper.c:62:9
    #3 0x7175d6 in commit_list_insert commit.c:540:33
    #4 0x71800f in commit_list_insert_by_date commit.c:604:9
    #5 0x8f8d2e in process_parents revision.c:1128:5
    #6 0x8f2f2c in limit_list revision.c:1418:7
    #7 0x8f210e in prepare_revision_walk revision.c:3577:7
    git#8 0x514170 in orphaned_commit_warning builtin/checkout.c:1185:6
    git#9 0x512f05 in switch_branches builtin/checkout.c:1250:3
    git#10 0x50f8de in checkout_branch builtin/checkout.c:1646:9
    git#11 0x50ba12 in checkout_main builtin/checkout.c:2003:9
    git#12 0x5086c0 in cmd_checkout builtin/checkout.c:2055:8
    git#13 0x4cd91d in run_builtin git.c:467:11
    git#14 0x4cb5f3 in handle_builtin git.c:719:3
    git#15 0x4ccf47 in run_argv git.c:808:4
    git#16 0x4caf49 in cmd_main git.c:939:19
    git#17 0x69dc0e in main common-main.c:52:11
    git#18 0x7faaabd0e349 in __libc_start_main (/lib64/libc.so.6+0x24349)

Indirect leak of 48 byte(s) in 3 object(s) allocated from:
    #0 0x49a85d in malloc ../projects/compiler-rt/lib/asan/asan_malloc_linux.cpp:145:3
    #1 0x9ac084 in do_xmalloc wrapper.c:41:8
    #2 0x9ac05a in xmalloc wrapper.c:62:9
    #3 0x717de6 in commit_list_append commit.c:1609:35
    #4 0x8f1f9b in prepare_revision_walk revision.c:3554:12
    #5 0x514170 in orphaned_commit_warning builtin/checkout.c:1185:6
    #6 0x512f05 in switch_branches builtin/checkout.c:1250:3
    #7 0x50f8de in checkout_branch builtin/checkout.c:1646:9
    git#8 0x50ba12 in checkout_main builtin/checkout.c:2003:9
    git#9 0x5086c0 in cmd_checkout builtin/checkout.c:2055:8
    git#10 0x4cd91d in run_builtin git.c:467:11
    git#11 0x4cb5f3 in handle_builtin git.c:719:3
    git#12 0x4ccf47 in run_argv git.c:808:4
    git#13 0x4caf49 in cmd_main git.c:939:19
    git#14 0x69dc0e in main common-main.c:52:11
    git#15 0x7faaabd0e349 in __libc_start_main (/lib64/libc.so.6+0x24349)

Signed-off-by: Andrzej Hunt <ajrhunt@google.com>
ahunt added a commit to ahunt/git that referenced this pull request Jun 10, 2021
Depending on the hashing algorithm being used, only part of
object_id.hash is actually being used. Therefore including it in a
memcmp() is technically incorrect - instead we should be calling oidcmp.

cache_entry contains an object_id, and compare_ce_content() would
include that field when calling memcmp on a subset of the cache_entry.
Instead we choose to exclude the object_id, and call oidcmp separately.

This issue was found when running t1700-split-index with MSAN, see MSAN
output below (on my machine, offset 76 corresponds to 4 bytes after the
start of object_id.hash).

Uninitialized bytes in MemcmpInterceptorCommon at offset 76 inside [0x7f60e7c00118, 92)
==27914==WARNING: MemorySanitizer: use-of-uninitialized-value
    #0 0x4524ee in memcmp /home/abuild/rpmbuild/BUILD/llvm-11.0.0.src/build/../projects/compiler-rt/lib/msan/../sanitizer_common/sanitizer_common_interceptors.inc:873:10
    #1 0xc867ae in compare_ce_content /home/ahunt/git/git/split-index.c:208:8
    #2 0xc859fb in prepare_to_write_split_index /home/ahunt/git/git/split-index.c:336:9
    #3 0xb4bbca in write_split_index /home/ahunt/git/git/read-cache.c:3107:2
    #4 0xb42b4d in write_locked_index /home/ahunt/git/git/read-cache.c:3295:8
    #5 0x638058 in try_merge_strategy /home/ahunt/git/git/builtin/merge.c:758:7
    #6 0x63057f in cmd_merge /home/ahunt/git/git/builtin/merge.c:1663:9
    #7 0x4a1e76 in run_builtin /home/ahunt/git/git/git.c:461:11
    git#8 0x49e1e7 in handle_builtin /home/ahunt/git/git/git.c:714:3
    git#9 0x4a0c08 in run_argv /home/ahunt/git/git/git.c:781:4
    git#10 0x49d5a8 in cmd_main /home/ahunt/git/git/git.c:912:19
    git#11 0x7974da in main /home/ahunt/git/git/common-main.c:52:11
    git#12 0x7f60e928e349 in __libc_start_main (/lib64/libc.so.6+0x24349)
    git#13 0x421bd9 in _start /home/abuild/rpmbuild/BUILD/glibc-2.26/csu/../sysdeps/x86_64/start.S:120

  Uninitialized value was stored to memory at
    #0 0x447eb9 in __msan_memcpy /home/abuild/rpmbuild/BUILD/llvm-11.0.0.src/build/../projects/compiler-rt/lib/msan/msan_interceptors.cpp:1558:3
    #1 0xb4d1e6 in dup_cache_entry /home/ahunt/git/git/read-cache.c:3457:2
    #2 0xd214fa in add_entry /home/ahunt/git/git/unpack-trees.c:215:18
    #3 0xd1fae0 in keep_entry /home/ahunt/git/git/unpack-trees.c:2276:2
    #4 0xd1ff9e in twoway_merge /home/ahunt/git/git/unpack-trees.c:2504:11
    #5 0xd27028 in call_unpack_fn /home/ahunt/git/git/unpack-trees.c:593:12
    #6 0xd2443d in unpack_nondirectories /home/ahunt/git/git/unpack-trees.c:1106:12
    #7 0xd19435 in unpack_callback /home/ahunt/git/git/unpack-trees.c:1306:6
    git#8 0xd0d7ff in traverse_trees /home/ahunt/git/git/tree-walk.c:532:17
    git#9 0xd1773a in unpack_trees /home/ahunt/git/git/unpack-trees.c:1683:9
    git#10 0xdc6370 in checkout /home/ahunt/git/git/merge-ort.c:3590:8
    git#11 0xdc51c3 in merge_switch_to_result /home/ahunt/git/git/merge-ort.c:3728:7
    git#12 0xa195a9 in merge_ort_recursive /home/ahunt/git/git/merge-ort-wrappers.c:58:2
    git#13 0x637fff in try_merge_strategy /home/ahunt/git/git/builtin/merge.c:751:12
    git#14 0x63057f in cmd_merge /home/ahunt/git/git/builtin/merge.c:1663:9
    git#15 0x4a1e76 in run_builtin /home/ahunt/git/git/git.c:461:11
    git#16 0x49e1e7 in handle_builtin /home/ahunt/git/git/git.c:714:3
    git#17 0x4a0c08 in run_argv /home/ahunt/git/git/git.c:781:4
    git#18 0x49d5a8 in cmd_main /home/ahunt/git/git/git.c:912:19
    git#19 0x7974da in main /home/ahunt/git/git/common-main.c:52:11

  Uninitialized value was created by a heap allocation
    #0 0x44e73d in malloc /home/abuild/rpmbuild/BUILD/llvm-11.0.0.src/build/../projects/compiler-rt/lib/msan/msan_interceptors.cpp:901:3
    #1 0xd592f6 in do_xmalloc /home/ahunt/git/git/wrapper.c:41:8
    #2 0xd59248 in xmalloc /home/ahunt/git/git/wrapper.c:62:9
    #3 0xa17088 in mem_pool_alloc_block /home/ahunt/git/git/mem-pool.c:22:6
    #4 0xa16f78 in mem_pool_init /home/ahunt/git/git/mem-pool.c:44:3
    #5 0xb481b8 in load_all_cache_entries /home/ahunt/git/git/read-cache.c
    #6 0xb44d40 in do_read_index /home/ahunt/git/git/read-cache.c:2298:17
    #7 0xb48a1b in read_index_from /home/ahunt/git/git/read-cache.c:2389:8
    git#8 0xbd5a0b in repo_read_index /home/ahunt/git/git/repository.c:276:8
    git#9 0xb4bcaf in repo_read_index_unmerged /home/ahunt/git/git/read-cache.c:3326:2
    git#10 0x62ed26 in cmd_merge /home/ahunt/git/git/builtin/merge.c:1362:6
    git#11 0x4a1e76 in run_builtin /home/ahunt/git/git/git.c:461:11
    git#12 0x49e1e7 in handle_builtin /home/ahunt/git/git/git.c:714:3
    git#13 0x4a0c08 in run_argv /home/ahunt/git/git/git.c:781:4
    git#14 0x49d5a8 in cmd_main /home/ahunt/git/git/git.c:912:19
    git#15 0x7974da in main /home/ahunt/git/git/common-main.c:52:11
    git#16 0x7f60e928e349 in __libc_start_main (/lib64/libc.so.6+0x24349)

SUMMARY: MemorySanitizer: use-of-uninitialized-value /home/abuild/rpmbuild/BUILD/llvm-11.0.0.src/build/../projects/compiler-rt/lib/msan/../sanitizer_common/sanitizer_common_interceptors.inc:873:10 in memcmp
Exiting

Signed-off-by: Andrzej Hunt <andrzej@ahunt.org>
ahunt added a commit to ahunt/git that referenced this pull request Jun 10, 2021
Depending on the hashing algorithm being used, only part of
object_id.hash is actually being used. Therefore including it in a
memcmp() is technically incorrect - instead we should be calling oidcmp.

cache_entry contains an object_id, and compare_ce_content() would
include that field when calling memcmp on a subset of the cache_entry.
Instead we choose to exclude the object_id, and call oidcmp separately.

This issue was found when running t1700-split-index with MSAN, see MSAN
output below (on my machine, offset 76 corresponds to 4 bytes after the
start of object_id.hash).

Uninitialized bytes in MemcmpInterceptorCommon at offset 76 inside [0x7f60e7c00118, 92)
==27914==WARNING: MemorySanitizer: use-of-uninitialized-value
    #0 0x4524ee in memcmp /home/abuild/rpmbuild/BUILD/llvm-11.0.0.src/build/../projects/compiler-rt/lib/msan/../sanitizer_common/sanitizer_common_interceptors.inc:873:10
    #1 0xc867ae in compare_ce_content /home/ahunt/git/git/split-index.c:208:8
    #2 0xc859fb in prepare_to_write_split_index /home/ahunt/git/git/split-index.c:336:9
    #3 0xb4bbca in write_split_index /home/ahunt/git/git/read-cache.c:3107:2
    #4 0xb42b4d in write_locked_index /home/ahunt/git/git/read-cache.c:3295:8
    #5 0x638058 in try_merge_strategy /home/ahunt/git/git/builtin/merge.c:758:7
    #6 0x63057f in cmd_merge /home/ahunt/git/git/builtin/merge.c:1663:9
    #7 0x4a1e76 in run_builtin /home/ahunt/git/git/git.c:461:11
    git#8 0x49e1e7 in handle_builtin /home/ahunt/git/git/git.c:714:3
    git#9 0x4a0c08 in run_argv /home/ahunt/git/git/git.c:781:4
    git#10 0x49d5a8 in cmd_main /home/ahunt/git/git/git.c:912:19
    git#11 0x7974da in main /home/ahunt/git/git/common-main.c:52:11
    git#12 0x7f60e928e349 in __libc_start_main (/lib64/libc.so.6+0x24349)
    git#13 0x421bd9 in _start /home/abuild/rpmbuild/BUILD/glibc-2.26/csu/../sysdeps/x86_64/start.S:120

  Uninitialized value was stored to memory at
    #0 0x447eb9 in __msan_memcpy /home/abuild/rpmbuild/BUILD/llvm-11.0.0.src/build/../projects/compiler-rt/lib/msan/msan_interceptors.cpp:1558:3
    #1 0xb4d1e6 in dup_cache_entry /home/ahunt/git/git/read-cache.c:3457:2
    #2 0xd214fa in add_entry /home/ahunt/git/git/unpack-trees.c:215:18
    #3 0xd1fae0 in keep_entry /home/ahunt/git/git/unpack-trees.c:2276:2
    #4 0xd1ff9e in twoway_merge /home/ahunt/git/git/unpack-trees.c:2504:11
    #5 0xd27028 in call_unpack_fn /home/ahunt/git/git/unpack-trees.c:593:12
    #6 0xd2443d in unpack_nondirectories /home/ahunt/git/git/unpack-trees.c:1106:12
    #7 0xd19435 in unpack_callback /home/ahunt/git/git/unpack-trees.c:1306:6
    git#8 0xd0d7ff in traverse_trees /home/ahunt/git/git/tree-walk.c:532:17
    git#9 0xd1773a in unpack_trees /home/ahunt/git/git/unpack-trees.c:1683:9
    git#10 0xdc6370 in checkout /home/ahunt/git/git/merge-ort.c:3590:8
    git#11 0xdc51c3 in merge_switch_to_result /home/ahunt/git/git/merge-ort.c:3728:7
    git#12 0xa195a9 in merge_ort_recursive /home/ahunt/git/git/merge-ort-wrappers.c:58:2
    git#13 0x637fff in try_merge_strategy /home/ahunt/git/git/builtin/merge.c:751:12
    git#14 0x63057f in cmd_merge /home/ahunt/git/git/builtin/merge.c:1663:9
    git#15 0x4a1e76 in run_builtin /home/ahunt/git/git/git.c:461:11
    git#16 0x49e1e7 in handle_builtin /home/ahunt/git/git/git.c:714:3
    git#17 0x4a0c08 in run_argv /home/ahunt/git/git/git.c:781:4
    git#18 0x49d5a8 in cmd_main /home/ahunt/git/git/git.c:912:19
    git#19 0x7974da in main /home/ahunt/git/git/common-main.c:52:11

  Uninitialized value was created by a heap allocation
    #0 0x44e73d in malloc /home/abuild/rpmbuild/BUILD/llvm-11.0.0.src/build/../projects/compiler-rt/lib/msan/msan_interceptors.cpp:901:3
    #1 0xd592f6 in do_xmalloc /home/ahunt/git/git/wrapper.c:41:8
    #2 0xd59248 in xmalloc /home/ahunt/git/git/wrapper.c:62:9
    #3 0xa17088 in mem_pool_alloc_block /home/ahunt/git/git/mem-pool.c:22:6
    #4 0xa16f78 in mem_pool_init /home/ahunt/git/git/mem-pool.c:44:3
    #5 0xb481b8 in load_all_cache_entries /home/ahunt/git/git/read-cache.c
    #6 0xb44d40 in do_read_index /home/ahunt/git/git/read-cache.c:2298:17
    #7 0xb48a1b in read_index_from /home/ahunt/git/git/read-cache.c:2389:8
    git#8 0xbd5a0b in repo_read_index /home/ahunt/git/git/repository.c:276:8
    git#9 0xb4bcaf in repo_read_index_unmerged /home/ahunt/git/git/read-cache.c:3326:2
    git#10 0x62ed26 in cmd_merge /home/ahunt/git/git/builtin/merge.c:1362:6
    git#11 0x4a1e76 in run_builtin /home/ahunt/git/git/git.c:461:11
    git#12 0x49e1e7 in handle_builtin /home/ahunt/git/git/git.c:714:3
    git#13 0x4a0c08 in run_argv /home/ahunt/git/git/git.c:781:4
    git#14 0x49d5a8 in cmd_main /home/ahunt/git/git/git.c:912:19
    git#15 0x7974da in main /home/ahunt/git/git/common-main.c:52:11
    git#16 0x7f60e928e349 in __libc_start_main (/lib64/libc.so.6+0x24349)

SUMMARY: MemorySanitizer: use-of-uninitialized-value /home/abuild/rpmbuild/BUILD/llvm-11.0.0.src/build/../projects/compiler-rt/lib/msan/../sanitizer_common/sanitizer_common_interceptors.inc:873:10 in memcmp
Exiting

Signed-off-by: Andrzej Hunt <andrzej@ahunt.org>
ahunt added a commit to ahunt/git that referenced this pull request Jun 10, 2021
Depending on the hashing algorithm being used, only part of
object_id.hash is actually being used. Therefore including it in a
memcmp() is technically incorrect - instead we should be calling oidcmp.

cache_entry contains an object_id, and compare_ce_content() would
include that field when calling memcmp on a subset of the cache_entry.
Instead we choose to exclude the object_id, and call oidcmp separately.

This issue was found when running t1700-split-index with MSAN, see MSAN
output below (on my machine, offset 76 corresponds to 4 bytes after the
start of object_id.hash).

Uninitialized bytes in MemcmpInterceptorCommon at offset 76 inside [0x7f60e7c00118, 92)
==27914==WARNING: MemorySanitizer: use-of-uninitialized-value
    #0 0x4524ee in memcmp /home/abuild/rpmbuild/BUILD/llvm-11.0.0.src/build/../projects/compiler-rt/lib/msan/../sanitizer_common/sanitizer_common_interceptors.inc:873:10
    #1 0xc867ae in compare_ce_content /home/ahunt/git/git/split-index.c:208:8
    #2 0xc859fb in prepare_to_write_split_index /home/ahunt/git/git/split-index.c:336:9
    #3 0xb4bbca in write_split_index /home/ahunt/git/git/read-cache.c:3107:2
    #4 0xb42b4d in write_locked_index /home/ahunt/git/git/read-cache.c:3295:8
    #5 0x638058 in try_merge_strategy /home/ahunt/git/git/builtin/merge.c:758:7
    #6 0x63057f in cmd_merge /home/ahunt/git/git/builtin/merge.c:1663:9
    #7 0x4a1e76 in run_builtin /home/ahunt/git/git/git.c:461:11
    git#8 0x49e1e7 in handle_builtin /home/ahunt/git/git/git.c:714:3
    git#9 0x4a0c08 in run_argv /home/ahunt/git/git/git.c:781:4
    git#10 0x49d5a8 in cmd_main /home/ahunt/git/git/git.c:912:19
    git#11 0x7974da in main /home/ahunt/git/git/common-main.c:52:11
    git#12 0x7f60e928e349 in __libc_start_main (/lib64/libc.so.6+0x24349)
    git#13 0x421bd9 in _start /home/abuild/rpmbuild/BUILD/glibc-2.26/csu/../sysdeps/x86_64/start.S:120

  Uninitialized value was stored to memory at
    #0 0x447eb9 in __msan_memcpy /home/abuild/rpmbuild/BUILD/llvm-11.0.0.src/build/../projects/compiler-rt/lib/msan/msan_interceptors.cpp:1558:3
    #1 0xb4d1e6 in dup_cache_entry /home/ahunt/git/git/read-cache.c:3457:2
    #2 0xd214fa in add_entry /home/ahunt/git/git/unpack-trees.c:215:18
    #3 0xd1fae0 in keep_entry /home/ahunt/git/git/unpack-trees.c:2276:2
    #4 0xd1ff9e in twoway_merge /home/ahunt/git/git/unpack-trees.c:2504:11
    #5 0xd27028 in call_unpack_fn /home/ahunt/git/git/unpack-trees.c:593:12
    #6 0xd2443d in unpack_nondirectories /home/ahunt/git/git/unpack-trees.c:1106:12
    #7 0xd19435 in unpack_callback /home/ahunt/git/git/unpack-trees.c:1306:6
    git#8 0xd0d7ff in traverse_trees /home/ahunt/git/git/tree-walk.c:532:17
    git#9 0xd1773a in unpack_trees /home/ahunt/git/git/unpack-trees.c:1683:9
    git#10 0xdc6370 in checkout /home/ahunt/git/git/merge-ort.c:3590:8
    git#11 0xdc51c3 in merge_switch_to_result /home/ahunt/git/git/merge-ort.c:3728:7
    git#12 0xa195a9 in merge_ort_recursive /home/ahunt/git/git/merge-ort-wrappers.c:58:2
    git#13 0x637fff in try_merge_strategy /home/ahunt/git/git/builtin/merge.c:751:12
    git#14 0x63057f in cmd_merge /home/ahunt/git/git/builtin/merge.c:1663:9
    git#15 0x4a1e76 in run_builtin /home/ahunt/git/git/git.c:461:11
    git#16 0x49e1e7 in handle_builtin /home/ahunt/git/git/git.c:714:3
    git#17 0x4a0c08 in run_argv /home/ahunt/git/git/git.c:781:4
    git#18 0x49d5a8 in cmd_main /home/ahunt/git/git/git.c:912:19
    git#19 0x7974da in main /home/ahunt/git/git/common-main.c:52:11

  Uninitialized value was created by a heap allocation
    #0 0x44e73d in malloc /home/abuild/rpmbuild/BUILD/llvm-11.0.0.src/build/../projects/compiler-rt/lib/msan/msan_interceptors.cpp:901:3
    #1 0xd592f6 in do_xmalloc /home/ahunt/git/git/wrapper.c:41:8
    #2 0xd59248 in xmalloc /home/ahunt/git/git/wrapper.c:62:9
    #3 0xa17088 in mem_pool_alloc_block /home/ahunt/git/git/mem-pool.c:22:6
    #4 0xa16f78 in mem_pool_init /home/ahunt/git/git/mem-pool.c:44:3
    #5 0xb481b8 in load_all_cache_entries /home/ahunt/git/git/read-cache.c
    #6 0xb44d40 in do_read_index /home/ahunt/git/git/read-cache.c:2298:17
    #7 0xb48a1b in read_index_from /home/ahunt/git/git/read-cache.c:2389:8
    git#8 0xbd5a0b in repo_read_index /home/ahunt/git/git/repository.c:276:8
    git#9 0xb4bcaf in repo_read_index_unmerged /home/ahunt/git/git/read-cache.c:3326:2
    git#10 0x62ed26 in cmd_merge /home/ahunt/git/git/builtin/merge.c:1362:6
    git#11 0x4a1e76 in run_builtin /home/ahunt/git/git/git.c:461:11
    git#12 0x49e1e7 in handle_builtin /home/ahunt/git/git/git.c:714:3
    git#13 0x4a0c08 in run_argv /home/ahunt/git/git/git.c:781:4
    git#14 0x49d5a8 in cmd_main /home/ahunt/git/git/git.c:912:19
    git#15 0x7974da in main /home/ahunt/git/git/common-main.c:52:11
    git#16 0x7f60e928e349 in __libc_start_main (/lib64/libc.so.6+0x24349)

SUMMARY: MemorySanitizer: use-of-uninitialized-value /home/abuild/rpmbuild/BUILD/llvm-11.0.0.src/build/../projects/compiler-rt/lib/msan/../sanitizer_common/sanitizer_common_interceptors.inc:873:10 in memcmp
Exiting

Signed-off-by: Andrzej Hunt <andrzej@ahunt.org>
ahunt added a commit to ahunt/git that referenced this pull request Jun 10, 2021
ibuf can be reused for multiple iterations of the loop. Specifically:
deflate() overwrites s.avail_in to show how much of the input buffer
has not been processed yet - and sometimes leaves 'avail_in > 0', in
which case ibuf will be processed again during the loop's subsequent
iteration.

But if we declare ibuf within the loop, then (in theory) we get a new
(and uninitialised) buffer for every iteration. In practice, my compiler
seems to resue the same buffer - meaning that this code does work - but
it doesn't seem safe to rely on this behaviour. MSAN correctly catches
this issue - as soon as we hit the 's.avail_in > 0' condition, we end up
reading from what seems to be uninitialised memory.

Therefore, we move ibuf out of the loop, making this reuse safe.

See MSAN output from t1050-large below - the interesting part is the
ibuf creation at the end, although there's a lot of indirection before
we reach the read from unitialised memory:

==11294==WARNING: MemorySanitizer: use-of-uninitialized-value
    #0 0x7f75db58fb1c in crc32_little crc32.c:283:9
    #1 0x7f75db58d5b3 in crc32_z crc32.c:220:20
    #2 0x7f75db59668c in crc32 crc32.c:242:12
    #3 0x8c94f8 in hashwrite csum-file.c:101:15
    #4 0x825faf in stream_to_pack bulk-checkin.c:154:5
    #5 0x82467b in deflate_to_pack bulk-checkin.c:225:8
    #6 0x823ff1 in index_bulk_checkin bulk-checkin.c:264:15
    #7 0xa7cff2 in index_stream object-file.c:2234:9
    git#8 0xa7bff7 in index_fd object-file.c:2256:9
    git#9 0xa7d22d in index_path object-file.c:2274:7
    git#10 0xb3c8c9 in add_to_index read-cache.c:802:7
    git#11 0xb3e039 in add_file_to_index read-cache.c:835:9
    git#12 0x4a99c3 in add_files add.c:458:7
    git#13 0x4a7276 in cmd_add add.c:670:18
    git#14 0x4a1e76 in run_builtin git.c:461:11
    git#15 0x49e1e7 in handle_builtin git.c:714:3
    git#16 0x4a0c08 in run_argv git.c:781:4
    git#17 0x49d5a8 in cmd_main git.c:912:19
    git#18 0x7974da in main common-main.c:52:11
    git#19 0x7f75da66f349 in __libc_start_main (/lib64/libc.so.6+0x24349)
    git#20 0x421bd9 in _start start.S:120

  Uninitialized value was stored to memory at
    #0 0x7f75db58fa6b in crc32_little crc32.c:283:9
    #1 0x7f75db58d5b3 in crc32_z crc32.c:220:20
    #2 0x7f75db59668c in crc32 crc32.c:242:12
    #3 0x8c94f8 in hashwrite csum-file.c:101:15
    #4 0x825faf in stream_to_pack bulk-checkin.c:154:5
    #5 0x82467b in deflate_to_pack bulk-checkin.c:225:8
    #6 0x823ff1 in index_bulk_checkin bulk-checkin.c:264:15
    #7 0xa7cff2 in index_stream object-file.c:2234:9
    git#8 0xa7bff7 in index_fd object-file.c:2256:9
    git#9 0xa7d22d in index_path object-file.c:2274:7
    git#10 0xb3c8c9 in add_to_index read-cache.c:802:7
    git#11 0xb3e039 in add_file_to_index read-cache.c:835:9
    git#12 0x4a99c3 in add_files add.c:458:7
    git#13 0x4a7276 in cmd_add add.c:670:18
    git#14 0x4a1e76 in run_builtin git.c:461:11
    git#15 0x49e1e7 in handle_builtin git.c:714:3
    git#16 0x4a0c08 in run_argv git.c:781:4
    git#17 0x49d5a8 in cmd_main git.c:912:19
    git#18 0x7974da in main common-main.c:52:11
    git#19 0x7f75da66f349 in __libc_start_main (/lib64/libc.so.6+0x24349)

  Uninitialized value was stored to memory at
    #0 0x447eb9 in __msan_memcpy msan_interceptors.cpp:1558:3
    #1 0x7f75db5c2011 in flush_pending deflate.c:746:5
    #2 0x7f75db5cafa0 in deflate_stored deflate.c:1815:9
    #3 0x7f75db5bb7d2 in deflate deflate.c:1005:34
    #4 0xd80b7f in git_deflate zlib.c:244:12
    #5 0x825dff in stream_to_pack bulk-checkin.c:140:12
    #6 0x82467b in deflate_to_pack bulk-checkin.c:225:8
    #7 0x823ff1 in index_bulk_checkin bulk-checkin.c:264:15
    git#8 0xa7cff2 in index_stream object-file.c:2234:9
    git#9 0xa7bff7 in index_fd object-file.c:2256:9
    git#10 0xa7d22d in index_path object-file.c:2274:7
    git#11 0xb3c8c9 in add_to_index read-cache.c:802:7
    git#12 0xb3e039 in add_file_to_index read-cache.c:835:9
    git#13 0x4a99c3 in add_files add.c:458:7
    git#14 0x4a7276 in cmd_add add.c:670:18
    git#15 0x4a1e76 in run_builtin git.c:461:11
    git#16 0x49e1e7 in handle_builtin git.c:714:3
    git#17 0x4a0c08 in run_argv git.c:781:4
    git#18 0x49d5a8 in cmd_main git.c:912:19
    git#19 0x7974da in main common-main.c:52:11

  Uninitialized value was stored to memory at
    #0 0x447eb9 in __msan_memcpy msan_interceptors.cpp:1558:3
    #1 0x7f75db644241 in _tr_stored_block trees.c:873:5
    #2 0x7f75db5cad7c in deflate_stored deflate.c:1813:9
    #3 0x7f75db5bb7d2 in deflate deflate.c:1005:34
    #4 0xd80b7f in git_deflate zlib.c:244:12
    #5 0x825dff in stream_to_pack bulk-checkin.c:140:12
    #6 0x82467b in deflate_to_pack bulk-checkin.c:225:8
    #7 0x823ff1 in index_bulk_checkin bulk-checkin.c:264:15
    git#8 0xa7cff2 in index_stream object-file.c:2234:9
    git#9 0xa7bff7 in index_fd object-file.c:2256:9
    git#10 0xa7d22d in index_path object-file.c:2274:7
    git#11 0xb3c8c9 in add_to_index read-cache.c:802:7
    git#12 0xb3e039 in add_file_to_index read-cache.c:835:9
    git#13 0x4a99c3 in add_files add.c:458:7
    git#14 0x4a7276 in cmd_add add.c:670:18
    git#15 0x4a1e76 in run_builtin git.c:461:11
    git#16 0x49e1e7 in handle_builtin git.c:714:3
    git#17 0x4a0c08 in run_argv git.c:781:4
    git#18 0x49d5a8 in cmd_main git.c:912:19
    git#19 0x7974da in main common-main.c:52:11

  Uninitialized value was stored to memory at
    #0 0x447eb9 in __msan_memcpy msan_interceptors.cpp:1558:3
    #1 0x7f75db5c8fcf in deflate_stored deflate.c:1783:9
    #2 0x7f75db5bb7d2 in deflate deflate.c:1005:34
    #3 0xd80b7f in git_deflate zlib.c:244:12
    #4 0x825dff in stream_to_pack bulk-checkin.c:140:12
    #5 0x82467b in deflate_to_pack bulk-checkin.c:225:8
    #6 0x823ff1 in index_bulk_checkin bulk-checkin.c:264:15
    #7 0xa7cff2 in index_stream object-file.c:2234:9
    git#8 0xa7bff7 in index_fd object-file.c:2256:9
    git#9 0xa7d22d in index_path object-file.c:2274:7
    git#10 0xb3c8c9 in add_to_index read-cache.c:802:7
    git#11 0xb3e039 in add_file_to_index read-cache.c:835:9
    git#12 0x4a99c3 in add_files add.c:458:7
    git#13 0x4a7276 in cmd_add add.c:670:18
    git#14 0x4a1e76 in run_builtin git.c:461:11
    git#15 0x49e1e7 in handle_builtin git.c:714:3
    git#16 0x4a0c08 in run_argv git.c:781:4
    git#17 0x49d5a8 in cmd_main git.c:912:19
    git#18 0x7974da in main common-main.c:52:11
    git#19 0x7f75da66f349 in __libc_start_main (/lib64/libc.so.6+0x24349)

  Uninitialized value was stored to memory at
    #0 0x447eb9 in __msan_memcpy msan_interceptors.cpp:1558:3
    #1 0x7f75db5ea545 in read_buf deflate.c:1181:5
    #2 0x7f75db5c97f7 in deflate_stored deflate.c:1791:9
    #3 0x7f75db5bb7d2 in deflate deflate.c:1005:34
    #4 0xd80b7f in git_deflate zlib.c:244:12
    #5 0x825dff in stream_to_pack bulk-checkin.c:140:12
    #6 0x82467b in deflate_to_pack bulk-checkin.c:225:8
    #7 0x823ff1 in index_bulk_checkin bulk-checkin.c:264:15
    git#8 0xa7cff2 in index_stream object-file.c:2234:9
    git#9 0xa7bff7 in index_fd object-file.c:2256:9
    git#10 0xa7d22d in index_path object-file.c:2274:7
    git#11 0xb3c8c9 in add_to_index read-cache.c:802:7
    git#12 0xb3e039 in add_file_to_index read-cache.c:835:9
    git#13 0x4a99c3 in add_files add.c:458:7
    git#14 0x4a7276 in cmd_add add.c:670:18
    git#15 0x4a1e76 in run_builtin git.c:461:11
    git#16 0x49e1e7 in handle_builtin git.c:714:3
    git#17 0x4a0c08 in run_argv git.c:781:4
    git#18 0x49d5a8 in cmd_main git.c:912:19
    git#19 0x7974da in main common-main.c:52:11

  Uninitialized value was created by an allocation of 'ibuf' in the stack frame of function 'stream_to_pack'
    #0 0x825710 in stream_to_pack bulk-checkin.c:101

SUMMARY: MemorySanitizer: use-of-uninitialized-value crc32.c:283:9 in crc32_little
Exiting

Signed-off-by: Andrzej Hunt <andrzej@ahunt.org>
ahunt added a commit to ahunt/git that referenced this pull request Jun 10, 2021
cache_entry contains an object_id, and compare_ce_content() would
include that field when calling memcmp on a subset of the cache_entry.
Depending on which hashing algorithm is being used, only part of
object_id.hash is actually being used, therefore including it in a
memcmp() is technically incorrect. Instead we choose to exclude the
object_id when calling memcmp(), and call oidcmp() separately.

This issue was found when running t1700-split-index with MSAN, see MSAN
output below (on my machine, offset 76 corresponds to 4 bytes after the
start of object_id.hash).

Uninitialized bytes in MemcmpInterceptorCommon at offset 76 inside [0x7f60e7c00118, 92)
==27914==WARNING: MemorySanitizer: use-of-uninitialized-value
    #0 0x4524ee in memcmp /home/abuild/rpmbuild/BUILD/llvm-11.0.0.src/build/../projects/compiler-rt/lib/msan/../sanitizer_common/sanitizer_common_interceptors.inc:873:10
    #1 0xc867ae in compare_ce_content /home/ahunt/git/git/split-index.c:208:8
    #2 0xc859fb in prepare_to_write_split_index /home/ahunt/git/git/split-index.c:336:9
    #3 0xb4bbca in write_split_index /home/ahunt/git/git/read-cache.c:3107:2
    #4 0xb42b4d in write_locked_index /home/ahunt/git/git/read-cache.c:3295:8
    #5 0x638058 in try_merge_strategy /home/ahunt/git/git/builtin/merge.c:758:7
    #6 0x63057f in cmd_merge /home/ahunt/git/git/builtin/merge.c:1663:9
    #7 0x4a1e76 in run_builtin /home/ahunt/git/git/git.c:461:11
    git#8 0x49e1e7 in handle_builtin /home/ahunt/git/git/git.c:714:3
    git#9 0x4a0c08 in run_argv /home/ahunt/git/git/git.c:781:4
    git#10 0x49d5a8 in cmd_main /home/ahunt/git/git/git.c:912:19
    git#11 0x7974da in main /home/ahunt/git/git/common-main.c:52:11
    git#12 0x7f60e928e349 in __libc_start_main (/lib64/libc.so.6+0x24349)
    git#13 0x421bd9 in _start /home/abuild/rpmbuild/BUILD/glibc-2.26/csu/../sysdeps/x86_64/start.S:120

  Uninitialized value was stored to memory at
    #0 0x447eb9 in __msan_memcpy /home/abuild/rpmbuild/BUILD/llvm-11.0.0.src/build/../projects/compiler-rt/lib/msan/msan_interceptors.cpp:1558:3
    #1 0xb4d1e6 in dup_cache_entry /home/ahunt/git/git/read-cache.c:3457:2
    #2 0xd214fa in add_entry /home/ahunt/git/git/unpack-trees.c:215:18
    #3 0xd1fae0 in keep_entry /home/ahunt/git/git/unpack-trees.c:2276:2
    #4 0xd1ff9e in twoway_merge /home/ahunt/git/git/unpack-trees.c:2504:11
    #5 0xd27028 in call_unpack_fn /home/ahunt/git/git/unpack-trees.c:593:12
    #6 0xd2443d in unpack_nondirectories /home/ahunt/git/git/unpack-trees.c:1106:12
    #7 0xd19435 in unpack_callback /home/ahunt/git/git/unpack-trees.c:1306:6
    git#8 0xd0d7ff in traverse_trees /home/ahunt/git/git/tree-walk.c:532:17
    git#9 0xd1773a in unpack_trees /home/ahunt/git/git/unpack-trees.c:1683:9
    git#10 0xdc6370 in checkout /home/ahunt/git/git/merge-ort.c:3590:8
    git#11 0xdc51c3 in merge_switch_to_result /home/ahunt/git/git/merge-ort.c:3728:7
    git#12 0xa195a9 in merge_ort_recursive /home/ahunt/git/git/merge-ort-wrappers.c:58:2
    git#13 0x637fff in try_merge_strategy /home/ahunt/git/git/builtin/merge.c:751:12
    git#14 0x63057f in cmd_merge /home/ahunt/git/git/builtin/merge.c:1663:9
    git#15 0x4a1e76 in run_builtin /home/ahunt/git/git/git.c:461:11
    git#16 0x49e1e7 in handle_builtin /home/ahunt/git/git/git.c:714:3
    git#17 0x4a0c08 in run_argv /home/ahunt/git/git/git.c:781:4
    git#18 0x49d5a8 in cmd_main /home/ahunt/git/git/git.c:912:19
    git#19 0x7974da in main /home/ahunt/git/git/common-main.c:52:11

  Uninitialized value was created by a heap allocation
    #0 0x44e73d in malloc /home/abuild/rpmbuild/BUILD/llvm-11.0.0.src/build/../projects/compiler-rt/lib/msan/msan_interceptors.cpp:901:3
    #1 0xd592f6 in do_xmalloc /home/ahunt/git/git/wrapper.c:41:8
    #2 0xd59248 in xmalloc /home/ahunt/git/git/wrapper.c:62:9
    #3 0xa17088 in mem_pool_alloc_block /home/ahunt/git/git/mem-pool.c:22:6
    #4 0xa16f78 in mem_pool_init /home/ahunt/git/git/mem-pool.c:44:3
    #5 0xb481b8 in load_all_cache_entries /home/ahunt/git/git/read-cache.c
    #6 0xb44d40 in do_read_index /home/ahunt/git/git/read-cache.c:2298:17
    #7 0xb48a1b in read_index_from /home/ahunt/git/git/read-cache.c:2389:8
    git#8 0xbd5a0b in repo_read_index /home/ahunt/git/git/repository.c:276:8
    git#9 0xb4bcaf in repo_read_index_unmerged /home/ahunt/git/git/read-cache.c:3326:2
    git#10 0x62ed26 in cmd_merge /home/ahunt/git/git/builtin/merge.c:1362:6
    git#11 0x4a1e76 in run_builtin /home/ahunt/git/git/git.c:461:11
    git#12 0x49e1e7 in handle_builtin /home/ahunt/git/git/git.c:714:3
    git#13 0x4a0c08 in run_argv /home/ahunt/git/git/git.c:781:4
    git#14 0x49d5a8 in cmd_main /home/ahunt/git/git/git.c:912:19
    git#15 0x7974da in main /home/ahunt/git/git/common-main.c:52:11
    git#16 0x7f60e928e349 in __libc_start_main (/lib64/libc.so.6+0x24349)

SUMMARY: MemorySanitizer: use-of-uninitialized-value /home/abuild/rpmbuild/BUILD/llvm-11.0.0.src/build/../projects/compiler-rt/lib/msan/../sanitizer_common/sanitizer_common_interceptors.inc:873:10 in memcmp
Exiting

Signed-off-by: Andrzej Hunt <andrzej@ahunt.org>
ahunt added a commit to ahunt/git that referenced this pull request Jun 10, 2021
cache_entry contains an object_id, and compare_ce_content() would
include that field when calling memcmp on a subset of the cache_entry.
Depending on which hashing algorithm is being used, only part of
object_id.hash is actually being used, therefore including it in a
memcmp() is technically incorrect. Instead we choose to exclude the
object_id when calling memcmp(), and call oideq() separately.

This issue was found when running t1700-split-index with MSAN, see MSAN
output below (on my machine, offset 76 corresponds to 4 bytes after the
start of object_id.hash).

Uninitialized bytes in MemcmpInterceptorCommon at offset 76 inside [0x7f60e7c00118, 92)
==27914==WARNING: MemorySanitizer: use-of-uninitialized-value
    #0 0x4524ee in memcmp /home/abuild/rpmbuild/BUILD/llvm-11.0.0.src/build/../projects/compiler-rt/lib/msan/../sanitizer_common/sanitizer_common_interceptors.inc:873:10
    #1 0xc867ae in compare_ce_content /home/ahunt/git/git/split-index.c:208:8
    #2 0xc859fb in prepare_to_write_split_index /home/ahunt/git/git/split-index.c:336:9
    #3 0xb4bbca in write_split_index /home/ahunt/git/git/read-cache.c:3107:2
    #4 0xb42b4d in write_locked_index /home/ahunt/git/git/read-cache.c:3295:8
    #5 0x638058 in try_merge_strategy /home/ahunt/git/git/builtin/merge.c:758:7
    #6 0x63057f in cmd_merge /home/ahunt/git/git/builtin/merge.c:1663:9
    #7 0x4a1e76 in run_builtin /home/ahunt/git/git/git.c:461:11
    git#8 0x49e1e7 in handle_builtin /home/ahunt/git/git/git.c:714:3
    git#9 0x4a0c08 in run_argv /home/ahunt/git/git/git.c:781:4
    git#10 0x49d5a8 in cmd_main /home/ahunt/git/git/git.c:912:19
    git#11 0x7974da in main /home/ahunt/git/git/common-main.c:52:11
    git#12 0x7f60e928e349 in __libc_start_main (/lib64/libc.so.6+0x24349)
    git#13 0x421bd9 in _start /home/abuild/rpmbuild/BUILD/glibc-2.26/csu/../sysdeps/x86_64/start.S:120

  Uninitialized value was stored to memory at
    #0 0x447eb9 in __msan_memcpy /home/abuild/rpmbuild/BUILD/llvm-11.0.0.src/build/../projects/compiler-rt/lib/msan/msan_interceptors.cpp:1558:3
    #1 0xb4d1e6 in dup_cache_entry /home/ahunt/git/git/read-cache.c:3457:2
    #2 0xd214fa in add_entry /home/ahunt/git/git/unpack-trees.c:215:18
    #3 0xd1fae0 in keep_entry /home/ahunt/git/git/unpack-trees.c:2276:2
    #4 0xd1ff9e in twoway_merge /home/ahunt/git/git/unpack-trees.c:2504:11
    #5 0xd27028 in call_unpack_fn /home/ahunt/git/git/unpack-trees.c:593:12
    #6 0xd2443d in unpack_nondirectories /home/ahunt/git/git/unpack-trees.c:1106:12
    #7 0xd19435 in unpack_callback /home/ahunt/git/git/unpack-trees.c:1306:6
    git#8 0xd0d7ff in traverse_trees /home/ahunt/git/git/tree-walk.c:532:17
    git#9 0xd1773a in unpack_trees /home/ahunt/git/git/unpack-trees.c:1683:9
    git#10 0xdc6370 in checkout /home/ahunt/git/git/merge-ort.c:3590:8
    git#11 0xdc51c3 in merge_switch_to_result /home/ahunt/git/git/merge-ort.c:3728:7
    git#12 0xa195a9 in merge_ort_recursive /home/ahunt/git/git/merge-ort-wrappers.c:58:2
    git#13 0x637fff in try_merge_strategy /home/ahunt/git/git/builtin/merge.c:751:12
    git#14 0x63057f in cmd_merge /home/ahunt/git/git/builtin/merge.c:1663:9
    git#15 0x4a1e76 in run_builtin /home/ahunt/git/git/git.c:461:11
    git#16 0x49e1e7 in handle_builtin /home/ahunt/git/git/git.c:714:3
    git#17 0x4a0c08 in run_argv /home/ahunt/git/git/git.c:781:4
    git#18 0x49d5a8 in cmd_main /home/ahunt/git/git/git.c:912:19
    git#19 0x7974da in main /home/ahunt/git/git/common-main.c:52:11

  Uninitialized value was created by a heap allocation
    #0 0x44e73d in malloc /home/abuild/rpmbuild/BUILD/llvm-11.0.0.src/build/../projects/compiler-rt/lib/msan/msan_interceptors.cpp:901:3
    #1 0xd592f6 in do_xmalloc /home/ahunt/git/git/wrapper.c:41:8
    #2 0xd59248 in xmalloc /home/ahunt/git/git/wrapper.c:62:9
    #3 0xa17088 in mem_pool_alloc_block /home/ahunt/git/git/mem-pool.c:22:6
    #4 0xa16f78 in mem_pool_init /home/ahunt/git/git/mem-pool.c:44:3
    #5 0xb481b8 in load_all_cache_entries /home/ahunt/git/git/read-cache.c
    #6 0xb44d40 in do_read_index /home/ahunt/git/git/read-cache.c:2298:17
    #7 0xb48a1b in read_index_from /home/ahunt/git/git/read-cache.c:2389:8
    git#8 0xbd5a0b in repo_read_index /home/ahunt/git/git/repository.c:276:8
    git#9 0xb4bcaf in repo_read_index_unmerged /home/ahunt/git/git/read-cache.c:3326:2
    git#10 0x62ed26 in cmd_merge /home/ahunt/git/git/builtin/merge.c:1362:6
    git#11 0x4a1e76 in run_builtin /home/ahunt/git/git/git.c:461:11
    git#12 0x49e1e7 in handle_builtin /home/ahunt/git/git/git.c:714:3
    git#13 0x4a0c08 in run_argv /home/ahunt/git/git/git.c:781:4
    git#14 0x49d5a8 in cmd_main /home/ahunt/git/git/git.c:912:19
    git#15 0x7974da in main /home/ahunt/git/git/common-main.c:52:11
    git#16 0x7f60e928e349 in __libc_start_main (/lib64/libc.so.6+0x24349)

SUMMARY: MemorySanitizer: use-of-uninitialized-value /home/abuild/rpmbuild/BUILD/llvm-11.0.0.src/build/../projects/compiler-rt/lib/msan/../sanitizer_common/sanitizer_common_interceptors.inc:873:10 in memcmp
Exiting

Signed-off-by: Andrzej Hunt <andrzej@ahunt.org>
gitster pushed a commit that referenced this pull request Jun 11, 2021
ibuf can be reused for multiple iterations of the loop. Specifically:
deflate() overwrites s.avail_in to show how much of the input buffer
has not been processed yet - and sometimes leaves 'avail_in > 0', in
which case ibuf will be processed again during the loop's subsequent
iteration.

But if we declare ibuf within the loop, then (in theory) we get a new
(and uninitialised) buffer for every iteration. In practice, my compiler
seems to resue the same buffer - meaning that this code does work - but
it doesn't seem safe to rely on this behaviour. MSAN correctly catches
this issue - as soon as we hit the 's.avail_in > 0' condition, we end up
reading from what seems to be uninitialised memory.

Therefore, we move ibuf out of the loop, making this reuse safe.

See MSAN output from t1050-large below - the interesting part is the
ibuf creation at the end, although there's a lot of indirection before
we reach the read from unitialised memory:

==11294==WARNING: MemorySanitizer: use-of-uninitialized-value
    #0 0x7f75db58fb1c in crc32_little crc32.c:283:9
    #1 0x7f75db58d5b3 in crc32_z crc32.c:220:20
    #2 0x7f75db59668c in crc32 crc32.c:242:12
    #3 0x8c94f8 in hashwrite csum-file.c:101:15
    #4 0x825faf in stream_to_pack bulk-checkin.c:154:5
    #5 0x82467b in deflate_to_pack bulk-checkin.c:225:8
    #6 0x823ff1 in index_bulk_checkin bulk-checkin.c:264:15
    #7 0xa7cff2 in index_stream object-file.c:2234:9
    #8 0xa7bff7 in index_fd object-file.c:2256:9
    #9 0xa7d22d in index_path object-file.c:2274:7
    #10 0xb3c8c9 in add_to_index read-cache.c:802:7
    #11 0xb3e039 in add_file_to_index read-cache.c:835:9
    #12 0x4a99c3 in add_files add.c:458:7
    #13 0x4a7276 in cmd_add add.c:670:18
    #14 0x4a1e76 in run_builtin git.c:461:11
    #15 0x49e1e7 in handle_builtin git.c:714:3
    #16 0x4a0c08 in run_argv git.c:781:4
    #17 0x49d5a8 in cmd_main git.c:912:19
    #18 0x7974da in main common-main.c:52:11
    #19 0x7f75da66f349 in __libc_start_main (/lib64/libc.so.6+0x24349)
    #20 0x421bd9 in _start start.S:120

  Uninitialized value was stored to memory at
    #0 0x7f75db58fa6b in crc32_little crc32.c:283:9
    #1 0x7f75db58d5b3 in crc32_z crc32.c:220:20
    #2 0x7f75db59668c in crc32 crc32.c:242:12
    #3 0x8c94f8 in hashwrite csum-file.c:101:15
    #4 0x825faf in stream_to_pack bulk-checkin.c:154:5
    #5 0x82467b in deflate_to_pack bulk-checkin.c:225:8
    #6 0x823ff1 in index_bulk_checkin bulk-checkin.c:264:15
    #7 0xa7cff2 in index_stream object-file.c:2234:9
    #8 0xa7bff7 in index_fd object-file.c:2256:9
    #9 0xa7d22d in index_path object-file.c:2274:7
    #10 0xb3c8c9 in add_to_index read-cache.c:802:7
    #11 0xb3e039 in add_file_to_index read-cache.c:835:9
    #12 0x4a99c3 in add_files add.c:458:7
    #13 0x4a7276 in cmd_add add.c:670:18
    #14 0x4a1e76 in run_builtin git.c:461:11
    #15 0x49e1e7 in handle_builtin git.c:714:3
    #16 0x4a0c08 in run_argv git.c:781:4
    #17 0x49d5a8 in cmd_main git.c:912:19
    #18 0x7974da in main common-main.c:52:11
    #19 0x7f75da66f349 in __libc_start_main (/lib64/libc.so.6+0x24349)

  Uninitialized value was stored to memory at
    #0 0x447eb9 in __msan_memcpy msan_interceptors.cpp:1558:3
    #1 0x7f75db5c2011 in flush_pending deflate.c:746:5
    #2 0x7f75db5cafa0 in deflate_stored deflate.c:1815:9
    #3 0x7f75db5bb7d2 in deflate deflate.c:1005:34
    #4 0xd80b7f in git_deflate zlib.c:244:12
    #5 0x825dff in stream_to_pack bulk-checkin.c:140:12
    #6 0x82467b in deflate_to_pack bulk-checkin.c:225:8
    #7 0x823ff1 in index_bulk_checkin bulk-checkin.c:264:15
    #8 0xa7cff2 in index_stream object-file.c:2234:9
    #9 0xa7bff7 in index_fd object-file.c:2256:9
    #10 0xa7d22d in index_path object-file.c:2274:7
    #11 0xb3c8c9 in add_to_index read-cache.c:802:7
    #12 0xb3e039 in add_file_to_index read-cache.c:835:9
    #13 0x4a99c3 in add_files add.c:458:7
    #14 0x4a7276 in cmd_add add.c:670:18
    #15 0x4a1e76 in run_builtin git.c:461:11
    #16 0x49e1e7 in handle_builtin git.c:714:3
    #17 0x4a0c08 in run_argv git.c:781:4
    #18 0x49d5a8 in cmd_main git.c:912:19
    #19 0x7974da in main common-main.c:52:11

  Uninitialized value was stored to memory at
    #0 0x447eb9 in __msan_memcpy msan_interceptors.cpp:1558:3
    #1 0x7f75db644241 in _tr_stored_block trees.c:873:5
    #2 0x7f75db5cad7c in deflate_stored deflate.c:1813:9
    #3 0x7f75db5bb7d2 in deflate deflate.c:1005:34
    #4 0xd80b7f in git_deflate zlib.c:244:12
    #5 0x825dff in stream_to_pack bulk-checkin.c:140:12
    #6 0x82467b in deflate_to_pack bulk-checkin.c:225:8
    #7 0x823ff1 in index_bulk_checkin bulk-checkin.c:264:15
    #8 0xa7cff2 in index_stream object-file.c:2234:9
    #9 0xa7bff7 in index_fd object-file.c:2256:9
    #10 0xa7d22d in index_path object-file.c:2274:7
    #11 0xb3c8c9 in add_to_index read-cache.c:802:7
    #12 0xb3e039 in add_file_to_index read-cache.c:835:9
    #13 0x4a99c3 in add_files add.c:458:7
    #14 0x4a7276 in cmd_add add.c:670:18
    #15 0x4a1e76 in run_builtin git.c:461:11
    #16 0x49e1e7 in handle_builtin git.c:714:3
    #17 0x4a0c08 in run_argv git.c:781:4
    #18 0x49d5a8 in cmd_main git.c:912:19
    #19 0x7974da in main common-main.c:52:11

  Uninitialized value was stored to memory at
    #0 0x447eb9 in __msan_memcpy msan_interceptors.cpp:1558:3
    #1 0x7f75db5c8fcf in deflate_stored deflate.c:1783:9
    #2 0x7f75db5bb7d2 in deflate deflate.c:1005:34
    #3 0xd80b7f in git_deflate zlib.c:244:12
    #4 0x825dff in stream_to_pack bulk-checkin.c:140:12
    #5 0x82467b in deflate_to_pack bulk-checkin.c:225:8
    #6 0x823ff1 in index_bulk_checkin bulk-checkin.c:264:15
    #7 0xa7cff2 in index_stream object-file.c:2234:9
    #8 0xa7bff7 in index_fd object-file.c:2256:9
    #9 0xa7d22d in index_path object-file.c:2274:7
    #10 0xb3c8c9 in add_to_index read-cache.c:802:7
    #11 0xb3e039 in add_file_to_index read-cache.c:835:9
    #12 0x4a99c3 in add_files add.c:458:7
    #13 0x4a7276 in cmd_add add.c:670:18
    #14 0x4a1e76 in run_builtin git.c:461:11
    #15 0x49e1e7 in handle_builtin git.c:714:3
    #16 0x4a0c08 in run_argv git.c:781:4
    #17 0x49d5a8 in cmd_main git.c:912:19
    #18 0x7974da in main common-main.c:52:11
    #19 0x7f75da66f349 in __libc_start_main (/lib64/libc.so.6+0x24349)

  Uninitialized value was stored to memory at
    #0 0x447eb9 in __msan_memcpy msan_interceptors.cpp:1558:3
    #1 0x7f75db5ea545 in read_buf deflate.c:1181:5
    #2 0x7f75db5c97f7 in deflate_stored deflate.c:1791:9
    #3 0x7f75db5bb7d2 in deflate deflate.c:1005:34
    #4 0xd80b7f in git_deflate zlib.c:244:12
    #5 0x825dff in stream_to_pack bulk-checkin.c:140:12
    #6 0x82467b in deflate_to_pack bulk-checkin.c:225:8
    #7 0x823ff1 in index_bulk_checkin bulk-checkin.c:264:15
    #8 0xa7cff2 in index_stream object-file.c:2234:9
    #9 0xa7bff7 in index_fd object-file.c:2256:9
    #10 0xa7d22d in index_path object-file.c:2274:7
    #11 0xb3c8c9 in add_to_index read-cache.c:802:7
    #12 0xb3e039 in add_file_to_index read-cache.c:835:9
    #13 0x4a99c3 in add_files add.c:458:7
    #14 0x4a7276 in cmd_add add.c:670:18
    #15 0x4a1e76 in run_builtin git.c:461:11
    #16 0x49e1e7 in handle_builtin git.c:714:3
    #17 0x4a0c08 in run_argv git.c:781:4
    #18 0x49d5a8 in cmd_main git.c:912:19
    #19 0x7974da in main common-main.c:52:11

  Uninitialized value was created by an allocation of 'ibuf' in the stack frame of function 'stream_to_pack'
    #0 0x825710 in stream_to_pack bulk-checkin.c:101

SUMMARY: MemorySanitizer: use-of-uninitialized-value crc32.c:283:9 in crc32_little
Exiting

Signed-off-by: Andrzej Hunt <andrzej@ahunt.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
gitster pushed a commit that referenced this pull request Jun 11, 2021
cache_entry contains an object_id, and compare_ce_content() would
include that field when calling memcmp on a subset of the cache_entry.
Depending on which hashing algorithm is being used, only part of
object_id.hash is actually being used, therefore including it in a
memcmp() is technically incorrect. Instead we choose to exclude the
object_id when calling memcmp(), and call oideq() separately.

This issue was found when running t1700-split-index with MSAN, see MSAN
output below (on my machine, offset 76 corresponds to 4 bytes after the
start of object_id.hash).

Uninitialized bytes in MemcmpInterceptorCommon at offset 76 inside [0x7f60e7c00118, 92)
==27914==WARNING: MemorySanitizer: use-of-uninitialized-value
    #0 0x4524ee in memcmp /home/abuild/rpmbuild/BUILD/llvm-11.0.0.src/build/../projects/compiler-rt/lib/msan/../sanitizer_common/sanitizer_common_interceptors.inc:873:10
    #1 0xc867ae in compare_ce_content /home/ahunt/git/git/split-index.c:208:8
    #2 0xc859fb in prepare_to_write_split_index /home/ahunt/git/git/split-index.c:336:9
    #3 0xb4bbca in write_split_index /home/ahunt/git/git/read-cache.c:3107:2
    #4 0xb42b4d in write_locked_index /home/ahunt/git/git/read-cache.c:3295:8
    #5 0x638058 in try_merge_strategy /home/ahunt/git/git/builtin/merge.c:758:7
    #6 0x63057f in cmd_merge /home/ahunt/git/git/builtin/merge.c:1663:9
    #7 0x4a1e76 in run_builtin /home/ahunt/git/git/git.c:461:11
    #8 0x49e1e7 in handle_builtin /home/ahunt/git/git/git.c:714:3
    #9 0x4a0c08 in run_argv /home/ahunt/git/git/git.c:781:4
    #10 0x49d5a8 in cmd_main /home/ahunt/git/git/git.c:912:19
    #11 0x7974da in main /home/ahunt/git/git/common-main.c:52:11
    #12 0x7f60e928e349 in __libc_start_main (/lib64/libc.so.6+0x24349)
    #13 0x421bd9 in _start /home/abuild/rpmbuild/BUILD/glibc-2.26/csu/../sysdeps/x86_64/start.S:120

  Uninitialized value was stored to memory at
    #0 0x447eb9 in __msan_memcpy /home/abuild/rpmbuild/BUILD/llvm-11.0.0.src/build/../projects/compiler-rt/lib/msan/msan_interceptors.cpp:1558:3
    #1 0xb4d1e6 in dup_cache_entry /home/ahunt/git/git/read-cache.c:3457:2
    #2 0xd214fa in add_entry /home/ahunt/git/git/unpack-trees.c:215:18
    #3 0xd1fae0 in keep_entry /home/ahunt/git/git/unpack-trees.c:2276:2
    #4 0xd1ff9e in twoway_merge /home/ahunt/git/git/unpack-trees.c:2504:11
    #5 0xd27028 in call_unpack_fn /home/ahunt/git/git/unpack-trees.c:593:12
    #6 0xd2443d in unpack_nondirectories /home/ahunt/git/git/unpack-trees.c:1106:12
    #7 0xd19435 in unpack_callback /home/ahunt/git/git/unpack-trees.c:1306:6
    #8 0xd0d7ff in traverse_trees /home/ahunt/git/git/tree-walk.c:532:17
    #9 0xd1773a in unpack_trees /home/ahunt/git/git/unpack-trees.c:1683:9
    #10 0xdc6370 in checkout /home/ahunt/git/git/merge-ort.c:3590:8
    #11 0xdc51c3 in merge_switch_to_result /home/ahunt/git/git/merge-ort.c:3728:7
    #12 0xa195a9 in merge_ort_recursive /home/ahunt/git/git/merge-ort-wrappers.c:58:2
    #13 0x637fff in try_merge_strategy /home/ahunt/git/git/builtin/merge.c:751:12
    #14 0x63057f in cmd_merge /home/ahunt/git/git/builtin/merge.c:1663:9
    #15 0x4a1e76 in run_builtin /home/ahunt/git/git/git.c:461:11
    #16 0x49e1e7 in handle_builtin /home/ahunt/git/git/git.c:714:3
    #17 0x4a0c08 in run_argv /home/ahunt/git/git/git.c:781:4
    #18 0x49d5a8 in cmd_main /home/ahunt/git/git/git.c:912:19
    #19 0x7974da in main /home/ahunt/git/git/common-main.c:52:11

  Uninitialized value was created by a heap allocation
    #0 0x44e73d in malloc /home/abuild/rpmbuild/BUILD/llvm-11.0.0.src/build/../projects/compiler-rt/lib/msan/msan_interceptors.cpp:901:3
    #1 0xd592f6 in do_xmalloc /home/ahunt/git/git/wrapper.c:41:8
    #2 0xd59248 in xmalloc /home/ahunt/git/git/wrapper.c:62:9
    #3 0xa17088 in mem_pool_alloc_block /home/ahunt/git/git/mem-pool.c:22:6
    #4 0xa16f78 in mem_pool_init /home/ahunt/git/git/mem-pool.c:44:3
    #5 0xb481b8 in load_all_cache_entries /home/ahunt/git/git/read-cache.c
    #6 0xb44d40 in do_read_index /home/ahunt/git/git/read-cache.c:2298:17
    #7 0xb48a1b in read_index_from /home/ahunt/git/git/read-cache.c:2389:8
    #8 0xbd5a0b in repo_read_index /home/ahunt/git/git/repository.c:276:8
    #9 0xb4bcaf in repo_read_index_unmerged /home/ahunt/git/git/read-cache.c:3326:2
    #10 0x62ed26 in cmd_merge /home/ahunt/git/git/builtin/merge.c:1362:6
    #11 0x4a1e76 in run_builtin /home/ahunt/git/git/git.c:461:11
    #12 0x49e1e7 in handle_builtin /home/ahunt/git/git/git.c:714:3
    #13 0x4a0c08 in run_argv /home/ahunt/git/git/git.c:781:4
    #14 0x49d5a8 in cmd_main /home/ahunt/git/git/git.c:912:19
    #15 0x7974da in main /home/ahunt/git/git/common-main.c:52:11
    #16 0x7f60e928e349 in __libc_start_main (/lib64/libc.so.6+0x24349)

SUMMARY: MemorySanitizer: use-of-uninitialized-value /home/abuild/rpmbuild/BUILD/llvm-11.0.0.src/build/../projects/compiler-rt/lib/msan/../sanitizer_common/sanitizer_common_interceptors.inc:873:10 in memcmp
Exiting

Signed-off-by: Andrzej Hunt <andrzej@ahunt.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
ahunt added a commit to ahunt/git that referenced this pull request Jun 11, 2021
cache_entry contains an object_id, and compare_ce_content() would
include that field when calling memcmp on a subset of the cache_entry.
Depending on which hashing algorithm is being used, only part of
object_id.hash is actually being used, therefore including it in a
memcmp() is incorrect. Instead we choose to exclude the object_id when
calling memcmp(), and call oideq() separately.

This issue was found when running t1700-split-index with MSAN, see MSAN
output below (on my machine, offset 76 corresponds to 4 bytes after the
start of object_id.hash).

Uninitialized bytes in MemcmpInterceptorCommon at offset 76 inside [0x7f60e7c00118, 92)
==27914==WARNING: MemorySanitizer: use-of-uninitialized-value
    #0 0x4524ee in memcmp /home/abuild/rpmbuild/BUILD/llvm-11.0.0.src/build/../projects/compiler-rt/lib/msan/../sanitizer_common/sanitizer_common_interceptors.inc:873:10
    #1 0xc867ae in compare_ce_content /home/ahunt/git/git/split-index.c:208:8
    #2 0xc859fb in prepare_to_write_split_index /home/ahunt/git/git/split-index.c:336:9
    #3 0xb4bbca in write_split_index /home/ahunt/git/git/read-cache.c:3107:2
    #4 0xb42b4d in write_locked_index /home/ahunt/git/git/read-cache.c:3295:8
    #5 0x638058 in try_merge_strategy /home/ahunt/git/git/builtin/merge.c:758:7
    #6 0x63057f in cmd_merge /home/ahunt/git/git/builtin/merge.c:1663:9
    #7 0x4a1e76 in run_builtin /home/ahunt/git/git/git.c:461:11
    git#8 0x49e1e7 in handle_builtin /home/ahunt/git/git/git.c:714:3
    git#9 0x4a0c08 in run_argv /home/ahunt/git/git/git.c:781:4
    git#10 0x49d5a8 in cmd_main /home/ahunt/git/git/git.c:912:19
    git#11 0x7974da in main /home/ahunt/git/git/common-main.c:52:11
    git#12 0x7f60e928e349 in __libc_start_main (/lib64/libc.so.6+0x24349)
    git#13 0x421bd9 in _start /home/abuild/rpmbuild/BUILD/glibc-2.26/csu/../sysdeps/x86_64/start.S:120

  Uninitialized value was stored to memory at
    #0 0x447eb9 in __msan_memcpy /home/abuild/rpmbuild/BUILD/llvm-11.0.0.src/build/../projects/compiler-rt/lib/msan/msan_interceptors.cpp:1558:3
    #1 0xb4d1e6 in dup_cache_entry /home/ahunt/git/git/read-cache.c:3457:2
    #2 0xd214fa in add_entry /home/ahunt/git/git/unpack-trees.c:215:18
    #3 0xd1fae0 in keep_entry /home/ahunt/git/git/unpack-trees.c:2276:2
    #4 0xd1ff9e in twoway_merge /home/ahunt/git/git/unpack-trees.c:2504:11
    #5 0xd27028 in call_unpack_fn /home/ahunt/git/git/unpack-trees.c:593:12
    #6 0xd2443d in unpack_nondirectories /home/ahunt/git/git/unpack-trees.c:1106:12
    #7 0xd19435 in unpack_callback /home/ahunt/git/git/unpack-trees.c:1306:6
    git#8 0xd0d7ff in traverse_trees /home/ahunt/git/git/tree-walk.c:532:17
    git#9 0xd1773a in unpack_trees /home/ahunt/git/git/unpack-trees.c:1683:9
    git#10 0xdc6370 in checkout /home/ahunt/git/git/merge-ort.c:3590:8
    git#11 0xdc51c3 in merge_switch_to_result /home/ahunt/git/git/merge-ort.c:3728:7
    git#12 0xa195a9 in merge_ort_recursive /home/ahunt/git/git/merge-ort-wrappers.c:58:2
    git#13 0x637fff in try_merge_strategy /home/ahunt/git/git/builtin/merge.c:751:12
    git#14 0x63057f in cmd_merge /home/ahunt/git/git/builtin/merge.c:1663:9
    git#15 0x4a1e76 in run_builtin /home/ahunt/git/git/git.c:461:11
    git#16 0x49e1e7 in handle_builtin /home/ahunt/git/git/git.c:714:3
    git#17 0x4a0c08 in run_argv /home/ahunt/git/git/git.c:781:4
    git#18 0x49d5a8 in cmd_main /home/ahunt/git/git/git.c:912:19
    git#19 0x7974da in main /home/ahunt/git/git/common-main.c:52:11

  Uninitialized value was created by a heap allocation
    #0 0x44e73d in malloc /home/abuild/rpmbuild/BUILD/llvm-11.0.0.src/build/../projects/compiler-rt/lib/msan/msan_interceptors.cpp:901:3
    #1 0xd592f6 in do_xmalloc /home/ahunt/git/git/wrapper.c:41:8
    #2 0xd59248 in xmalloc /home/ahunt/git/git/wrapper.c:62:9
    #3 0xa17088 in mem_pool_alloc_block /home/ahunt/git/git/mem-pool.c:22:6
    #4 0xa16f78 in mem_pool_init /home/ahunt/git/git/mem-pool.c:44:3
    #5 0xb481b8 in load_all_cache_entries /home/ahunt/git/git/read-cache.c
    #6 0xb44d40 in do_read_index /home/ahunt/git/git/read-cache.c:2298:17
    #7 0xb48a1b in read_index_from /home/ahunt/git/git/read-cache.c:2389:8
    git#8 0xbd5a0b in repo_read_index /home/ahunt/git/git/repository.c:276:8
    git#9 0xb4bcaf in repo_read_index_unmerged /home/ahunt/git/git/read-cache.c:3326:2
    git#10 0x62ed26 in cmd_merge /home/ahunt/git/git/builtin/merge.c:1362:6
    git#11 0x4a1e76 in run_builtin /home/ahunt/git/git/git.c:461:11
    git#12 0x49e1e7 in handle_builtin /home/ahunt/git/git/git.c:714:3
    git#13 0x4a0c08 in run_argv /home/ahunt/git/git/git.c:781:4
    git#14 0x49d5a8 in cmd_main /home/ahunt/git/git/git.c:912:19
    git#15 0x7974da in main /home/ahunt/git/git/common-main.c:52:11
    git#16 0x7f60e928e349 in __libc_start_main (/lib64/libc.so.6+0x24349)

SUMMARY: MemorySanitizer: use-of-uninitialized-value /home/abuild/rpmbuild/BUILD/llvm-11.0.0.src/build/../projects/compiler-rt/lib/msan/../sanitizer_common/sanitizer_common_interceptors.inc:873:10 in memcmp
Exiting

Signed-off-by: Andrzej Hunt <andrzej@ahunt.org>
gitster pushed a commit that referenced this pull request Jun 15, 2021
cache_entry contains an object_id, and compare_ce_content() would
include that field when calling memcmp on a subset of the cache_entry.
Depending on which hashing algorithm is being used, only part of
object_id.hash is actually being used, therefore including it in a
memcmp() is incorrect. Instead we choose to exclude the object_id when
calling memcmp(), and call oideq() separately.

This issue was found when running t1700-split-index with MSAN, see MSAN
output below (on my machine, offset 76 corresponds to 4 bytes after the
start of object_id.hash).

Uninitialized bytes in MemcmpInterceptorCommon at offset 76 inside [0x7f60e7c00118, 92)
==27914==WARNING: MemorySanitizer: use-of-uninitialized-value
    #0 0x4524ee in memcmp /home/abuild/rpmbuild/BUILD/llvm-11.0.0.src/build/../projects/compiler-rt/lib/msan/../sanitizer_common/sanitizer_common_interceptors.inc:873:10
    #1 0xc867ae in compare_ce_content /home/ahunt/git/git/split-index.c:208:8
    #2 0xc859fb in prepare_to_write_split_index /home/ahunt/git/git/split-index.c:336:9
    #3 0xb4bbca in write_split_index /home/ahunt/git/git/read-cache.c:3107:2
    #4 0xb42b4d in write_locked_index /home/ahunt/git/git/read-cache.c:3295:8
    #5 0x638058 in try_merge_strategy /home/ahunt/git/git/builtin/merge.c:758:7
    #6 0x63057f in cmd_merge /home/ahunt/git/git/builtin/merge.c:1663:9
    #7 0x4a1e76 in run_builtin /home/ahunt/git/git/git.c:461:11
    #8 0x49e1e7 in handle_builtin /home/ahunt/git/git/git.c:714:3
    #9 0x4a0c08 in run_argv /home/ahunt/git/git/git.c:781:4
    #10 0x49d5a8 in cmd_main /home/ahunt/git/git/git.c:912:19
    #11 0x7974da in main /home/ahunt/git/git/common-main.c:52:11
    #12 0x7f60e928e349 in __libc_start_main (/lib64/libc.so.6+0x24349)
    #13 0x421bd9 in _start /home/abuild/rpmbuild/BUILD/glibc-2.26/csu/../sysdeps/x86_64/start.S:120

  Uninitialized value was stored to memory at
    #0 0x447eb9 in __msan_memcpy /home/abuild/rpmbuild/BUILD/llvm-11.0.0.src/build/../projects/compiler-rt/lib/msan/msan_interceptors.cpp:1558:3
    #1 0xb4d1e6 in dup_cache_entry /home/ahunt/git/git/read-cache.c:3457:2
    #2 0xd214fa in add_entry /home/ahunt/git/git/unpack-trees.c:215:18
    #3 0xd1fae0 in keep_entry /home/ahunt/git/git/unpack-trees.c:2276:2
    #4 0xd1ff9e in twoway_merge /home/ahunt/git/git/unpack-trees.c:2504:11
    #5 0xd27028 in call_unpack_fn /home/ahunt/git/git/unpack-trees.c:593:12
    #6 0xd2443d in unpack_nondirectories /home/ahunt/git/git/unpack-trees.c:1106:12
    #7 0xd19435 in unpack_callback /home/ahunt/git/git/unpack-trees.c:1306:6
    #8 0xd0d7ff in traverse_trees /home/ahunt/git/git/tree-walk.c:532:17
    #9 0xd1773a in unpack_trees /home/ahunt/git/git/unpack-trees.c:1683:9
    #10 0xdc6370 in checkout /home/ahunt/git/git/merge-ort.c:3590:8
    #11 0xdc51c3 in merge_switch_to_result /home/ahunt/git/git/merge-ort.c:3728:7
    #12 0xa195a9 in merge_ort_recursive /home/ahunt/git/git/merge-ort-wrappers.c:58:2
    #13 0x637fff in try_merge_strategy /home/ahunt/git/git/builtin/merge.c:751:12
    #14 0x63057f in cmd_merge /home/ahunt/git/git/builtin/merge.c:1663:9
    #15 0x4a1e76 in run_builtin /home/ahunt/git/git/git.c:461:11
    #16 0x49e1e7 in handle_builtin /home/ahunt/git/git/git.c:714:3
    #17 0x4a0c08 in run_argv /home/ahunt/git/git/git.c:781:4
    #18 0x49d5a8 in cmd_main /home/ahunt/git/git/git.c:912:19
    #19 0x7974da in main /home/ahunt/git/git/common-main.c:52:11

  Uninitialized value was created by a heap allocation
    #0 0x44e73d in malloc /home/abuild/rpmbuild/BUILD/llvm-11.0.0.src/build/../projects/compiler-rt/lib/msan/msan_interceptors.cpp:901:3
    #1 0xd592f6 in do_xmalloc /home/ahunt/git/git/wrapper.c:41:8
    #2 0xd59248 in xmalloc /home/ahunt/git/git/wrapper.c:62:9
    #3 0xa17088 in mem_pool_alloc_block /home/ahunt/git/git/mem-pool.c:22:6
    #4 0xa16f78 in mem_pool_init /home/ahunt/git/git/mem-pool.c:44:3
    #5 0xb481b8 in load_all_cache_entries /home/ahunt/git/git/read-cache.c
    #6 0xb44d40 in do_read_index /home/ahunt/git/git/read-cache.c:2298:17
    #7 0xb48a1b in read_index_from /home/ahunt/git/git/read-cache.c:2389:8
    #8 0xbd5a0b in repo_read_index /home/ahunt/git/git/repository.c:276:8
    #9 0xb4bcaf in repo_read_index_unmerged /home/ahunt/git/git/read-cache.c:3326:2
    #10 0x62ed26 in cmd_merge /home/ahunt/git/git/builtin/merge.c:1362:6
    #11 0x4a1e76 in run_builtin /home/ahunt/git/git/git.c:461:11
    #12 0x49e1e7 in handle_builtin /home/ahunt/git/git/git.c:714:3
    #13 0x4a0c08 in run_argv /home/ahunt/git/git/git.c:781:4
    #14 0x49d5a8 in cmd_main /home/ahunt/git/git/git.c:912:19
    #15 0x7974da in main /home/ahunt/git/git/common-main.c:52:11
    #16 0x7f60e928e349 in __libc_start_main (/lib64/libc.so.6+0x24349)

SUMMARY: MemorySanitizer: use-of-uninitialized-value /home/abuild/rpmbuild/BUILD/llvm-11.0.0.src/build/../projects/compiler-rt/lib/msan/../sanitizer_common/sanitizer_common_interceptors.inc:873:10 in memcmp
Exiting

Signed-off-by: Andrzej Hunt <andrzej@ahunt.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
ahunt added a commit to ahunt/git that referenced this pull request Jun 15, 2021
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
    git#8 0x8b48cd in index_stream_convert_blob object-file.c:2156:2
    git#9 0x8b48cd in index_fd object-file.c:2248:9
    git#10 0x597411 in hash_fd builtin/hash-object.c:43:9
    git#11 0x596be1 in hash_object builtin/hash-object.c:59:2
    git#12 0x596be1 in cmd_hash_object builtin/hash-object.c:153:3
    git#13 0x4ce83e in run_builtin git.c:475:11
    git#14 0x4ccafe in handle_builtin git.c:729:3
    git#15 0x4cb01c in run_argv git.c:818:4
    git#16 0x4cb01c in cmd_main git.c:949:19
    git#17 0x6bdc2d in main common-main.c:52:11
    git#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
    git#8 0xa5f469 in unpack_trees unpack-trees.c:1747:8
    git#9 0x525971 in checkout builtin/clone.c:815:6
    git#10 0x525971 in cmd_clone builtin/clone.c:1409:8
    git#11 0x4ce83e in run_builtin git.c:475:11
    git#12 0x4ccafe in handle_builtin git.c:729:3
    git#13 0x4cb01c in run_argv git.c:818:4
    git#14 0x4cb01c in cmd_main git.c:949:19
    git#15 0x6bdc2d in main common-main.c:52:11
    git#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>
ahunt added a commit to ahunt/git that referenced this pull request Jun 16, 2021
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
    git#8 0x8b48cd in index_stream_convert_blob object-file.c:2156:2
    git#9 0x8b48cd in index_fd object-file.c:2248:9
    git#10 0x597411 in hash_fd builtin/hash-object.c:43:9
    git#11 0x596be1 in hash_object builtin/hash-object.c:59:2
    git#12 0x596be1 in cmd_hash_object builtin/hash-object.c:153:3
    git#13 0x4ce83e in run_builtin git.c:475:11
    git#14 0x4ccafe in handle_builtin git.c:729:3
    git#15 0x4cb01c in run_argv git.c:818:4
    git#16 0x4cb01c in cmd_main git.c:949:19
    git#17 0x6bdc2d in main common-main.c:52:11
    git#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
    git#8 0xa5f469 in unpack_trees unpack-trees.c:1747:8
    git#9 0x525971 in checkout builtin/clone.c:815:6
    git#10 0x525971 in cmd_clone builtin/clone.c:1409:8
    git#11 0x4ce83e in run_builtin git.c:475:11
    git#12 0x4ccafe in handle_builtin git.c:729:3
    git#13 0x4cb01c in run_argv git.c:818:4
    git#14 0x4cb01c in cmd_main git.c:949:19
    git#15 0x6bdc2d in main common-main.c:52:11
    git#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>
ahunt added a commit to ahunt/git that referenced this pull request Jun 20, 2021
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
    git#8 0x8b48cd in index_stream_convert_blob object-file.c:2156:2
    git#9 0x8b48cd in index_fd object-file.c:2248:9
    git#10 0x597411 in hash_fd builtin/hash-object.c:43:9
    git#11 0x596be1 in hash_object builtin/hash-object.c:59:2
    git#12 0x596be1 in cmd_hash_object builtin/hash-object.c:153:3
    git#13 0x4ce83e in run_builtin git.c:475:11
    git#14 0x4ccafe in handle_builtin git.c:729:3
    git#15 0x4cb01c in run_argv git.c:818:4
    git#16 0x4cb01c in cmd_main git.c:949:19
    git#17 0x6bdc2d in main common-main.c:52:11
    git#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
    git#8 0xa5f469 in unpack_trees unpack-trees.c:1747:8
    git#9 0x525971 in checkout builtin/clone.c:815:6
    git#10 0x525971 in cmd_clone builtin/clone.c:1409:8
    git#11 0x4ce83e in run_builtin git.c:475:11
    git#12 0x4ccafe in handle_builtin git.c:729:3
    git#13 0x4cb01c in run_argv git.c:818:4
    git#14 0x4cb01c in cmd_main git.c:949:19
    git#15 0x6bdc2d in main common-main.c:52:11
    git#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>
ahunt added a commit to ahunt/git that referenced this pull request Jun 20, 2021
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
    git#8 0x8b48cd in index_stream_convert_blob object-file.c:2156:2
    git#9 0x8b48cd in index_fd object-file.c:2248:9
    git#10 0x597411 in hash_fd builtin/hash-object.c:43:9
    git#11 0x596be1 in hash_object builtin/hash-object.c:59:2
    git#12 0x596be1 in cmd_hash_object builtin/hash-object.c:153:3
    git#13 0x4ce83e in run_builtin git.c:475:11
    git#14 0x4ccafe in handle_builtin git.c:729:3
    git#15 0x4cb01c in run_argv git.c:818:4
    git#16 0x4cb01c in cmd_main git.c:949:19
    git#17 0x6bdc2d in main common-main.c:52:11
    git#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
    git#8 0xa5f469 in unpack_trees unpack-trees.c:1747:8
    git#9 0x525971 in checkout builtin/clone.c:815:6
    git#10 0x525971 in cmd_clone builtin/clone.c:1409:8
    git#11 0x4ce83e in run_builtin git.c:475:11
    git#12 0x4ccafe in handle_builtin git.c:729:3
    git#13 0x4cb01c in run_argv git.c:818:4
    git#14 0x4cb01c in cmd_main git.c:949:19
    git#15 0x6bdc2d in main common-main.c:52:11
    git#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>
ahunt added a commit to ahunt/git that referenced this pull request Jul 25, 2021
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
    git#8 0x8b48cd in index_stream_convert_blob object-file.c:2156:2
    git#9 0x8b48cd in index_fd object-file.c:2248:9
    git#10 0x597411 in hash_fd builtin/hash-object.c:43:9
    git#11 0x596be1 in hash_object builtin/hash-object.c:59:2
    git#12 0x596be1 in cmd_hash_object builtin/hash-object.c:153:3
    git#13 0x4ce83e in run_builtin git.c:475:11
    git#14 0x4ccafe in handle_builtin git.c:729:3
    git#15 0x4cb01c in run_argv git.c:818:4
    git#16 0x4cb01c in cmd_main git.c:949:19
    git#17 0x6bdc2d in main common-main.c:52:11
    git#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
    git#8 0xa5f469 in unpack_trees unpack-trees.c:1747:8
    git#9 0x525971 in checkout builtin/clone.c:815:6
    git#10 0x525971 in cmd_clone builtin/clone.c:1409:8
    git#11 0x4ce83e in run_builtin git.c:475:11
    git#12 0x4ccafe in handle_builtin git.c:729:3
    git#13 0x4cb01c in run_argv git.c:818:4
    git#14 0x4cb01c in cmd_main git.c:949:19
    git#15 0x6bdc2d in main common-main.c:52:11
    git#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>
gitster pushed a commit that referenced this pull request Jul 26, 2021
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>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
avar added a commit to avar/git that referenced this pull request Sep 12, 2021
Start adding a release_git_diff_header() to free the memory in
read_patches(). This makes:

    git range-diff origin/master origin/next origin/seen

Not error out under SANITIZE=leak, before it was all leaks like:

    Direct leak of 4148 byte(s) in 228 object(s) allocated from:
    #0 0x7f79696f8db0 in __interceptor_malloc ../../../../src/libsanitizer/lsan/lsan_interceptors.cpp:54
    #1 0x5628bf8d532f in do_xmalloc /home/avar/g/git/wrapper.c:41
    #2 0x5628bf8d541c in do_xmallocz /home/avar/g/git/wrapper.c:75
    #3 0x5628bf8d5458 in xmallocz /home/avar/g/git/wrapper.c:83
    #4 0x5628bf8d5495 in xmemdupz /home/avar/g/git/wrapper.c:99
    #5 0x5628bf72a48f in find_name_common /home/avar/g/git/apply.c:681
    git#6 0x5628bf72a509 in find_name /home/avar/g/git/apply.c:696
    #7 0x5628bf72acf9 in gitdiff_verify_name /home/avar/g/git/apply.c:914
    git#8 0x5628bf72aeb2 in gitdiff_newname /home/avar/g/git/apply.c:952
    git#9 0x5628bf72bd09 in parse_git_diff_header /home/avar/g/git/apply.c:1357
    git#10 0x5628bf82f289 in read_patches /home/avar/g/git/range-diff.c:134
    git#11 0x5628bf830a67 in show_range_diff /home/avar/g/git/range-diff.c:558
    git#12 0x5628bf6d08a4 in cmd_range_diff builtin/range-diff.c:97
    git#13 0x5628bf6399f1 in run_builtin /home/avar/g/git/git.c:461
    git#14 0x5628bf639df6 in handle_builtin /home/avar/g/git/git.c:714
    git#15 0x5628bf63a060 in run_argv /home/avar/g/git/git.c:781
    git#16 0x5628bf63a4e1 in cmd_main /home/avar/g/git/git.c:912
    git#17 0x5628bf71d48a in main /home/avar/g/git/common-main.c:52
    git#18 0x7f7969488d09 in __libc_start_main ../csu/libc-start.c:308

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
avar added a commit to avar/git that referenced this pull request Sep 21, 2021
Start adding a release_git_diff_header() to free the memory in
read_patches(). This makes:

    git range-diff origin/master origin/next origin/seen

Not error out under SANITIZE=leak, before it was all leaks like:

    Direct leak of 4148 byte(s) in 228 object(s) allocated from:
    #0 0x7f79696f8db0 in __interceptor_malloc ../../../../src/libsanitizer/lsan/lsan_interceptors.cpp:54
    #1 0x5628bf8d532f in do_xmalloc /home/avar/g/git/wrapper.c:41
    #2 0x5628bf8d541c in do_xmallocz /home/avar/g/git/wrapper.c:75
    #3 0x5628bf8d5458 in xmallocz /home/avar/g/git/wrapper.c:83
    #4 0x5628bf8d5495 in xmemdupz /home/avar/g/git/wrapper.c:99
    #5 0x5628bf72a48f in find_name_common /home/avar/g/git/apply.c:681
    git#6 0x5628bf72a509 in find_name /home/avar/g/git/apply.c:696
    #7 0x5628bf72acf9 in gitdiff_verify_name /home/avar/g/git/apply.c:914
    git#8 0x5628bf72aeb2 in gitdiff_newname /home/avar/g/git/apply.c:952
    git#9 0x5628bf72bd09 in parse_git_diff_header /home/avar/g/git/apply.c:1357
    git#10 0x5628bf82f289 in read_patches /home/avar/g/git/range-diff.c:134
    git#11 0x5628bf830a67 in show_range_diff /home/avar/g/git/range-diff.c:558
    git#12 0x5628bf6d08a4 in cmd_range_diff builtin/range-diff.c:97
    git#13 0x5628bf6399f1 in run_builtin /home/avar/g/git/git.c:461
    git#14 0x5628bf639df6 in handle_builtin /home/avar/g/git/git.c:714
    git#15 0x5628bf63a060 in run_argv /home/avar/g/git/git.c:781
    git#16 0x5628bf63a4e1 in cmd_main /home/avar/g/git/git.c:912
    git#17 0x5628bf71d48a in main /home/avar/g/git/common-main.c:52
    git#18 0x7f7969488d09 in __libc_start_main ../csu/libc-start.c:308

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
avar added a commit to avar/git that referenced this pull request Oct 4, 2021
Start adding a release_git_diff_header() to free the memory in
read_patches(). This makes:

    git range-diff origin/master origin/next origin/seen

Not error out under SANITIZE=leak, before it was all leaks like:

    Direct leak of 4148 byte(s) in 228 object(s) allocated from:
    #0 0x7f79696f8db0 in __interceptor_malloc ../../../../src/libsanitizer/lsan/lsan_interceptors.cpp:54
    #1 0x5628bf8d532f in do_xmalloc /home/avar/g/git/wrapper.c:41
    #2 0x5628bf8d541c in do_xmallocz /home/avar/g/git/wrapper.c:75
    #3 0x5628bf8d5458 in xmallocz /home/avar/g/git/wrapper.c:83
    #4 0x5628bf8d5495 in xmemdupz /home/avar/g/git/wrapper.c:99
    #5 0x5628bf72a48f in find_name_common /home/avar/g/git/apply.c:681
    git#6 0x5628bf72a509 in find_name /home/avar/g/git/apply.c:696
    #7 0x5628bf72acf9 in gitdiff_verify_name /home/avar/g/git/apply.c:914
    git#8 0x5628bf72aeb2 in gitdiff_newname /home/avar/g/git/apply.c:952
    git#9 0x5628bf72bd09 in parse_git_diff_header /home/avar/g/git/apply.c:1357
    git#10 0x5628bf82f289 in read_patches /home/avar/g/git/range-diff.c:134
    git#11 0x5628bf830a67 in show_range_diff /home/avar/g/git/range-diff.c:558
    git#12 0x5628bf6d08a4 in cmd_range_diff builtin/range-diff.c:97
    git#13 0x5628bf6399f1 in run_builtin /home/avar/g/git/git.c:461
    git#14 0x5628bf639df6 in handle_builtin /home/avar/g/git/git.c:714
    git#15 0x5628bf63a060 in run_argv /home/avar/g/git/git.c:781
    git#16 0x5628bf63a4e1 in cmd_main /home/avar/g/git/git.c:912
    git#17 0x5628bf71d48a in main /home/avar/g/git/common-main.c:52
    git#18 0x7f7969488d09 in __libc_start_main ../csu/libc-start.c:308

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
phillipwood added a commit to phillipwood/git that referenced this pull request Oct 6, 2021
In a sparse index it is possible for the tree that is being verified
to be freed while it is being verified. This happens when
index_name_pos() looks up a entry that is missing from the index and
that would be a descendant of a sparse entry. That triggers a call to
ensure_full_index() which frees the cache tree that is being verified.
Carrying on trying to verify the tree after this results in a
use-after-free bug. Instead restart the verification if a sparse index
is converted to a full index. This bug is triggered by a call to
reset_head() in "git rebase --apply". Thanks to René Scharfe for his
help analyzing the problem.

==74345==ERROR: AddressSanitizer: heap-use-after-free on address 0x606000001b20 at pc 0x557cbe82d3a2 bp 0x7ffdfee08090 sp 0x7ffdfee08080
READ of size 4 at 0x606000001b20 thread T0
    #0 0x557cbe82d3a1 in verify_one /home/phil/src/git/cache-tree.c:863
    #1 0x557cbe82ca9d in verify_one /home/phil/src/git/cache-tree.c:840
    #2 0x557cbe82ca9d in verify_one /home/phil/src/git/cache-tree.c:840
    #3 0x557cbe82ca9d in verify_one /home/phil/src/git/cache-tree.c:840
    #4 0x557cbe830a2b in cache_tree_verify /home/phil/src/git/cache-tree.c:910
    #5 0x557cbea53741 in write_locked_index /home/phil/src/git/read-cache.c:3250
    git#6 0x557cbeab7fdd in reset_head /home/phil/src/git/reset.c:87
    #7 0x557cbe72147f in cmd_rebase builtin/rebase.c:2074
    git#8 0x557cbe5bd151 in run_builtin /home/phil/src/git/git.c:461
    git#9 0x557cbe5bd151 in handle_builtin /home/phil/src/git/git.c:714
    git#10 0x557cbe5c0503 in run_argv /home/phil/src/git/git.c:781
    git#11 0x557cbe5c0503 in cmd_main /home/phil/src/git/git.c:912
    git#12 0x557cbe5bad28 in main /home/phil/src/git/common-main.c:52
    git#13 0x7fdd4b82eb24 in __libc_start_main (/usr/lib/libc.so.6+0x27b24)
    git#14 0x557cbe5bcb8d in _start (/home/phil/src/git/git+0x1b9b8d)

0x606000001b20 is located 0 bytes inside of 56-byte region [0x606000001b20,0x606000001b58)
freed by thread T0 here:
    #0 0x7fdd4bacff19 in __interceptor_free /build/gcc/src/gcc/libsanitizer/asan/asan_malloc_linux.cpp:127
    #1 0x557cbe82af60 in cache_tree_free /home/phil/src/git/cache-tree.c:35
    #2 0x557cbe82aee5 in cache_tree_free /home/phil/src/git/cache-tree.c:31
    #3 0x557cbe82aee5 in cache_tree_free /home/phil/src/git/cache-tree.c:31
    #4 0x557cbe82aee5 in cache_tree_free /home/phil/src/git/cache-tree.c:31
    #5 0x557cbeb2557a in ensure_full_index /home/phil/src/git/sparse-index.c:310
    git#6 0x557cbea45c4a in index_name_stage_pos /home/phil/src/git/read-cache.c:588
    #7 0x557cbe82ce37 in verify_one /home/phil/src/git/cache-tree.c:850
    git#8 0x557cbe82ca9d in verify_one /home/phil/src/git/cache-tree.c:840
    git#9 0x557cbe82ca9d in verify_one /home/phil/src/git/cache-tree.c:840
    git#10 0x557cbe82ca9d in verify_one /home/phil/src/git/cache-tree.c:840
    git#11 0x557cbe830a2b in cache_tree_verify /home/phil/src/git/cache-tree.c:910
    git#12 0x557cbea53741 in write_locked_index /home/phil/src/git/read-cache.c:3250
    git#13 0x557cbeab7fdd in reset_head /home/phil/src/git/reset.c:87
    git#14 0x557cbe72147f in cmd_rebase builtin/rebase.c:2074
    git#15 0x557cbe5bd151 in run_builtin /home/phil/src/git/git.c:461
    git#16 0x557cbe5bd151 in handle_builtin /home/phil/src/git/git.c:714
    git#17 0x557cbe5c0503 in run_argv /home/phil/src/git/git.c:781
    git#18 0x557cbe5c0503 in cmd_main /home/phil/src/git/git.c:912
    git#19 0x557cbe5bad28 in main /home/phil/src/git/common-main.c:52
    git#20 0x7fdd4b82eb24 in __libc_start_main (/usr/lib/libc.so.6+0x27b24)

previously allocated by thread T0 here:
    #0 0x7fdd4bad0459 in __interceptor_calloc /build/gcc/src/gcc/libsanitizer/asan/asan_malloc_linux.cpp:154
    #1 0x557cbebc1807 in xcalloc /home/phil/src/git/wrapper.c:140
    #2 0x557cbe82b7d8 in cache_tree /home/phil/src/git/cache-tree.c:17
    #3 0x557cbe82b7d8 in prime_cache_tree_rec /home/phil/src/git/cache-tree.c:763
    #4 0x557cbe82b837 in prime_cache_tree_rec /home/phil/src/git/cache-tree.c:764
    #5 0x557cbe82b837 in prime_cache_tree_rec /home/phil/src/git/cache-tree.c:764
    git#6 0x557cbe8304e1 in prime_cache_tree /home/phil/src/git/cache-tree.c:779
    #7 0x557cbeab7fa7 in reset_head /home/phil/src/git/reset.c:85
    git#8 0x557cbe72147f in cmd_rebase builtin/rebase.c:2074
    git#9 0x557cbe5bd151 in run_builtin /home/phil/src/git/git.c:461
    git#10 0x557cbe5bd151 in handle_builtin /home/phil/src/git/git.c:714
    git#11 0x557cbe5c0503 in run_argv /home/phil/src/git/git.c:781
    git#12 0x557cbe5c0503 in cmd_main /home/phil/src/git/git.c:912
    git#13 0x557cbe5bad28 in main /home/phil/src/git/common-main.c:52
    git#14 0x7fdd4b82eb24 in __libc_start_main (/usr/lib/libc.so.6+0x27b24)

Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
gitster pushed a commit that referenced this pull request Oct 6, 2021
In a sparse index it is possible for the tree that is being verified
to be freed while it is being verified. This happens when
index_name_pos() looks up a entry that is missing from the index and
that would be a descendant of a sparse entry. That triggers a call to
ensure_full_index() which frees the cache tree that is being verified.
Carrying on trying to verify the tree after this results in a
use-after-free bug. Instead restart the verification if a sparse index
is converted to a full index. This bug is triggered by a call to
reset_head() in "git rebase --apply". Thanks to René Scharfe for his
help analyzing the problem.

==74345==ERROR: AddressSanitizer: heap-use-after-free on address 0x606000001b20 at pc 0x557cbe82d3a2 bp 0x7ffdfee08090 sp 0x7ffdfee08080
READ of size 4 at 0x606000001b20 thread T0
    #0 0x557cbe82d3a1 in verify_one /home/phil/src/git/cache-tree.c:863
    #1 0x557cbe82ca9d in verify_one /home/phil/src/git/cache-tree.c:840
    #2 0x557cbe82ca9d in verify_one /home/phil/src/git/cache-tree.c:840
    #3 0x557cbe82ca9d in verify_one /home/phil/src/git/cache-tree.c:840
    #4 0x557cbe830a2b in cache_tree_verify /home/phil/src/git/cache-tree.c:910
    #5 0x557cbea53741 in write_locked_index /home/phil/src/git/read-cache.c:3250
    #6 0x557cbeab7fdd in reset_head /home/phil/src/git/reset.c:87
    #7 0x557cbe72147f in cmd_rebase builtin/rebase.c:2074
    #8 0x557cbe5bd151 in run_builtin /home/phil/src/git/git.c:461
    #9 0x557cbe5bd151 in handle_builtin /home/phil/src/git/git.c:714
    #10 0x557cbe5c0503 in run_argv /home/phil/src/git/git.c:781
    #11 0x557cbe5c0503 in cmd_main /home/phil/src/git/git.c:912
    #12 0x557cbe5bad28 in main /home/phil/src/git/common-main.c:52
    #13 0x7fdd4b82eb24 in __libc_start_main (/usr/lib/libc.so.6+0x27b24)
    #14 0x557cbe5bcb8d in _start (/home/phil/src/git/git+0x1b9b8d)

0x606000001b20 is located 0 bytes inside of 56-byte region [0x606000001b20,0x606000001b58)
freed by thread T0 here:
    #0 0x7fdd4bacff19 in __interceptor_free /build/gcc/src/gcc/libsanitizer/asan/asan_malloc_linux.cpp:127
    #1 0x557cbe82af60 in cache_tree_free /home/phil/src/git/cache-tree.c:35
    #2 0x557cbe82aee5 in cache_tree_free /home/phil/src/git/cache-tree.c:31
    #3 0x557cbe82aee5 in cache_tree_free /home/phil/src/git/cache-tree.c:31
    #4 0x557cbe82aee5 in cache_tree_free /home/phil/src/git/cache-tree.c:31
    #5 0x557cbeb2557a in ensure_full_index /home/phil/src/git/sparse-index.c:310
    #6 0x557cbea45c4a in index_name_stage_pos /home/phil/src/git/read-cache.c:588
    #7 0x557cbe82ce37 in verify_one /home/phil/src/git/cache-tree.c:850
    #8 0x557cbe82ca9d in verify_one /home/phil/src/git/cache-tree.c:840
    #9 0x557cbe82ca9d in verify_one /home/phil/src/git/cache-tree.c:840
    #10 0x557cbe82ca9d in verify_one /home/phil/src/git/cache-tree.c:840
    #11 0x557cbe830a2b in cache_tree_verify /home/phil/src/git/cache-tree.c:910
    #12 0x557cbea53741 in write_locked_index /home/phil/src/git/read-cache.c:3250
    #13 0x557cbeab7fdd in reset_head /home/phil/src/git/reset.c:87
    #14 0x557cbe72147f in cmd_rebase builtin/rebase.c:2074
    #15 0x557cbe5bd151 in run_builtin /home/phil/src/git/git.c:461
    #16 0x557cbe5bd151 in handle_builtin /home/phil/src/git/git.c:714
    #17 0x557cbe5c0503 in run_argv /home/phil/src/git/git.c:781
    #18 0x557cbe5c0503 in cmd_main /home/phil/src/git/git.c:912
    #19 0x557cbe5bad28 in main /home/phil/src/git/common-main.c:52
    #20 0x7fdd4b82eb24 in __libc_start_main (/usr/lib/libc.so.6+0x27b24)

previously allocated by thread T0 here:
    #0 0x7fdd4bad0459 in __interceptor_calloc /build/gcc/src/gcc/libsanitizer/asan/asan_malloc_linux.cpp:154
    #1 0x557cbebc1807 in xcalloc /home/phil/src/git/wrapper.c:140
    #2 0x557cbe82b7d8 in cache_tree /home/phil/src/git/cache-tree.c:17
    #3 0x557cbe82b7d8 in prime_cache_tree_rec /home/phil/src/git/cache-tree.c:763
    #4 0x557cbe82b837 in prime_cache_tree_rec /home/phil/src/git/cache-tree.c:764
    #5 0x557cbe82b837 in prime_cache_tree_rec /home/phil/src/git/cache-tree.c:764
    #6 0x557cbe8304e1 in prime_cache_tree /home/phil/src/git/cache-tree.c:779
    #7 0x557cbeab7fa7 in reset_head /home/phil/src/git/reset.c:85
    #8 0x557cbe72147f in cmd_rebase builtin/rebase.c:2074
    #9 0x557cbe5bd151 in run_builtin /home/phil/src/git/git.c:461
    #10 0x557cbe5bd151 in handle_builtin /home/phil/src/git/git.c:714
    #11 0x557cbe5c0503 in run_argv /home/phil/src/git/git.c:781
    #12 0x557cbe5c0503 in cmd_main /home/phil/src/git/git.c:912
    #13 0x557cbe5bad28 in main /home/phil/src/git/common-main.c:52
    #14 0x7fdd4b82eb24 in __libc_start_main (/usr/lib/libc.so.6+0x27b24)

Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
phillipwood added a commit to phillipwood/git that referenced this pull request Oct 7, 2021
In a sparse index it is possible for the tree that is being verified
to be freed while it is being verified. This happens when the index is
sparse but the cache tree is not and index_name_pos() looks up a path
from the cache tree that is a descendant of a sparse index entry. That
triggers a call to ensure_full_index() which frees the cache tree that
is being verified.  Carrying on trying to verify the tree after this
results in a use-after-free bug. Instead restart the verification if a
sparse index is converted to a full index. This bug is triggered by a
call to reset_head() in "git rebase --apply". Thanks to René Scharfe
and Derick Stolee for their help analyzing the problem.

==74345==ERROR: AddressSanitizer: heap-use-after-free on address 0x606000001b20 at pc 0x557cbe82d3a2 bp 0x7ffdfee08090 sp 0x7ffdfee08080
READ of size 4 at 0x606000001b20 thread T0
    #0 0x557cbe82d3a1 in verify_one /home/phil/src/git/cache-tree.c:863
    #1 0x557cbe82ca9d in verify_one /home/phil/src/git/cache-tree.c:840
    #2 0x557cbe82ca9d in verify_one /home/phil/src/git/cache-tree.c:840
    #3 0x557cbe82ca9d in verify_one /home/phil/src/git/cache-tree.c:840
    #4 0x557cbe830a2b in cache_tree_verify /home/phil/src/git/cache-tree.c:910
    #5 0x557cbea53741 in write_locked_index /home/phil/src/git/read-cache.c:3250
    git#6 0x557cbeab7fdd in reset_head /home/phil/src/git/reset.c:87
    #7 0x557cbe72147f in cmd_rebase builtin/rebase.c:2074
    git#8 0x557cbe5bd151 in run_builtin /home/phil/src/git/git.c:461
    git#9 0x557cbe5bd151 in handle_builtin /home/phil/src/git/git.c:714
    git#10 0x557cbe5c0503 in run_argv /home/phil/src/git/git.c:781
    git#11 0x557cbe5c0503 in cmd_main /home/phil/src/git/git.c:912
    git#12 0x557cbe5bad28 in main /home/phil/src/git/common-main.c:52
    git#13 0x7fdd4b82eb24 in __libc_start_main (/usr/lib/libc.so.6+0x27b24)
    git#14 0x557cbe5bcb8d in _start (/home/phil/src/git/git+0x1b9b8d)

0x606000001b20 is located 0 bytes inside of 56-byte region [0x606000001b20,0x606000001b58)
freed by thread T0 here:
    #0 0x7fdd4bacff19 in __interceptor_free /build/gcc/src/gcc/libsanitizer/asan/asan_malloc_linux.cpp:127
    #1 0x557cbe82af60 in cache_tree_free /home/phil/src/git/cache-tree.c:35
    #2 0x557cbe82aee5 in cache_tree_free /home/phil/src/git/cache-tree.c:31
    #3 0x557cbe82aee5 in cache_tree_free /home/phil/src/git/cache-tree.c:31
    #4 0x557cbe82aee5 in cache_tree_free /home/phil/src/git/cache-tree.c:31
    #5 0x557cbeb2557a in ensure_full_index /home/phil/src/git/sparse-index.c:310
    git#6 0x557cbea45c4a in index_name_stage_pos /home/phil/src/git/read-cache.c:588
    #7 0x557cbe82ce37 in verify_one /home/phil/src/git/cache-tree.c:850
    git#8 0x557cbe82ca9d in verify_one /home/phil/src/git/cache-tree.c:840
    git#9 0x557cbe82ca9d in verify_one /home/phil/src/git/cache-tree.c:840
    git#10 0x557cbe82ca9d in verify_one /home/phil/src/git/cache-tree.c:840
    git#11 0x557cbe830a2b in cache_tree_verify /home/phil/src/git/cache-tree.c:910
    git#12 0x557cbea53741 in write_locked_index /home/phil/src/git/read-cache.c:3250
    git#13 0x557cbeab7fdd in reset_head /home/phil/src/git/reset.c:87
    git#14 0x557cbe72147f in cmd_rebase builtin/rebase.c:2074
    git#15 0x557cbe5bd151 in run_builtin /home/phil/src/git/git.c:461
    git#16 0x557cbe5bd151 in handle_builtin /home/phil/src/git/git.c:714
    git#17 0x557cbe5c0503 in run_argv /home/phil/src/git/git.c:781
    git#18 0x557cbe5c0503 in cmd_main /home/phil/src/git/git.c:912
    git#19 0x557cbe5bad28 in main /home/phil/src/git/common-main.c:52
    git#20 0x7fdd4b82eb24 in __libc_start_main (/usr/lib/libc.so.6+0x27b24)

previously allocated by thread T0 here:
    #0 0x7fdd4bad0459 in __interceptor_calloc /build/gcc/src/gcc/libsanitizer/asan/asan_malloc_linux.cpp:154
    #1 0x557cbebc1807 in xcalloc /home/phil/src/git/wrapper.c:140
    #2 0x557cbe82b7d8 in cache_tree /home/phil/src/git/cache-tree.c:17
    #3 0x557cbe82b7d8 in prime_cache_tree_rec /home/phil/src/git/cache-tree.c:763
    #4 0x557cbe82b837 in prime_cache_tree_rec /home/phil/src/git/cache-tree.c:764
    #5 0x557cbe82b837 in prime_cache_tree_rec /home/phil/src/git/cache-tree.c:764
    git#6 0x557cbe8304e1 in prime_cache_tree /home/phil/src/git/cache-tree.c:779
    #7 0x557cbeab7fa7 in reset_head /home/phil/src/git/reset.c:85
    git#8 0x557cbe72147f in cmd_rebase builtin/rebase.c:2074
    git#9 0x557cbe5bd151 in run_builtin /home/phil/src/git/git.c:461
    git#10 0x557cbe5bd151 in handle_builtin /home/phil/src/git/git.c:714
    git#11 0x557cbe5c0503 in run_argv /home/phil/src/git/git.c:781
    git#12 0x557cbe5c0503 in cmd_main /home/phil/src/git/git.c:912
    git#13 0x557cbe5bad28 in main /home/phil/src/git/common-main.c:52
    git#14 0x7fdd4b82eb24 in __libc_start_main (/usr/lib/libc.so.6+0x27b24)

Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
derrickstolee referenced this pull request in derrickstolee/git Oct 7, 2021
In a sparse index it is possible for the tree that is being verified
to be freed while it is being verified. This happens when the index is
sparse but the cache tree is not and index_name_pos() looks up a path
from the cache tree that is a descendant of a sparse index entry. That
triggers a call to ensure_full_index() which frees the cache tree that
is being verified.  Carrying on trying to verify the tree after this
results in a use-after-free bug. Instead restart the verification if a
sparse index is converted to a full index. This bug is triggered by a
call to reset_head() in "git rebase --apply". Thanks to René Scharfe
and Derick Stolee for their help analyzing the problem.

==74345==ERROR: AddressSanitizer: heap-use-after-free on address 0x606000001b20 at pc 0x557cbe82d3a2 bp 0x7ffdfee08090 sp 0x7ffdfee08080
READ of size 4 at 0x606000001b20 thread T0
    #0 0x557cbe82d3a1 in verify_one /home/phil/src/git/cache-tree.c:863
    #1 0x557cbe82ca9d in verify_one /home/phil/src/git/cache-tree.c:840
    #2 0x557cbe82ca9d in verify_one /home/phil/src/git/cache-tree.c:840
    #3 0x557cbe82ca9d in verify_one /home/phil/src/git/cache-tree.c:840
    #4 0x557cbe830a2b in cache_tree_verify /home/phil/src/git/cache-tree.c:910
    #5 0x557cbea53741 in write_locked_index /home/phil/src/git/read-cache.c:3250
    #6 0x557cbeab7fdd in reset_head /home/phil/src/git/reset.c:87
    #7 0x557cbe72147f in cmd_rebase builtin/rebase.c:2074
    #8 0x557cbe5bd151 in run_builtin /home/phil/src/git/git.c:461
    #9 0x557cbe5bd151 in handle_builtin /home/phil/src/git/git.c:714
    #10 0x557cbe5c0503 in run_argv /home/phil/src/git/git.c:781
    #11 0x557cbe5c0503 in cmd_main /home/phil/src/git/git.c:912
    #12 0x557cbe5bad28 in main /home/phil/src/git/common-main.c:52
    #13 0x7fdd4b82eb24 in __libc_start_main (/usr/lib/libc.so.6+0x27b24)
    #14 0x557cbe5bcb8d in _start (/home/phil/src/git/git+0x1b9b8d)

0x606000001b20 is located 0 bytes inside of 56-byte region [0x606000001b20,0x606000001b58)
freed by thread T0 here:
    #0 0x7fdd4bacff19 in __interceptor_free /build/gcc/src/gcc/libsanitizer/asan/asan_malloc_linux.cpp:127
    #1 0x557cbe82af60 in cache_tree_free /home/phil/src/git/cache-tree.c:35
    #2 0x557cbe82aee5 in cache_tree_free /home/phil/src/git/cache-tree.c:31
    #3 0x557cbe82aee5 in cache_tree_free /home/phil/src/git/cache-tree.c:31
    #4 0x557cbe82aee5 in cache_tree_free /home/phil/src/git/cache-tree.c:31
    #5 0x557cbeb2557a in ensure_full_index /home/phil/src/git/sparse-index.c:310
    #6 0x557cbea45c4a in index_name_stage_pos /home/phil/src/git/read-cache.c:588
    #7 0x557cbe82ce37 in verify_one /home/phil/src/git/cache-tree.c:850
    #8 0x557cbe82ca9d in verify_one /home/phil/src/git/cache-tree.c:840
    #9 0x557cbe82ca9d in verify_one /home/phil/src/git/cache-tree.c:840
    #10 0x557cbe82ca9d in verify_one /home/phil/src/git/cache-tree.c:840
    #11 0x557cbe830a2b in cache_tree_verify /home/phil/src/git/cache-tree.c:910
    #12 0x557cbea53741 in write_locked_index /home/phil/src/git/read-cache.c:3250
    #13 0x557cbeab7fdd in reset_head /home/phil/src/git/reset.c:87
    #14 0x557cbe72147f in cmd_rebase builtin/rebase.c:2074
    #15 0x557cbe5bd151 in run_builtin /home/phil/src/git/git.c:461
    #16 0x557cbe5bd151 in handle_builtin /home/phil/src/git/git.c:714
    #17 0x557cbe5c0503 in run_argv /home/phil/src/git/git.c:781
    #18 0x557cbe5c0503 in cmd_main /home/phil/src/git/git.c:912
    #19 0x557cbe5bad28 in main /home/phil/src/git/common-main.c:52
    #20 0x7fdd4b82eb24 in __libc_start_main (/usr/lib/libc.so.6+0x27b24)

previously allocated by thread T0 here:
    #0 0x7fdd4bad0459 in __interceptor_calloc /build/gcc/src/gcc/libsanitizer/asan/asan_malloc_linux.cpp:154
    #1 0x557cbebc1807 in xcalloc /home/phil/src/git/wrapper.c:140
    #2 0x557cbe82b7d8 in cache_tree /home/phil/src/git/cache-tree.c:17
    #3 0x557cbe82b7d8 in prime_cache_tree_rec /home/phil/src/git/cache-tree.c:763
    #4 0x557cbe82b837 in prime_cache_tree_rec /home/phil/src/git/cache-tree.c:764
    #5 0x557cbe82b837 in prime_cache_tree_rec /home/phil/src/git/cache-tree.c:764
    #6 0x557cbe8304e1 in prime_cache_tree /home/phil/src/git/cache-tree.c:779
    #7 0x557cbeab7fa7 in reset_head /home/phil/src/git/reset.c:85
    #8 0x557cbe72147f in cmd_rebase builtin/rebase.c:2074
    #9 0x557cbe5bd151 in run_builtin /home/phil/src/git/git.c:461
    #10 0x557cbe5bd151 in handle_builtin /home/phil/src/git/git.c:714
    #11 0x557cbe5c0503 in run_argv /home/phil/src/git/git.c:781
    #12 0x557cbe5c0503 in cmd_main /home/phil/src/git/git.c:912
    #13 0x557cbe5bad28 in main /home/phil/src/git/common-main.c:52
    #14 0x7fdd4b82eb24 in __libc_start_main (/usr/lib/libc.so.6+0x27b24)

Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
derrickstolee referenced this pull request in derrickstolee/git Oct 7, 2021
In a sparse index it is possible for the tree that is being verified
to be freed while it is being verified. This happens when the index is
sparse but the cache tree is not and index_name_pos() looks up a path
from the cache tree that is a descendant of a sparse index entry. That
triggers a call to ensure_full_index() which frees the cache tree that
is being verified.  Carrying on trying to verify the tree after this
results in a use-after-free bug. Instead restart the verification if a
sparse index is converted to a full index. This bug is triggered by a
call to reset_head() in "git rebase --apply". Thanks to René Scharfe
and Derrick Stolee for their help analyzing the problem.

==74345==ERROR: AddressSanitizer: heap-use-after-free on address 0x606000001b20 at pc 0x557cbe82d3a2 bp 0x7ffdfee08090 sp 0x7ffdfee08080
READ of size 4 at 0x606000001b20 thread T0
    #0 0x557cbe82d3a1 in verify_one /home/phil/src/git/cache-tree.c:863
    #1 0x557cbe82ca9d in verify_one /home/phil/src/git/cache-tree.c:840
    #2 0x557cbe82ca9d in verify_one /home/phil/src/git/cache-tree.c:840
    #3 0x557cbe82ca9d in verify_one /home/phil/src/git/cache-tree.c:840
    #4 0x557cbe830a2b in cache_tree_verify /home/phil/src/git/cache-tree.c:910
    #5 0x557cbea53741 in write_locked_index /home/phil/src/git/read-cache.c:3250
    #6 0x557cbeab7fdd in reset_head /home/phil/src/git/reset.c:87
    #7 0x557cbe72147f in cmd_rebase builtin/rebase.c:2074
    #8 0x557cbe5bd151 in run_builtin /home/phil/src/git/git.c:461
    #9 0x557cbe5bd151 in handle_builtin /home/phil/src/git/git.c:714
    #10 0x557cbe5c0503 in run_argv /home/phil/src/git/git.c:781
    #11 0x557cbe5c0503 in cmd_main /home/phil/src/git/git.c:912
    #12 0x557cbe5bad28 in main /home/phil/src/git/common-main.c:52
    #13 0x7fdd4b82eb24 in __libc_start_main (/usr/lib/libc.so.6+0x27b24)
    #14 0x557cbe5bcb8d in _start (/home/phil/src/git/git+0x1b9b8d)

0x606000001b20 is located 0 bytes inside of 56-byte region [0x606000001b20,0x606000001b58)
freed by thread T0 here:
    #0 0x7fdd4bacff19 in __interceptor_free /build/gcc/src/gcc/libsanitizer/asan/asan_malloc_linux.cpp:127
    #1 0x557cbe82af60 in cache_tree_free /home/phil/src/git/cache-tree.c:35
    #2 0x557cbe82aee5 in cache_tree_free /home/phil/src/git/cache-tree.c:31
    #3 0x557cbe82aee5 in cache_tree_free /home/phil/src/git/cache-tree.c:31
    #4 0x557cbe82aee5 in cache_tree_free /home/phil/src/git/cache-tree.c:31
    #5 0x557cbeb2557a in ensure_full_index /home/phil/src/git/sparse-index.c:310
    #6 0x557cbea45c4a in index_name_stage_pos /home/phil/src/git/read-cache.c:588
    #7 0x557cbe82ce37 in verify_one /home/phil/src/git/cache-tree.c:850
    #8 0x557cbe82ca9d in verify_one /home/phil/src/git/cache-tree.c:840
    #9 0x557cbe82ca9d in verify_one /home/phil/src/git/cache-tree.c:840
    #10 0x557cbe82ca9d in verify_one /home/phil/src/git/cache-tree.c:840
    #11 0x557cbe830a2b in cache_tree_verify /home/phil/src/git/cache-tree.c:910
    #12 0x557cbea53741 in write_locked_index /home/phil/src/git/read-cache.c:3250
    #13 0x557cbeab7fdd in reset_head /home/phil/src/git/reset.c:87
    #14 0x557cbe72147f in cmd_rebase builtin/rebase.c:2074
    #15 0x557cbe5bd151 in run_builtin /home/phil/src/git/git.c:461
    #16 0x557cbe5bd151 in handle_builtin /home/phil/src/git/git.c:714
    #17 0x557cbe5c0503 in run_argv /home/phil/src/git/git.c:781
    #18 0x557cbe5c0503 in cmd_main /home/phil/src/git/git.c:912
    #19 0x557cbe5bad28 in main /home/phil/src/git/common-main.c:52
    #20 0x7fdd4b82eb24 in __libc_start_main (/usr/lib/libc.so.6+0x27b24)

previously allocated by thread T0 here:
    #0 0x7fdd4bad0459 in __interceptor_calloc /build/gcc/src/gcc/libsanitizer/asan/asan_malloc_linux.cpp:154
    #1 0x557cbebc1807 in xcalloc /home/phil/src/git/wrapper.c:140
    #2 0x557cbe82b7d8 in cache_tree /home/phil/src/git/cache-tree.c:17
    #3 0x557cbe82b7d8 in prime_cache_tree_rec /home/phil/src/git/cache-tree.c:763
    #4 0x557cbe82b837 in prime_cache_tree_rec /home/phil/src/git/cache-tree.c:764
    #5 0x557cbe82b837 in prime_cache_tree_rec /home/phil/src/git/cache-tree.c:764
    #6 0x557cbe8304e1 in prime_cache_tree /home/phil/src/git/cache-tree.c:779
    #7 0x557cbeab7fa7 in reset_head /home/phil/src/git/reset.c:85
    #8 0x557cbe72147f in cmd_rebase builtin/rebase.c:2074
    #9 0x557cbe5bd151 in run_builtin /home/phil/src/git/git.c:461
    #10 0x557cbe5bd151 in handle_builtin /home/phil/src/git/git.c:714
    #11 0x557cbe5c0503 in run_argv /home/phil/src/git/git.c:781
    #12 0x557cbe5c0503 in cmd_main /home/phil/src/git/git.c:912
    #13 0x557cbe5bad28 in main /home/phil/src/git/common-main.c:52
    #14 0x7fdd4b82eb24 in __libc_start_main (/usr/lib/libc.so.6+0x27b24)

Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
phillipwood added a commit to phillipwood/git that referenced this pull request Oct 7, 2021
In a sparse index it is possible for the tree that is being verified
to be freed while it is being verified. This happens when the index is
sparse but the cache tree is not and index_name_pos() looks up a path
from the cache tree that is a descendant of a sparse index entry. That
triggers a call to ensure_full_index() which frees the cache tree that
is being verified.  Carrying on trying to verify the tree after this
results in a use-after-free bug. Instead restart the verification if a
sparse index is converted to a full index. This bug is triggered by a
call to reset_head() in "git rebase --apply". Thanks to René Scharfe
and Derrick Stolee for their help analyzing the problem.

==74345==ERROR: AddressSanitizer: heap-use-after-free on address 0x606000001b20 at pc 0x557cbe82d3a2 bp 0x7ffdfee08090 sp 0x7ffdfee08080
READ of size 4 at 0x606000001b20 thread T0
    #0 0x557cbe82d3a1 in verify_one /home/phil/src/git/cache-tree.c:863
    #1 0x557cbe82ca9d in verify_one /home/phil/src/git/cache-tree.c:840
    #2 0x557cbe82ca9d in verify_one /home/phil/src/git/cache-tree.c:840
    #3 0x557cbe82ca9d in verify_one /home/phil/src/git/cache-tree.c:840
    #4 0x557cbe830a2b in cache_tree_verify /home/phil/src/git/cache-tree.c:910
    #5 0x557cbea53741 in write_locked_index /home/phil/src/git/read-cache.c:3250
    git#6 0x557cbeab7fdd in reset_head /home/phil/src/git/reset.c:87
    #7 0x557cbe72147f in cmd_rebase builtin/rebase.c:2074
    git#8 0x557cbe5bd151 in run_builtin /home/phil/src/git/git.c:461
    git#9 0x557cbe5bd151 in handle_builtin /home/phil/src/git/git.c:714
    git#10 0x557cbe5c0503 in run_argv /home/phil/src/git/git.c:781
    git#11 0x557cbe5c0503 in cmd_main /home/phil/src/git/git.c:912
    git#12 0x557cbe5bad28 in main /home/phil/src/git/common-main.c:52
    git#13 0x7fdd4b82eb24 in __libc_start_main (/usr/lib/libc.so.6+0x27b24)
    git#14 0x557cbe5bcb8d in _start (/home/phil/src/git/git+0x1b9b8d)

0x606000001b20 is located 0 bytes inside of 56-byte region [0x606000001b20,0x606000001b58)
freed by thread T0 here:
    #0 0x7fdd4bacff19 in __interceptor_free /build/gcc/src/gcc/libsanitizer/asan/asan_malloc_linux.cpp:127
    #1 0x557cbe82af60 in cache_tree_free /home/phil/src/git/cache-tree.c:35
    #2 0x557cbe82aee5 in cache_tree_free /home/phil/src/git/cache-tree.c:31
    #3 0x557cbe82aee5 in cache_tree_free /home/phil/src/git/cache-tree.c:31
    #4 0x557cbe82aee5 in cache_tree_free /home/phil/src/git/cache-tree.c:31
    #5 0x557cbeb2557a in ensure_full_index /home/phil/src/git/sparse-index.c:310
    git#6 0x557cbea45c4a in index_name_stage_pos /home/phil/src/git/read-cache.c:588
    #7 0x557cbe82ce37 in verify_one /home/phil/src/git/cache-tree.c:850
    git#8 0x557cbe82ca9d in verify_one /home/phil/src/git/cache-tree.c:840
    git#9 0x557cbe82ca9d in verify_one /home/phil/src/git/cache-tree.c:840
    git#10 0x557cbe82ca9d in verify_one /home/phil/src/git/cache-tree.c:840
    git#11 0x557cbe830a2b in cache_tree_verify /home/phil/src/git/cache-tree.c:910
    git#12 0x557cbea53741 in write_locked_index /home/phil/src/git/read-cache.c:3250
    git#13 0x557cbeab7fdd in reset_head /home/phil/src/git/reset.c:87
    git#14 0x557cbe72147f in cmd_rebase builtin/rebase.c:2074
    git#15 0x557cbe5bd151 in run_builtin /home/phil/src/git/git.c:461
    git#16 0x557cbe5bd151 in handle_builtin /home/phil/src/git/git.c:714
    git#17 0x557cbe5c0503 in run_argv /home/phil/src/git/git.c:781
    git#18 0x557cbe5c0503 in cmd_main /home/phil/src/git/git.c:912
    git#19 0x557cbe5bad28 in main /home/phil/src/git/common-main.c:52
    git#20 0x7fdd4b82eb24 in __libc_start_main (/usr/lib/libc.so.6+0x27b24)

previously allocated by thread T0 here:
    #0 0x7fdd4bad0459 in __interceptor_calloc /build/gcc/src/gcc/libsanitizer/asan/asan_malloc_linux.cpp:154
    #1 0x557cbebc1807 in xcalloc /home/phil/src/git/wrapper.c:140
    #2 0x557cbe82b7d8 in cache_tree /home/phil/src/git/cache-tree.c:17
    #3 0x557cbe82b7d8 in prime_cache_tree_rec /home/phil/src/git/cache-tree.c:763
    #4 0x557cbe82b837 in prime_cache_tree_rec /home/phil/src/git/cache-tree.c:764
    #5 0x557cbe82b837 in prime_cache_tree_rec /home/phil/src/git/cache-tree.c:764
    git#6 0x557cbe8304e1 in prime_cache_tree /home/phil/src/git/cache-tree.c:779
    #7 0x557cbeab7fa7 in reset_head /home/phil/src/git/reset.c:85
    git#8 0x557cbe72147f in cmd_rebase builtin/rebase.c:2074
    git#9 0x557cbe5bd151 in run_builtin /home/phil/src/git/git.c:461
    git#10 0x557cbe5bd151 in handle_builtin /home/phil/src/git/git.c:714
    git#11 0x557cbe5c0503 in run_argv /home/phil/src/git/git.c:781
    git#12 0x557cbe5c0503 in cmd_main /home/phil/src/git/git.c:912
    git#13 0x557cbe5bad28 in main /home/phil/src/git/common-main.c:52
    git#14 0x7fdd4b82eb24 in __libc_start_main (/usr/lib/libc.so.6+0x27b24)

Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
gitster pushed a commit that referenced this pull request Oct 7, 2021
In a sparse index it is possible for the tree that is being verified
to be freed while it is being verified. This happens when the index is
sparse but the cache tree is not and index_name_pos() looks up a path
from the cache tree that is a descendant of a sparse index entry. That
triggers a call to ensure_full_index() which frees the cache tree that
is being verified.  Carrying on trying to verify the tree after this
results in a use-after-free bug. Instead restart the verification if a
sparse index is converted to a full index. This bug is triggered by a
call to reset_head() in "git rebase --apply". Thanks to René Scharfe
and Derrick Stolee for their help analyzing the problem.

==74345==ERROR: AddressSanitizer: heap-use-after-free on address 0x606000001b20 at pc 0x557cbe82d3a2 bp 0x7ffdfee08090 sp 0x7ffdfee08080
READ of size 4 at 0x606000001b20 thread T0
    #0 0x557cbe82d3a1 in verify_one /home/phil/src/git/cache-tree.c:863
    #1 0x557cbe82ca9d in verify_one /home/phil/src/git/cache-tree.c:840
    #2 0x557cbe82ca9d in verify_one /home/phil/src/git/cache-tree.c:840
    #3 0x557cbe82ca9d in verify_one /home/phil/src/git/cache-tree.c:840
    #4 0x557cbe830a2b in cache_tree_verify /home/phil/src/git/cache-tree.c:910
    #5 0x557cbea53741 in write_locked_index /home/phil/src/git/read-cache.c:3250
    #6 0x557cbeab7fdd in reset_head /home/phil/src/git/reset.c:87
    #7 0x557cbe72147f in cmd_rebase builtin/rebase.c:2074
    #8 0x557cbe5bd151 in run_builtin /home/phil/src/git/git.c:461
    #9 0x557cbe5bd151 in handle_builtin /home/phil/src/git/git.c:714
    #10 0x557cbe5c0503 in run_argv /home/phil/src/git/git.c:781
    #11 0x557cbe5c0503 in cmd_main /home/phil/src/git/git.c:912
    #12 0x557cbe5bad28 in main /home/phil/src/git/common-main.c:52
    #13 0x7fdd4b82eb24 in __libc_start_main (/usr/lib/libc.so.6+0x27b24)
    #14 0x557cbe5bcb8d in _start (/home/phil/src/git/git+0x1b9b8d)

0x606000001b20 is located 0 bytes inside of 56-byte region [0x606000001b20,0x606000001b58)
freed by thread T0 here:
    #0 0x7fdd4bacff19 in __interceptor_free /build/gcc/src/gcc/libsanitizer/asan/asan_malloc_linux.cpp:127
    #1 0x557cbe82af60 in cache_tree_free /home/phil/src/git/cache-tree.c:35
    #2 0x557cbe82aee5 in cache_tree_free /home/phil/src/git/cache-tree.c:31
    #3 0x557cbe82aee5 in cache_tree_free /home/phil/src/git/cache-tree.c:31
    #4 0x557cbe82aee5 in cache_tree_free /home/phil/src/git/cache-tree.c:31
    #5 0x557cbeb2557a in ensure_full_index /home/phil/src/git/sparse-index.c:310
    #6 0x557cbea45c4a in index_name_stage_pos /home/phil/src/git/read-cache.c:588
    #7 0x557cbe82ce37 in verify_one /home/phil/src/git/cache-tree.c:850
    #8 0x557cbe82ca9d in verify_one /home/phil/src/git/cache-tree.c:840
    #9 0x557cbe82ca9d in verify_one /home/phil/src/git/cache-tree.c:840
    #10 0x557cbe82ca9d in verify_one /home/phil/src/git/cache-tree.c:840
    #11 0x557cbe830a2b in cache_tree_verify /home/phil/src/git/cache-tree.c:910
    #12 0x557cbea53741 in write_locked_index /home/phil/src/git/read-cache.c:3250
    #13 0x557cbeab7fdd in reset_head /home/phil/src/git/reset.c:87
    #14 0x557cbe72147f in cmd_rebase builtin/rebase.c:2074
    #15 0x557cbe5bd151 in run_builtin /home/phil/src/git/git.c:461
    #16 0x557cbe5bd151 in handle_builtin /home/phil/src/git/git.c:714
    #17 0x557cbe5c0503 in run_argv /home/phil/src/git/git.c:781
    #18 0x557cbe5c0503 in cmd_main /home/phil/src/git/git.c:912
    #19 0x557cbe5bad28 in main /home/phil/src/git/common-main.c:52
    #20 0x7fdd4b82eb24 in __libc_start_main (/usr/lib/libc.so.6+0x27b24)

previously allocated by thread T0 here:
    #0 0x7fdd4bad0459 in __interceptor_calloc /build/gcc/src/gcc/libsanitizer/asan/asan_malloc_linux.cpp:154
    #1 0x557cbebc1807 in xcalloc /home/phil/src/git/wrapper.c:140
    #2 0x557cbe82b7d8 in cache_tree /home/phil/src/git/cache-tree.c:17
    #3 0x557cbe82b7d8 in prime_cache_tree_rec /home/phil/src/git/cache-tree.c:763
    #4 0x557cbe82b837 in prime_cache_tree_rec /home/phil/src/git/cache-tree.c:764
    #5 0x557cbe82b837 in prime_cache_tree_rec /home/phil/src/git/cache-tree.c:764
    #6 0x557cbe8304e1 in prime_cache_tree /home/phil/src/git/cache-tree.c:779
    #7 0x557cbeab7fa7 in reset_head /home/phil/src/git/reset.c:85
    #8 0x557cbe72147f in cmd_rebase builtin/rebase.c:2074
    #9 0x557cbe5bd151 in run_builtin /home/phil/src/git/git.c:461
    #10 0x557cbe5bd151 in handle_builtin /home/phil/src/git/git.c:714
    #11 0x557cbe5c0503 in run_argv /home/phil/src/git/git.c:781
    #12 0x557cbe5c0503 in cmd_main /home/phil/src/git/git.c:912
    #13 0x557cbe5bad28 in main /home/phil/src/git/common-main.c:52
    #14 0x7fdd4b82eb24 in __libc_start_main (/usr/lib/libc.so.6+0x27b24)

Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
phillipwood added a commit to phillipwood/git that referenced this pull request Oct 16, 2021
In a sparse index it is possible for the tree that is being verified
to be freed while it is being verified. This happens when the index is
sparse but the cache tree is not and index_name_pos() looks up a path
from the cache tree that is a descendant of a sparse index entry. That
triggers a call to ensure_full_index() which frees the cache tree that
is being verified.  Carrying on trying to verify the tree after this
results in a use-after-free bug. Instead restart the verification if a
sparse index is converted to a full index. This bug is triggered by a
call to reset_head() in "git rebase --apply". Thanks to René Scharfe
and Derrick Stolee for their help analyzing the problem.

==74345==ERROR: AddressSanitizer: heap-use-after-free on address 0x606000001b20 at pc 0x557cbe82d3a2 bp 0x7ffdfee08090 sp 0x7ffdfee08080
READ of size 4 at 0x606000001b20 thread T0
    #0 0x557cbe82d3a1 in verify_one /home/phil/src/git/cache-tree.c:863
    #1 0x557cbe82ca9d in verify_one /home/phil/src/git/cache-tree.c:840
    #2 0x557cbe82ca9d in verify_one /home/phil/src/git/cache-tree.c:840
    #3 0x557cbe82ca9d in verify_one /home/phil/src/git/cache-tree.c:840
    #4 0x557cbe830a2b in cache_tree_verify /home/phil/src/git/cache-tree.c:910
    #5 0x557cbea53741 in write_locked_index /home/phil/src/git/read-cache.c:3250
    git#6 0x557cbeab7fdd in reset_head /home/phil/src/git/reset.c:87
    #7 0x557cbe72147f in cmd_rebase builtin/rebase.c:2074
    git#8 0x557cbe5bd151 in run_builtin /home/phil/src/git/git.c:461
    git#9 0x557cbe5bd151 in handle_builtin /home/phil/src/git/git.c:714
    git#10 0x557cbe5c0503 in run_argv /home/phil/src/git/git.c:781
    git#11 0x557cbe5c0503 in cmd_main /home/phil/src/git/git.c:912
    git#12 0x557cbe5bad28 in main /home/phil/src/git/common-main.c:52
    git#13 0x7fdd4b82eb24 in __libc_start_main (/usr/lib/libc.so.6+0x27b24)
    git#14 0x557cbe5bcb8d in _start (/home/phil/src/git/git+0x1b9b8d)

0x606000001b20 is located 0 bytes inside of 56-byte region [0x606000001b20,0x606000001b58)
freed by thread T0 here:
    #0 0x7fdd4bacff19 in __interceptor_free /build/gcc/src/gcc/libsanitizer/asan/asan_malloc_linux.cpp:127
    #1 0x557cbe82af60 in cache_tree_free /home/phil/src/git/cache-tree.c:35
    #2 0x557cbe82aee5 in cache_tree_free /home/phil/src/git/cache-tree.c:31
    #3 0x557cbe82aee5 in cache_tree_free /home/phil/src/git/cache-tree.c:31
    #4 0x557cbe82aee5 in cache_tree_free /home/phil/src/git/cache-tree.c:31
    #5 0x557cbeb2557a in ensure_full_index /home/phil/src/git/sparse-index.c:310
    git#6 0x557cbea45c4a in index_name_stage_pos /home/phil/src/git/read-cache.c:588
    #7 0x557cbe82ce37 in verify_one /home/phil/src/git/cache-tree.c:850
    git#8 0x557cbe82ca9d in verify_one /home/phil/src/git/cache-tree.c:840
    git#9 0x557cbe82ca9d in verify_one /home/phil/src/git/cache-tree.c:840
    git#10 0x557cbe82ca9d in verify_one /home/phil/src/git/cache-tree.c:840
    git#11 0x557cbe830a2b in cache_tree_verify /home/phil/src/git/cache-tree.c:910
    git#12 0x557cbea53741 in write_locked_index /home/phil/src/git/read-cache.c:3250
    git#13 0x557cbeab7fdd in reset_head /home/phil/src/git/reset.c:87
    git#14 0x557cbe72147f in cmd_rebase builtin/rebase.c:2074
    git#15 0x557cbe5bd151 in run_builtin /home/phil/src/git/git.c:461
    git#16 0x557cbe5bd151 in handle_builtin /home/phil/src/git/git.c:714
    git#17 0x557cbe5c0503 in run_argv /home/phil/src/git/git.c:781
    git#18 0x557cbe5c0503 in cmd_main /home/phil/src/git/git.c:912
    git#19 0x557cbe5bad28 in main /home/phil/src/git/common-main.c:52
    git#20 0x7fdd4b82eb24 in __libc_start_main (/usr/lib/libc.so.6+0x27b24)

previously allocated by thread T0 here:
    #0 0x7fdd4bad0459 in __interceptor_calloc /build/gcc/src/gcc/libsanitizer/asan/asan_malloc_linux.cpp:154
    #1 0x557cbebc1807 in xcalloc /home/phil/src/git/wrapper.c:140
    #2 0x557cbe82b7d8 in cache_tree /home/phil/src/git/cache-tree.c:17
    #3 0x557cbe82b7d8 in prime_cache_tree_rec /home/phil/src/git/cache-tree.c:763
    #4 0x557cbe82b837 in prime_cache_tree_rec /home/phil/src/git/cache-tree.c:764
    #5 0x557cbe82b837 in prime_cache_tree_rec /home/phil/src/git/cache-tree.c:764
    git#6 0x557cbe8304e1 in prime_cache_tree /home/phil/src/git/cache-tree.c:779
    #7 0x557cbeab7fa7 in reset_head /home/phil/src/git/reset.c:85
    git#8 0x557cbe72147f in cmd_rebase builtin/rebase.c:2074
    git#9 0x557cbe5bd151 in run_builtin /home/phil/src/git/git.c:461
    git#10 0x557cbe5bd151 in handle_builtin /home/phil/src/git/git.c:714
    git#11 0x557cbe5c0503 in run_argv /home/phil/src/git/git.c:781
    git#12 0x557cbe5c0503 in cmd_main /home/phil/src/git/git.c:912
    git#13 0x557cbe5bad28 in main /home/phil/src/git/common-main.c:52
    git#14 0x7fdd4b82eb24 in __libc_start_main (/usr/lib/libc.so.6+0x27b24)

Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
gitster pushed a commit that referenced this pull request Jan 7, 2022
When fetching packfiles, we write a bunch of lockfiles for the packfiles
we're writing into the repository. In order to not leave behind any
cruft in case we exit or receive a signal, we register both an exit
handler as well as signal handlers for common signals like SIGINT. These
handlers will then unlink the locks and free the data structure tracking
them. We have observed a deadlock in this logic though:

    (gdb) bt
    #0  __lll_lock_wait_private () at ../sysdeps/unix/sysv/linux/x86_64/lowlevellock.S:95
    #1  0x00007f4932bea2cd in _int_free (av=0x7f4932f2eb20 <main_arena>, p=0x3e3e4200, have_lock=0) at malloc.c:3969
    #2  0x00007f4932bee58c in __GI___libc_free (mem=<optimized out>) at malloc.c:2975
    #3  0x0000000000662ab1 in string_list_clear ()
    #4  0x000000000044f5bc in unlock_pack_on_signal ()
    #5  <signal handler called>
    #6  _int_free (av=0x7f4932f2eb20 <main_arena>, p=<optimized out>, have_lock=0) at malloc.c:4024
    #7  0x00007f4932bee58c in __GI___libc_free (mem=<optimized out>) at malloc.c:2975
    #8  0x000000000065afd5 in strbuf_release ()
    #9  0x000000000066ddb9 in delete_tempfile ()
    #10 0x0000000000610d0b in files_transaction_cleanup.isra ()
    #11 0x0000000000611718 in files_transaction_abort ()
    #12 0x000000000060d2ef in ref_transaction_abort ()
    #13 0x000000000060d441 in ref_transaction_prepare ()
    #14 0x000000000060e0b5 in ref_transaction_commit ()
    #15 0x00000000004511c2 in fetch_and_consume_refs ()
    #16 0x000000000045279a in cmd_fetch ()
    #17 0x0000000000407c48 in handle_builtin ()
    #18 0x0000000000408df2 in cmd_main ()
    #19 0x00000000004078b5 in main ()

The process was killed with a signal, which caused the signal handler to
kick in and try free the data structures after we have unlinked the
locks. It then deadlocks while calling free(3P).

The root cause of this is that it is not allowed to call certain
functions in async-signal handlers, as specified by signal-safety(7).
Next to most I/O functions, this list of disallowed functions also
includes memory-handling functions like malloc(3P) and free(3P) because
they may not be reentrant. As a result, if we execute such functions in
the signal handler, then they may operate on inconistent state and fail
in unexpected ways.

Fix this bug by not calling non-async-signal-safe functions when running
in the signal handler. We're about to re-raise the signal anyway and
will thus exit, so it's not much of a problem to keep the string list of
lockfiles untouched. Note that it's fine though to call unlink(2), so
we'll still clean up the lockfiles correctly.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Reviewed-by: brian m. carlson <sandals@crustytoothpaste.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
avar added a commit to avar/git that referenced this pull request Feb 25, 2022
For those tests that have 17 occurances of 'test_i18ngrep', replace it
with 'grep'. Splitting this incremental conversion by number of
occurances in the file is arbitrary, but help so keep the commit size
down.

The 'test_i18ngrep' wrapper has been deprecated since
d162b25 (tests: remove support for GIT_TEST_GETTEXT_POISON,
2021-01-20).

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
avar added a commit to avar/git that referenced this pull request Mar 2, 2022
For those tests that have 17 occurances of 'test_i18ngrep', replace it
with 'grep'. Splitting this incremental conversion by number of
occurances in the file is arbitrary, but help so keep the commit size
down.

The 'test_i18ngrep' wrapper has been deprecated since
d162b25 (tests: remove support for GIT_TEST_GETTEXT_POISON,
2021-01-20).

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants