Cater for cases where upstream path is one level lower than the root SVN... #18

chernjie · 2012-04-21T06:37:40Z

... URL:

e.g. https://svn.project.com/svn/Repo/one_level_down/trunk

…SVN URL: e.g. https://svn.project.com/svn/Repo/one_level_down/trunk

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)

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)

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)

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>

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>

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>

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>

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>

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>

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>

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>

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>

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>

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.) However the list pointer is later reused to iterate over our new list, but only for the limiting_can_increase_treesame() branch. We therefore need to introduce a new variable for that branch - and while we're here we can rename the original list to original_list as that makes its purpose more obvious. 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>

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.) However the list pointer is later reused to iterate over our new list, but only for the limiting_can_increase_treesame() branch. We therefore need to introduce a new variable for that branch - and while we're here we can rename the original list to original_list as that makes its purpose more obvious. 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 #8 0x514170 in orphaned_commit_warning builtin/checkout.c:1185:6 #9 0x512f05 in switch_branches builtin/checkout.c:1250:3 #10 0x50f8de in checkout_branch builtin/checkout.c:1646:9 #11 0x50ba12 in checkout_main builtin/checkout.c:2003:9 #12 0x5086c0 in cmd_checkout builtin/checkout.c:2055:8 #13 0x4cd91d in run_builtin git.c:467:11 #14 0x4cb5f3 in handle_builtin git.c:719:3 #15 0x4ccf47 in run_argv git.c:808:4 #16 0x4caf49 in cmd_main git.c:939:19 #17 0x69dc0e in main common-main.c:52:11 #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 #8 0x50ba12 in checkout_main builtin/checkout.c:2003:9 #9 0x5086c0 in cmd_checkout builtin/checkout.c:2055:8 #10 0x4cd91d in run_builtin git.c:467:11 #11 0x4cb5f3 in handle_builtin git.c:719:3 #12 0x4ccf47 in run_argv git.c:808:4 #13 0x4caf49 in cmd_main git.c:939:19 #14 0x69dc0e in main common-main.c:52:11 #15 0x7faaabd0e349 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>

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 loops 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, it seems like my compiler reuses 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. 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>

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>

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 loops 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, it seems like my compiler reuses 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. 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>

test-results/t0010-racy-git.out... ------------------------------------------------------------------------ Initialized empty Git repository in /__w/git/git/t/trash directory.t0010-racy-git/.git/ ================================================================= ==7972==ERROR: AddressSanitizer: invalid-pointer-pair: 0x6020000004f0 0x6020000004ef #0 0x7cfcbb in count_trailing_blank diff.c:621 #1 0x7cfe73 in check_blank_at_eof diff.c:637 #2 0x804cb9 in builtin_diff diff.c:3583 #3 0x809494 in run_diff_cmd diff.c:4428 #4 0x80ae96 in run_diff diff.c:4517 #5 0x80ae96 in diff_flush_patch diff.c:5870 git#6 0x80cb6a in diff_flush_patch_all_file_pairs diff.c:6409 #7 0x80cb6a in diff_flush diff.c:6552 git#8 0x7c831f in run_diff_files diff-lib.c:265 git#9 0x4ac215 in cmd_diff_files builtin/diff-files.c:82 git#10 0x41e8c9 in run_builtin git.c:465 git#11 0x41e8c9 in handle_builtin git.c:719 git#12 0x41f9ac in run_argv git.c:786 git#13 0x41f9ac in cmd_main git.c:917 git#14 0x419515 in main common-main.c:56 git#15 0x7f8a83bad55f in __libc_start_call_main (/lib64/libc.so.6+0x2d55f) git#16 0x7f8a83bad60b in __libc_start_main_impl (/lib64/libc.so.6+0x2d60b) git#17 0x41b494 in _start (git+0x41b494) 0x6020000004f0 is located 0 bytes inside of 7-byte region [0x6020000004f0,0x6020000004f7) allocated by thread T0 here: #0 0x7f8a847c291f in __interceptor_malloc (/lib64/libasan.so.6+0xae91f) #1 0xc30972 in do_xmalloc wrapper.c:51 #2 0xc30afd in do_xmallocz wrapper.c:85 #3 0xc30afd in do_xmallocz wrapper.c:75 #4 0xc30afd in xmallocz wrapper.c:93 #5 0x97fbe1 in unpack_loose_rest object-file.c:1312 git#6 0x98d70c in loose_object_info object-file.c:1479 #7 0x98e270 in do_oid_object_info_extended object-file.c:1577 git#8 0x98e9fe in oid_object_info_extended object-file.c:1639 git#9 0x7f3204 in diff_populate_filespec diff.c:4100 git#10 0x7f3ab9 in diff_filespec_is_binary diff.c:3329 git#11 0x805ec6 in builtin_diff diff.c:3507 git#12 0x809494 in run_diff_cmd diff.c:4428 git#13 0x80ae96 in run_diff diff.c:4517 git#14 0x80ae96 in diff_flush_patch diff.c:5870 git#15 0x80cb6a in diff_flush_patch_all_file_pairs diff.c:6409 git#16 0x80cb6a in diff_flush diff.c:6552 git#17 0x7c831f in run_diff_files diff-lib.c:265 git#18 0x4ac215 in cmd_diff_files builtin/diff-files.c:82 git#19 0x41e8c9 in run_builtin git.c:465 git#20 0x41e8c9 in handle_builtin git.c:719 git#21 0x41f9ac in run_argv git.c:786 git#22 0x41f9ac in cmd_main git.c:917 git#23 0x419515 in main common-main.c:56 git#24 0x7f8a83bad55f in __libc_start_call_main (/lib64/libc.so.6+0x2d55f) git#25 0x7f8a83bad60b in __libc_start_main_impl (/lib64/libc.so.6+0x2d60b) git#26 0x41b494 in _start (git+0x41b494) 0x6020000004ef is located 1 bytes to the left of 7-byte region [0x6020000004f0,0x6020000004f7) allocated by thread T0 here: #0 0x7f8a847c291f in __interceptor_malloc (/lib64/libasan.so.6+0xae91f) #1 0xc30972 in do_xmalloc wrapper.c:51 #2 0xc30afd in do_xmallocz wrapper.c:85 #3 0xc30afd in do_xmallocz wrapper.c:75 #4 0xc30afd in xmallocz wrapper.c:93 #5 0x97fbe1 in unpack_loose_rest object-file.c:1312 git#6 0x98d70c in loose_object_info object-file.c:1479 #7 0x98e270 in do_oid_object_info_extended object-file.c:1577 git#8 0x98e9fe in oid_object_info_extended object-file.c:1639 git#9 0x7f3204 in diff_populate_filespec diff.c:4100 git#10 0x7f3ab9 in diff_filespec_is_binary diff.c:3329 git#11 0x805ec6 in builtin_diff diff.c:3507 git#12 0x809494 in run_diff_cmd diff.c:4428 git#13 0x80ae96 in run_diff diff.c:4517 git#14 0x80ae96 in diff_flush_patch diff.c:5870 git#15 0x80cb6a in diff_flush_patch_all_file_pairs diff.c:6409 git#16 0x80cb6a in diff_flush diff.c:6552 git#17 0x7c831f in run_diff_files diff-lib.c:265 git#18 0x4ac215 in cmd_diff_files builtin/diff-files.c:82 git#19 0x41e8c9 in run_builtin git.c:465 git#20 0x41e8c9 in handle_builtin git.c:719 git#21 0x41f9ac in run_argv git.c:786 git#22 0x41f9ac in cmd_main git.c:917 git#23 0x419515 in main common-main.c:56 git#24 0x7f8a83bad55f in __libc_start_call_main (/lib64/libc.so.6+0x2d55f) git#25 0x7f8a83bad60b in __libc_start_main_impl (/lib64/libc.so.6+0x2d60b) git#26 0x41b494 in _start (git+0x41b494) SUMMARY: AddressSanitizer: invalid-pointer-pair diff.c:621 in count_trailing_blank ==7972==ABORTING Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>

This fails with e.g. this t9350-fast-export.sh test, perhaps we have a \0-delimited and use that last byte for an implicit \n? + git tag -a -m valentin muss ================================================================= ==32504==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x60d0000005ab at pc 0x556431ac3cac bp 0x7ffe67fcff50 sp 0x7ffe67fcff48 READ of size 1 at 0x60d0000005ab thread T0 #0 0x556431ac3cab in parse_tag_buffer tag.c:155 #1 0x5564319194f3 in parse_object_buffer object.c:245 #2 0x556431919a11 in parse_object object.c:298 #3 0x5564319e2e96 in write_ref_to_lockfile refs/files-backend.c:1781 #4 0x5564319e71e8 in lock_ref_for_update refs/files-backend.c:2590 #5 0x5564319e7db5 in files_transaction_prepare refs/files-backend.c:2763 git#6 0x5564319d41d9 in ref_transaction_prepare refs.c:2146 #7 0x5564319d4465 in ref_transaction_commit refs.c:2195 git#8 0x5564316706b9 in cmd_tag builtin/tag.c:630 git#9 0x5564314168ff in run_builtin git.c:466 git#10 0x5564314172ab in handle_builtin git.c:720 git#11 0x5564314179d5 in run_argv git.c:787 git#12 0x55643141874f in cmd_main git.c:920 git#13 0x556431695a4d in main common-main.c:56 git#14 0x7fa9ba2aa81c in __libc_start_main ../csu/libc-start.c:332 git#15 0x556431412209 in _start (git+0x1d2209) 0x60d0000005ab is located 0 bytes to the right of 139-byte region [0x60d000000520,0x60d0000005ab) allocated by thread T0 here: #0 0x7fa9ba5fc9cf in __interceptor_malloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:69 #1 0x556431b3f334 in do_xmalloc wrapper.c:51 #2 0x556431b3f43a in do_xmallocz wrapper.c:85 #3 0x556431b3f4ab in xmallocz wrapper.c:93 #4 0x556431902114 in unpack_loose_rest object-file.c:1312 #5 0x556431902ed9 in loose_object_info object-file.c:1479 git#6 0x556431903785 in do_oid_object_info_extended object-file.c:1577 #7 0x556431903b2d in oid_object_info_extended object-file.c:1639 git#8 0x556431903f3f in read_object object-file.c:1671 git#9 0x5564319043c7 in read_object_file_extended object-file.c:1714 git#10 0x556431917ff0 in repo_read_object_file object-store.h:253 git#11 0x5564319198b3 in parse_object object.c:290 git#12 0x5564319e2e96 in write_ref_to_lockfile refs/files-backend.c:1781 git#13 0x5564319e71e8 in lock_ref_for_update refs/files-backend.c:2590 git#14 0x5564319e7db5 in files_transaction_prepare refs/files-backend.c:2763 git#15 0x5564319d41d9 in ref_transaction_prepare refs.c:2146 git#16 0x5564319d4465 in ref_transaction_commit refs.c:2195 git#17 0x5564316706b9 in cmd_tag builtin/tag.c:630 git#18 0x5564314168ff in run_builtin git.c:466 git#19 0x5564314172ab in handle_builtin git.c:720 git#20 0x5564314179d5 in run_argv git.c:787 git#21 0x55643141874f in cmd_main git.c:920 git#22 0x556431695a4d in main common-main.c:56 git#23 0x7fa9ba2aa81c in __libc_start_main ../csu/libc-start.c:332 git#24 0x556431412209 in _start (git+0x1d2209) SUMMARY: AddressSanitizer: heap-buffer-overflow tag.c:155 in parse_tag_buffer Shadow bytes around the buggy address: 0x0c1a7fff8060: fd fd fd fd fd fd fd fa fa fa fa fa fa fa fa fa 0x0c1a7fff8070: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0x0c1a7fff8080: 00 03 fa fa fa fa fa fa fa fa fd fd fd fd fd fd 0x0c1a7fff8090: fd fd fd fd fd fd fd fd fd fd fd fa fa fa fa fa 0x0c1a7fff80a0: fa fa fa fa 00 00 00 00 00 00 00 00 00 00 00 00 =>0x0c1a7fff80b0: 00 00 00 00 00[03]fa fa fa fa fa fa fa fa fa fa 0x0c1a7fff80c0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa 0x0c1a7fff80d0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa 0x0c1a7fff80e0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa 0x0c1a7fff80f0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa 0x0c1a7fff8100: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa Shadow byte legend (one shadow byte represents 8 application bytes): Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>

struct ref contains a FLEX_ARRAY and was allocated without taking that in account which resulted in the following error when running t1006-cat-file.sh if git is compiled with SANITIZE=address. ==16304==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x60f0000002d0 at pc 0x7fed25a474da bp 0x7ffd936d2960 sp 0x7ffd936d2108 READ of size 9 at 0x60f0000002d0 thread T0 #0 0x7fed25a474d9 in __interceptor_strlen /usr/src/debug/gcc/libsanitizer/sanitizer_common/sanitizer_common_interceptors.inc:389 #1 0x5566018e9a27 in copy_ref remote.c:1044 #2 0x5566018e9c57 in copy_ref_list remote.c:1059 #3 0x55660172da37 in do_fetch_pack_v2 fetch-pack.c:1626 #4 0x55660172da37 in fetch_pack fetch-pack.c:2035 #5 0x5566019d0644 in fetch_refs_via_pack transport.c:524 git#6 0x5566019d3fbc in transport_fetch_refs transport.c:1580 #7 0x5566013e92cf in get_remote_info builtin/cat-file.c:547 git#8 0x5566013e92cf in parse_remote_info builtin/cat-file.c:670 git#9 0x5566013ee84d in batch_objects_command builtin/cat-file.c:775 git#10 0x5566013ee84d in batch_objects builtin/cat-file.c:885 git#11 0x5566013ee84d in cmd_cat_file builtin/cat-file.c:1063 git#12 0x5566013af2c5 in run_builtin git.c:466 git#13 0x5566013af2c5 in handle_builtin git.c:720 git#14 0x5566013b2853 in run_argv git.c:787 git#15 0x5566013b2853 in cmd_main git.c:920 git#16 0x5566013aceb2 in main common-main.c:56 git#17 0x7fed2583f2cf (/usr/lib/libc.so.6+0x232cf) git#18 0x7fed2583f389 in __libc_start_main (/usr/lib/libc.so.6+0x23389) git#19 0x5566013aedb4 in _start (git+0x1c9db4) 0x60f0000002d0 is located 0 bytes to the right of 176-byte region [0x60f000000220,0x60f0000002d0) allocated by thread T0 here: #0 0x7fed25abf411 in __interceptor_calloc /usr/src/debug/gcc/libsanitizer/asan/asan_malloc_linux.cpp:77 #1 0x556601a1281a in xcalloc wrapper.c:150 #2 0x5566019d0e41 in fetch_refs_via_pack transport.c:482 #3 0x5566019d3fbc in transport_fetch_refs transport.c:1580 #4 0x5566013e92cf in get_remote_info builtin/cat-file.c:547 #5 0x5566013e92cf in parse_remote_info builtin/cat-file.c:670 git#6 0x5566013ee84d in batch_objects_command builtin/cat-file.c:775 #7 0x5566013ee84d in batch_objects builtin/cat-file.c:885 git#8 0x5566013ee84d in cmd_cat_file builtin/cat-file.c:1063 git#9 0x5566013af2c5 in run_builtin git.c:466 git#10 0x5566013af2c5 in handle_builtin git.c:720 git#11 0x5566013b2853 in run_argv git.c:787 git#12 0x5566013b2853 in cmd_main git.c:920 git#13 0x5566013aceb2 in main common-main.c:56 git#14 0x7fed2583f2cf (/usr/lib/libc.so.6+0x232cf) git#15 0x7fed2583f389 in __libc_start_main (/usr/lib/libc.so.6+0x23389) git#16 0x5566013aedb4 in _start (git+0x1c9db4) SUMMARY: AddressSanitizer: heap-buffer-overflow /usr/src/debug/gcc/libsanitizer/sanitizer_common/sanitizer_common_interceptors.inc:389 in __interceptor_strlen Use FLEX_ALLOC_STR() with an empty string to fix this. Also use array notation when indexing the oid array. Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>

struct ref contains a FLEX_ARRAY and was allocated without taking that in account which resulted in the following error when running t1006-cat-file.sh when git is compiled with SANITIZE=address. ==16304==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x60f0000002d0 at pc 0x7fed25a474da bp 0x7ffd936d2960 sp 0x7ffd936d2108 READ of size 9 at 0x60f0000002d0 thread T0 #0 0x7fed25a474d9 in __interceptor_strlen /usr/src/debug/gcc/libsanitizer/sanitizer_common/sanitizer_common_interceptors.inc:389 #1 0x5566018e9a27 in copy_ref remote.c:1044 #2 0x5566018e9c57 in copy_ref_list remote.c:1059 #3 0x55660172da37 in do_fetch_pack_v2 fetch-pack.c:1626 #4 0x55660172da37 in fetch_pack fetch-pack.c:2035 #5 0x5566019d0644 in fetch_refs_via_pack transport.c:524 git#6 0x5566019d3fbc in transport_fetch_refs transport.c:1580 #7 0x5566013e92cf in get_remote_info builtin/cat-file.c:547 git#8 0x5566013e92cf in parse_remote_info builtin/cat-file.c:670 git#9 0x5566013ee84d in batch_objects_command builtin/cat-file.c:775 git#10 0x5566013ee84d in batch_objects builtin/cat-file.c:885 git#11 0x5566013ee84d in cmd_cat_file builtin/cat-file.c:1063 git#12 0x5566013af2c5 in run_builtin git.c:466 git#13 0x5566013af2c5 in handle_builtin git.c:720 git#14 0x5566013b2853 in run_argv git.c:787 git#15 0x5566013b2853 in cmd_main git.c:920 git#16 0x5566013aceb2 in main common-main.c:56 git#17 0x7fed2583f2cf (/usr/lib/libc.so.6+0x232cf) git#18 0x7fed2583f389 in __libc_start_main (/usr/lib/libc.so.6+0x23389) git#19 0x5566013aedb4 in _start (git+0x1c9db4) 0x60f0000002d0 is located 0 bytes to the right of 176-byte region [0x60f000000220,0x60f0000002d0) allocated by thread T0 here: #0 0x7fed25abf411 in __interceptor_calloc /usr/src/debug/gcc/libsanitizer/asan/asan_malloc_linux.cpp:77 #1 0x556601a1281a in xcalloc wrapper.c:150 #2 0x5566019d0e41 in fetch_refs_via_pack transport.c:482 #3 0x5566019d3fbc in transport_fetch_refs transport.c:1580 #4 0x5566013e92cf in get_remote_info builtin/cat-file.c:547 #5 0x5566013e92cf in parse_remote_info builtin/cat-file.c:670 git#6 0x5566013ee84d in batch_objects_command builtin/cat-file.c:775 #7 0x5566013ee84d in batch_objects builtin/cat-file.c:885 git#8 0x5566013ee84d in cmd_cat_file builtin/cat-file.c:1063 git#9 0x5566013af2c5 in run_builtin git.c:466 git#10 0x5566013af2c5 in handle_builtin git.c:720 git#11 0x5566013b2853 in run_argv git.c:787 git#12 0x5566013b2853 in cmd_main git.c:920 git#13 0x5566013aceb2 in main common-main.c:56 git#14 0x7fed2583f2cf (/usr/lib/libc.so.6+0x232cf) git#15 0x7fed2583f389 in __libc_start_main (/usr/lib/libc.so.6+0x23389) git#16 0x5566013aedb4 in _start (git+0x1c9db4) SUMMARY: AddressSanitizer: heap-buffer-overflow /usr/src/debug/gcc/libsanitizer/sanitizer_common/sanitizer_common_interceptors.inc:389 in __interceptor_strlen Use FLEX_ALLOC_STR() with an empty string to fix this and avoid allocating objects_info_refs unless it is going to be used. As this is intended to be squashed into the next re-roll I've also included an unrelated style fix to use array indexing when accessing the oid array. Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>

struct ref contains a FLEX_ARRAY and was allocated without taking that in account which resulted in the following error when running t1006-cat-file.sh when git is compiled with SANITIZE=address. ==16304==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x60f0000002d0 at pc 0x7fed25a474da bp 0x7ffd936d2960 sp 0x7ffd936d2108 READ of size 9 at 0x60f0000002d0 thread T0 #0 0x7fed25a474d9 in __interceptor_strlen /usr/src/debug/gcc/libsanitizer/sanitizer_common/sanitizer_common_interceptors.inc:389 #1 0x5566018e9a27 in copy_ref remote.c:1044 #2 0x5566018e9c57 in copy_ref_list remote.c:1059 #3 0x55660172da37 in do_fetch_pack_v2 fetch-pack.c:1626 #4 0x55660172da37 in fetch_pack fetch-pack.c:2035 #5 0x5566019d0644 in fetch_refs_via_pack transport.c:524 git#6 0x5566019d3fbc in transport_fetch_refs transport.c:1580 #7 0x5566013e92cf in get_remote_info builtin/cat-file.c:547 git#8 0x5566013e92cf in parse_remote_info builtin/cat-file.c:670 git#9 0x5566013ee84d in batch_objects_command builtin/cat-file.c:775 git#10 0x5566013ee84d in batch_objects builtin/cat-file.c:885 git#11 0x5566013ee84d in cmd_cat_file builtin/cat-file.c:1063 git#12 0x5566013af2c5 in run_builtin git.c:466 git#13 0x5566013af2c5 in handle_builtin git.c:720 git#14 0x5566013b2853 in run_argv git.c:787 git#15 0x5566013b2853 in cmd_main git.c:920 git#16 0x5566013aceb2 in main common-main.c:56 git#17 0x7fed2583f2cf (/usr/lib/libc.so.6+0x232cf) git#18 0x7fed2583f389 in __libc_start_main (/usr/lib/libc.so.6+0x23389) git#19 0x5566013aedb4 in _start (git+0x1c9db4) 0x60f0000002d0 is located 0 bytes to the right of 176-byte region [0x60f000000220,0x60f0000002d0) allocated by thread T0 here: #0 0x7fed25abf411 in __interceptor_calloc /usr/src/debug/gcc/libsanitizer/asan/asan_malloc_linux.cpp:77 #1 0x556601a1281a in xcalloc wrapper.c:150 #2 0x5566019d0e41 in fetch_refs_via_pack transport.c:482 #3 0x5566019d3fbc in transport_fetch_refs transport.c:1580 #4 0x5566013e92cf in get_remote_info builtin/cat-file.c:547 #5 0x5566013e92cf in parse_remote_info builtin/cat-file.c:670 git#6 0x5566013ee84d in batch_objects_command builtin/cat-file.c:775 #7 0x5566013ee84d in batch_objects builtin/cat-file.c:885 git#8 0x5566013ee84d in cmd_cat_file builtin/cat-file.c:1063 git#9 0x5566013af2c5 in run_builtin git.c:466 git#10 0x5566013af2c5 in handle_builtin git.c:720 git#11 0x5566013b2853 in run_argv git.c:787 git#12 0x5566013b2853 in cmd_main git.c:920 git#13 0x5566013aceb2 in main common-main.c:56 git#14 0x7fed2583f2cf (/usr/lib/libc.so.6+0x232cf) git#15 0x7fed2583f389 in __libc_start_main (/usr/lib/libc.so.6+0x23389) git#16 0x5566013aedb4 in _start (git+0x1c9db4) SUMMARY: AddressSanitizer: heap-buffer-overflow /usr/src/debug/gcc/libsanitizer/sanitizer_common/sanitizer_common_interceptors.inc:389 in __interceptor_strlen Use alloc_ref() with an empty string to fix this and avoid allocating objects_info_refs unless it is going to be used. As this is intended to be squashed into the next re-roll I've also included an unrelated style fix to use array indexing when accessing the oid array. Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>

This fails with e.g. this t9350-fast-export.sh test, perhaps we have a \0-delimited and use that last byte for an implicit \n? + git tag -a -m valentin muss ================================================================= ==32504==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x60d0000005ab at pc 0x556431ac3cac bp 0x7ffe67fcff50 sp 0x7ffe67fcff48 READ of size 1 at 0x60d0000005ab thread T0 #0 0x556431ac3cab in parse_tag_buffer tag.c:155 #1 0x5564319194f3 in parse_object_buffer object.c:245 #2 0x556431919a11 in parse_object object.c:298 #3 0x5564319e2e96 in write_ref_to_lockfile refs/files-backend.c:1781 #4 0x5564319e71e8 in lock_ref_for_update refs/files-backend.c:2590 #5 0x5564319e7db5 in files_transaction_prepare refs/files-backend.c:2763 git#6 0x5564319d41d9 in ref_transaction_prepare refs.c:2146 #7 0x5564319d4465 in ref_transaction_commit refs.c:2195 git#8 0x5564316706b9 in cmd_tag builtin/tag.c:630 git#9 0x5564314168ff in run_builtin git.c:466 git#10 0x5564314172ab in handle_builtin git.c:720 git#11 0x5564314179d5 in run_argv git.c:787 git#12 0x55643141874f in cmd_main git.c:920 git#13 0x556431695a4d in main common-main.c:56 git#14 0x7fa9ba2aa81c in __libc_start_main ../csu/libc-start.c:332 git#15 0x556431412209 in _start (git+0x1d2209) 0x60d0000005ab is located 0 bytes to the right of 139-byte region [0x60d000000520,0x60d0000005ab) allocated by thread T0 here: #0 0x7fa9ba5fc9cf in __interceptor_malloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:69 #1 0x556431b3f334 in do_xmalloc wrapper.c:51 #2 0x556431b3f43a in do_xmallocz wrapper.c:85 #3 0x556431b3f4ab in xmallocz wrapper.c:93 #4 0x556431902114 in unpack_loose_rest object-file.c:1312 #5 0x556431902ed9 in loose_object_info object-file.c:1479 git#6 0x556431903785 in do_oid_object_info_extended object-file.c:1577 #7 0x556431903b2d in oid_object_info_extended object-file.c:1639 git#8 0x556431903f3f in read_object object-file.c:1671 git#9 0x5564319043c7 in read_object_file_extended object-file.c:1714 git#10 0x556431917ff0 in repo_read_object_file object-store.h:253 git#11 0x5564319198b3 in parse_object object.c:290 git#12 0x5564319e2e96 in write_ref_to_lockfile refs/files-backend.c:1781 git#13 0x5564319e71e8 in lock_ref_for_update refs/files-backend.c:2590 git#14 0x5564319e7db5 in files_transaction_prepare refs/files-backend.c:2763 git#15 0x5564319d41d9 in ref_transaction_prepare refs.c:2146 git#16 0x5564319d4465 in ref_transaction_commit refs.c:2195 git#17 0x5564316706b9 in cmd_tag builtin/tag.c:630 git#18 0x5564314168ff in run_builtin git.c:466 git#19 0x5564314172ab in handle_builtin git.c:720 git#20 0x5564314179d5 in run_argv git.c:787 git#21 0x55643141874f in cmd_main git.c:920 git#22 0x556431695a4d in main common-main.c:56 git#23 0x7fa9ba2aa81c in __libc_start_main ../csu/libc-start.c:332 git#24 0x556431412209 in _start (git+0x1d2209) SUMMARY: AddressSanitizer: heap-buffer-overflow tag.c:155 in parse_tag_buffer Shadow bytes around the buggy address: 0x0c1a7fff8060: fd fd fd fd fd fd fd fa fa fa fa fa fa fa fa fa 0x0c1a7fff8070: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0x0c1a7fff8080: 00 03 fa fa fa fa fa fa fa fa fd fd fd fd fd fd 0x0c1a7fff8090: fd fd fd fd fd fd fd fd fd fd fd fa fa fa fa fa 0x0c1a7fff80a0: fa fa fa fa 00 00 00 00 00 00 00 00 00 00 00 00 =>0x0c1a7fff80b0: 00 00 00 00 00[03]fa fa fa fa fa fa fa fa fa fa 0x0c1a7fff80c0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa 0x0c1a7fff80d0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa 0x0c1a7fff80e0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa 0x0c1a7fff80f0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa 0x0c1a7fff8100: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa Shadow byte legend (one shadow byte represents 8 application bytes): Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>

In the tmp-objdir api, tmp_objdir_create will create a temporary directory but also register signal handlers responsible for removing the directory's contents and the directory itself. However, the function responsible for recursively removing the contents and directory, remove_dir_recurse() calls opendir(3) and closedir(3). This can be problematic because these functions allocate and free memory, which are not async-signal-safe functions. This can lead to deadlocks. One place we call tmp_objdir_create() is in git-receive-pack, where we create a temporary quarantine directory "incoming". Incoming objects will be written to this directory before they get moved to the object directory. We have observed this code leading to a deadlock: Thread 1 (Thread 0x7f621ba0b200 (LWP 326305)): #0 __lll_lock_wait_private (futex=futex@entry=0x7f621bbf8b80 <main_arena>) at ./lowlevellock.c:35 #1 0x00007f621baa635b in __GI___libc_malloc (bytes=bytes@entry=32816) at malloc.c:3064 #2 0x00007f621bae9f49 in __alloc_dir (statp=0x7fff2ea7ed60, flags=0, close_fd=true, fd=5) at ../sysdeps/posix/opendir.c:118 #3 opendir_tail (fd=5) at ../sysdeps/posix/opendir.c:69 #4 __opendir (name=<optimized out>) at ../sysdeps/posix/opendir.c:92 #5 0x0000557c19c77de1 in remove_dir_recurse () git#6 0x0000557c19d81a4f in remove_tmp_objdir_on_signal () #7 <signal handler called> git#8 _int_malloc (av=av@entry=0x7f621bbf8b80 <main_arena>, bytes=bytes@entry=7160) at malloc.c:4116 git#9 0x00007f621baa62c9 in __GI___libc_malloc (bytes=7160) at malloc.c:3066 git#10 0x00007f621bd1e987 in inflateInit2_ () from /opt/gitlab/embedded/lib/libz.so.1 git#11 0x0000557c19dbe5f4 in git_inflate_init () git#12 0x0000557c19cee02a in unpack_compressed_entry () git#13 0x0000557c19cf08cb in unpack_entry () git#14 0x0000557c19cf0f32 in packed_object_info () git#15 0x0000557c19cd68cd in do_oid_object_info_extended () git#16 0x0000557c19cd6e2b in read_object_file_extended () git#17 0x0000557c19cdec2f in parse_object () git#18 0x0000557c19c34977 in lookup_commit_reference_gently () git#19 0x0000557c19d69309 in mark_uninteresting () git#20 0x0000557c19d2d180 in do_for_each_repo_ref_iterator () git#21 0x0000557c19d21678 in for_each_ref () git#22 0x0000557c19d6a94f in assign_shallow_commits_to_refs () git#23 0x0000557c19bc02b2 in cmd_receive_pack () git#24 0x0000557c19b29fdd in handle_builtin () git#25 0x0000557c19b2a526 in cmd_main () git#26 0x0000557c19b28ea2 in main () To prevent this, add a flag REMOVE_DIR_SIGNAL that allows remove_dir_recurse() to know that a signal is being handled and avoid calling opendir(3). One implication of this change is that when signal handling, the temporary directory may not get cleaned up properly. Signed-off-by: John Cai <johncai86@gmail.com>

In the tmp-objdir api, tmp_objdir_create will create a temporary directory but also register signal handlers responsible for removing the directory's contents and the directory itself. However, the function responsible for recursively removing the contents and directory, remove_dir_recurse() calls opendir(3) and closedir(3). This can be problematic because these functions allocate and free memory, which are not async-signal-safe functions. This can lead to deadlocks. One place we call tmp_objdir_create() is in git-receive-pack, where we create a temporary quarantine directory "incoming". Incoming objects will be written to this directory before they get moved to the object directory. We have observed this code leading to a deadlock: Thread 1 (Thread 0x7f621ba0b200 (LWP 326305)): #0 __lll_lock_wait_private (futex=futex@entry=0x7f621bbf8b80 <main_arena>) at ./lowlevellock.c:35 #1 0x00007f621baa635b in __GI___libc_malloc (bytes=bytes@entry=32816) at malloc.c:3064 #2 0x00007f621bae9f49 in __alloc_dir (statp=0x7fff2ea7ed60, flags=0, close_fd=true, fd=5) at ../sysdeps/posix/opendir.c:118 #3 opendir_tail (fd=5) at ../sysdeps/posix/opendir.c:69 #4 __opendir (name=<optimized out>) at ../sysdeps/posix/opendir.c:92 #5 0x0000557c19c77de1 in remove_dir_recurse () git#6 0x0000557c19d81a4f in remove_tmp_objdir_on_signal () #7 <signal handler called> git#8 _int_malloc (av=av@entry=0x7f621bbf8b80 <main_arena>, bytes=bytes@entry=7160) at malloc.c:4116 git#9 0x00007f621baa62c9 in __GI___libc_malloc (bytes=7160) at malloc.c:3066 git#10 0x00007f621bd1e987 in inflateInit2_ () from /opt/gitlab/embedded/lib/libz.so.1 git#11 0x0000557c19dbe5f4 in git_inflate_init () git#12 0x0000557c19cee02a in unpack_compressed_entry () git#13 0x0000557c19cf08cb in unpack_entry () git#14 0x0000557c19cf0f32 in packed_object_info () git#15 0x0000557c19cd68cd in do_oid_object_info_extended () git#16 0x0000557c19cd6e2b in read_object_file_extended () git#17 0x0000557c19cdec2f in parse_object () git#18 0x0000557c19c34977 in lookup_commit_reference_gently () git#19 0x0000557c19d69309 in mark_uninteresting () git#20 0x0000557c19d2d180 in do_for_each_repo_ref_iterator () git#21 0x0000557c19d21678 in for_each_ref () git#22 0x0000557c19d6a94f in assign_shallow_commits_to_refs () git#23 0x0000557c19bc02b2 in cmd_receive_pack () git#24 0x0000557c19b29fdd in handle_builtin () git#25 0x0000557c19b2a526 in cmd_main () git#26 0x0000557c19b28ea2 in main () Since we can't do the cleanup in a portable and signal-safe way, skip the cleanup when we're handling a signal. This means that when signal handling, the temporary directory may not get cleaned up properly. This is mitigated by b3cecf4 (tmp-objdir: new API for creating temporary writable databases, 2021-12-06) which changed the default name and allows gc to clean up these temporary directories. Signed-off-by: John Cai <johncai86@gmail.com>

In the tmp-objdir api, tmp_objdir_create will create a temporary directory but also register signal handlers responsible for removing the directory's contents and the directory itself. However, the function responsible for recursively removing the contents and directory, remove_dir_recurse() calls opendir(3) and closedir(3). This can be problematic because these functions allocate and free memory, which are not async-signal-safe functions. This can lead to deadlocks. One place we call tmp_objdir_create() is in git-receive-pack, where we create a temporary quarantine directory "incoming". Incoming objects will be written to this directory before they get moved to the object directory. We have observed this code leading to a deadlock: Thread 1 (Thread 0x7f621ba0b200 (LWP 326305)): #0 __lll_lock_wait_private (futex=futex@entry=0x7f621bbf8b80 <main_arena>) at ./lowlevellock.c:35 #1 0x00007f621baa635b in __GI___libc_malloc (bytes=bytes@entry=32816) at malloc.c:3064 #2 0x00007f621bae9f49 in __alloc_dir (statp=0x7fff2ea7ed60, flags=0, close_fd=true, fd=5) at ../sysdeps/posix/opendir.c:118 #3 opendir_tail (fd=5) at ../sysdeps/posix/opendir.c:69 #4 __opendir (name=<optimized out>) at ../sysdeps/posix/opendir.c:92 #5 0x0000557c19c77de1 in remove_dir_recurse () git#6 0x0000557c19d81a4f in remove_tmp_objdir_on_signal () #7 <signal handler called> git#8 _int_malloc (av=av@entry=0x7f621bbf8b80 <main_arena>, bytes=bytes@entry=7160) at malloc.c:4116 git#9 0x00007f621baa62c9 in __GI___libc_malloc (bytes=7160) at malloc.c:3066 git#10 0x00007f621bd1e987 in inflateInit2_ () from /opt/gitlab/embedded/lib/libz.so.1 git#11 0x0000557c19dbe5f4 in git_inflate_init () git#12 0x0000557c19cee02a in unpack_compressed_entry () git#13 0x0000557c19cf08cb in unpack_entry () git#14 0x0000557c19cf0f32 in packed_object_info () git#15 0x0000557c19cd68cd in do_oid_object_info_extended () git#16 0x0000557c19cd6e2b in read_object_file_extended () git#17 0x0000557c19cdec2f in parse_object () git#18 0x0000557c19c34977 in lookup_commit_reference_gently () git#19 0x0000557c19d69309 in mark_uninteresting () git#20 0x0000557c19d2d180 in do_for_each_repo_ref_iterator () git#21 0x0000557c19d21678 in for_each_ref () git#22 0x0000557c19d6a94f in assign_shallow_commits_to_refs () git#23 0x0000557c19bc02b2 in cmd_receive_pack () git#24 0x0000557c19b29fdd in handle_builtin () git#25 0x0000557c19b2a526 in cmd_main () git#26 0x0000557c19b28ea2 in main () Since we can't do the cleanup in a portable and signal-safe way, skip the cleanup when we're handling a signal. This means that when signal handling, the temporary directory may not get cleaned up properly. This is mitigated by b3cecf4 (tmp-objdir: new API for creating temporary writable databases, 2021-12-06) which changed the default name and allows gc to clean up these temporary directories. In the event of a normal exit, we should still be cleaning up via the atexit() handler. Helped-by: Jeff King <peff@peff.net> Signed-off-by: John Cai <johncai86@gmail.com>

In the tmp-objdir api, tmp_objdir_create will create a temporary directory but also register signal handlers responsible for removing the directory's contents and the directory itself. However, the function responsible for recursively removing the contents and directory, remove_dir_recurse() calls opendir(3) and closedir(3). This can be problematic because these functions allocate and free memory, which are not async-signal-safe functions. This can lead to deadlocks. One place we call tmp_objdir_create() is in git-receive-pack, where we create a temporary quarantine directory "incoming". Incoming objects will be written to this directory before they get moved to the object directory. We have observed this code leading to a deadlock: Thread 1 (Thread 0x7f621ba0b200 (LWP 326305)): #0 __lll_lock_wait_private (futex=futex@entry=0x7f621bbf8b80 <main_arena>) at ./lowlevellock.c:35 #1 0x00007f621baa635b in __GI___libc_malloc (bytes=bytes@entry=32816) at malloc.c:3064 #2 0x00007f621bae9f49 in __alloc_dir (statp=0x7fff2ea7ed60, flags=0, close_fd=true, fd=5) at ../sysdeps/posix/opendir.c:118 #3 opendir_tail (fd=5) at ../sysdeps/posix/opendir.c:69 #4 __opendir (name=<optimized out>) at ../sysdeps/posix/opendir.c:92 #5 0x0000557c19c77de1 in remove_dir_recurse () #6 0x0000557c19d81a4f in remove_tmp_objdir_on_signal () #7 <signal handler called> #8 _int_malloc (av=av@entry=0x7f621bbf8b80 <main_arena>, bytes=bytes@entry=7160) at malloc.c:4116 #9 0x00007f621baa62c9 in __GI___libc_malloc (bytes=7160) at malloc.c:3066 #10 0x00007f621bd1e987 in inflateInit2_ () from /opt/gitlab/embedded/lib/libz.so.1 #11 0x0000557c19dbe5f4 in git_inflate_init () #12 0x0000557c19cee02a in unpack_compressed_entry () #13 0x0000557c19cf08cb in unpack_entry () #14 0x0000557c19cf0f32 in packed_object_info () #15 0x0000557c19cd68cd in do_oid_object_info_extended () #16 0x0000557c19cd6e2b in read_object_file_extended () #17 0x0000557c19cdec2f in parse_object () #18 0x0000557c19c34977 in lookup_commit_reference_gently () #19 0x0000557c19d69309 in mark_uninteresting () #20 0x0000557c19d2d180 in do_for_each_repo_ref_iterator () #21 0x0000557c19d21678 in for_each_ref () #22 0x0000557c19d6a94f in assign_shallow_commits_to_refs () #23 0x0000557c19bc02b2 in cmd_receive_pack () #24 0x0000557c19b29fdd in handle_builtin () #25 0x0000557c19b2a526 in cmd_main () #26 0x0000557c19b28ea2 in main () Since we can't do the cleanup in a portable and signal-safe way, skip the cleanup when we're handling a signal. This means that when signal handling, the temporary directory may not get cleaned up properly. This is mitigated by b3cecf4 (tmp-objdir: new API for creating temporary writable databases, 2021-12-06) which changed the default name and allows gc to clean up these temporary directories. In the event of a normal exit, we should still be cleaning up via the atexit() handler. Helped-by: Jeff King <peff@peff.net> Signed-off-by: John Cai <johncai86@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

In the tmp-objdir api, tmp_objdir_create will create a temporary directory but also register signal handlers responsible for removing the directory's contents and the directory itself. However, the function responsible for recursively removing the contents and directory, remove_dir_recurse() calls opendir(3) and closedir(3). This can be problematic because these functions allocate and free memory, which are not async-signal-safe functions. This can lead to deadlocks. One place we call tmp_objdir_create() is in git-receive-pack, where we create a temporary quarantine directory "incoming". Incoming objects will be written to this directory before they get moved to the object directory. We have observed this code leading to a deadlock: Thread 1 (Thread 0x7f621ba0b200 (LWP 326305)): #0 __lll_lock_wait_private (futex=futex@entry=0x7f621bbf8b80 <main_arena>) at ./lowlevellock.c:35 #1 0x00007f621baa635b in __GI___libc_malloc (bytes=bytes@entry=32816) at malloc.c:3064 #2 0x00007f621bae9f49 in __alloc_dir (statp=0x7fff2ea7ed60, flags=0, close_fd=true, fd=5) at ../sysdeps/posix/opendir.c:118 #3 opendir_tail (fd=5) at ../sysdeps/posix/opendir.c:69 #4 __opendir (name=<optimized out>) at ../sysdeps/posix/opendir.c:92 #5 0x0000557c19c77de1 in remove_dir_recurse () git#6 0x0000557c19d81a4f in remove_tmp_objdir_on_signal () #7 <signal handler called> git#8 _int_malloc (av=av@entry=0x7f621bbf8b80 <main_arena>, bytes=bytes@entry=7160) at malloc.c:4116 git#9 0x00007f621baa62c9 in __GI___libc_malloc (bytes=7160) at malloc.c:3066 git#10 0x00007f621bd1e987 in inflateInit2_ () from /opt/gitlab/embedded/lib/libz.so.1 git#11 0x0000557c19dbe5f4 in git_inflate_init () git#12 0x0000557c19cee02a in unpack_compressed_entry () git#13 0x0000557c19cf08cb in unpack_entry () git#14 0x0000557c19cf0f32 in packed_object_info () git#15 0x0000557c19cd68cd in do_oid_object_info_extended () git#16 0x0000557c19cd6e2b in read_object_file_extended () git#17 0x0000557c19cdec2f in parse_object () git#18 0x0000557c19c34977 in lookup_commit_reference_gently () git#19 0x0000557c19d69309 in mark_uninteresting () git#20 0x0000557c19d2d180 in do_for_each_repo_ref_iterator () git#21 0x0000557c19d21678 in for_each_ref () git#22 0x0000557c19d6a94f in assign_shallow_commits_to_refs () git#23 0x0000557c19bc02b2 in cmd_receive_pack () git#24 0x0000557c19b29fdd in handle_builtin () git#25 0x0000557c19b2a526 in cmd_main () git#26 0x0000557c19b28ea2 in main () Since we can't do the cleanup in a portable and signal-safe way, skip the cleanup when we're handling a signal. This means that when signal handling, the temporary directory may not get cleaned up properly. This is mitigated by b3cecf4 (tmp-objdir: new API for creating temporary writable databases, 2021-12-06) which changed the default name and allows gc to clean up these temporary directories. In the event of a normal exit, we should still be cleaning up via the atexit() handler. Helped-by: Jeff King <peff@peff.net> Signed-off-by: John Cai <johncai86@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

There is an out-of-bounds read possible when parsing gitattributes that have an attribute that is 2^31+1 bytes long. This is caused due to an integer overflow when we assign the result of strlen(3P) to an `int`, where we use the wrapped-around value in a subsequent call to memcpy(3P). The following code reproduces the issue: blob=$(perl -e 'print "a" x 2147483649 . " attr"' | git hash-object -w --stdin) git update-index --add --cacheinfo 100644,$blob,.gitattributes git check-attr --all file AddressSanitizer:DEADLYSIGNAL ================================================================= ==8451==ERROR: AddressSanitizer: SEGV on unknown address 0x7f93efa00800 (pc 0x7f94f1f8f082 bp 0x7ffddb59b3a0 sp 0x7ffddb59ab28 T0) ==8451==The signal is caused by a READ memory access. #0 0x7f94f1f8f082 (/usr/lib/libc.so.6+0x176082) #1 0x7f94f2047d9c in __interceptor_strspn /usr/src/debug/gcc/libsanitizer/sanitizer_common/sanitizer_common_interceptors.inc:752 #2 0x560e190f7f26 in parse_attr_line attr.c:375 #3 0x560e190f9663 in handle_attr_line attr.c:660 #4 0x560e190f9ddd in read_attr_from_index attr.c:769 #5 0x560e190f9f14 in read_attr attr.c:797 #6 0x560e190fa24e in bootstrap_attr_stack attr.c:867 #7 0x560e190fa4a5 in prepare_attr_stack attr.c:902 #8 0x560e190fb5dc in collect_some_attrs attr.c:1097 #9 0x560e190fb93f in git_all_attrs attr.c:1128 #10 0x560e18e6136e in check_attr builtin/check-attr.c:67 #11 0x560e18e61c12 in cmd_check_attr builtin/check-attr.c:183 #12 0x560e18e15993 in run_builtin git.c:466 #13 0x560e18e16397 in handle_builtin git.c:721 #14 0x560e18e16b2b in run_argv git.c:788 #15 0x560e18e17991 in cmd_main git.c:926 #16 0x560e190ae2bd in main common-main.c:57 #17 0x7f94f1e3c28f (/usr/lib/libc.so.6+0x2328f) #18 0x7f94f1e3c349 in __libc_start_main (/usr/lib/libc.so.6+0x23349) #19 0x560e18e110e4 in _start ../sysdeps/x86_64/start.S:115 AddressSanitizer can not provide additional info. SUMMARY: AddressSanitizer: SEGV (/usr/lib/libc.so.6+0x176082) ==8451==ABORTING Fix this bug by converting the variable to a `size_t` instead. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>

It is possible to trigger an integer overflow when parsing attribute names when there are more than 2^31 of them for a single pattern. This can either lead to us dying due to trying to request too many bytes: blob=$(perl -e 'print "f" . " a=" x 2147483649' | git hash-object -w --stdin) git update-index --add --cacheinfo 100644,$blob,.gitattributes git attr-check --all file ================================================================= ==1022==ERROR: AddressSanitizer: requested allocation size 0xfffffff800000032 (0xfffffff800001038 after adjustments for alignment, red zones etc.) exceeds maximum supported size of 0x10000000000 (thread T0) #0 0x7fd3efabf411 in __interceptor_calloc /usr/src/debug/gcc/libsanitizer/asan/asan_malloc_linux.cpp:77 #1 0x5563a0a1e3d3 in xcalloc wrapper.c:150 #2 0x5563a058d005 in parse_attr_line attr.c:384 #3 0x5563a058e661 in handle_attr_line attr.c:660 #4 0x5563a058eddb in read_attr_from_index attr.c:769 #5 0x5563a058ef12 in read_attr attr.c:797 #6 0x5563a058f24c in bootstrap_attr_stack attr.c:867 #7 0x5563a058f4a3 in prepare_attr_stack attr.c:902 #8 0x5563a05905da in collect_some_attrs attr.c:1097 #9 0x5563a059093d in git_all_attrs attr.c:1128 #10 0x5563a02f636e in check_attr builtin/check-attr.c:67 #11 0x5563a02f6c12 in cmd_check_attr builtin/check-attr.c:183 #12 0x5563a02aa993 in run_builtin git.c:466 #13 0x5563a02ab397 in handle_builtin git.c:721 #14 0x5563a02abb2b in run_argv git.c:788 #15 0x5563a02ac991 in cmd_main git.c:926 #16 0x5563a05432bd in main common-main.c:57 #17 0x7fd3ef82228f (/usr/lib/libc.so.6+0x2328f) ==1022==HINT: if you don't care about these errors you may set allocator_may_return_null=1 SUMMARY: AddressSanitizer: allocation-size-too-big /usr/src/debug/gcc/libsanitizer/asan/asan_malloc_linux.cpp:77 in __interceptor_calloc ==1022==ABORTING Or, much worse, it can lead to an out-of-bounds write because we underallocate and then memcpy(3P) into an array: perl -e ' print "A " . "\rh="x2000000000; print "\rh="x2000000000; print "\rh="x294967294 . "\n" ' >.gitattributes git add .gitattributes git commit -am "evil attributes" $ git clone --quiet /path/to/repo ================================================================= ==15062==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x602000002550 at pc 0x5555559884d5 bp 0x7fffffffbc60 sp 0x7fffffffbc58 WRITE of size 8 at 0x602000002550 thread T0 #0 0x5555559884d4 in parse_attr_line attr.c:393 #1 0x5555559884d4 in handle_attr_line attr.c:660 #2 0x555555988902 in read_attr_from_index attr.c:784 #3 0x555555988902 in read_attr_from_index attr.c:747 #4 0x555555988a1d in read_attr attr.c:800 #5 0x555555989b0c in bootstrap_attr_stack attr.c:882 #6 0x555555989b0c in prepare_attr_stack attr.c:917 #7 0x555555989b0c in collect_some_attrs attr.c:1112 #8 0x55555598b141 in git_check_attr attr.c:1126 #9 0x555555a13004 in convert_attrs convert.c:1311 #10 0x555555a95e04 in checkout_entry_ca entry.c:553 #11 0x555555d58bf6 in checkout_entry entry.h:42 #12 0x555555d58bf6 in check_updates unpack-trees.c:480 #13 0x555555d5eb55 in unpack_trees unpack-trees.c:2040 #14 0x555555785ab7 in checkout builtin/clone.c:724 #15 0x555555785ab7 in cmd_clone builtin/clone.c:1384 #16 0x55555572443c in run_builtin git.c:466 #17 0x55555572443c in handle_builtin git.c:721 #18 0x555555727872 in run_argv git.c:788 #19 0x555555727872 in cmd_main git.c:926 #20 0x555555721fa0 in main common-main.c:57 #21 0x7ffff73f1d09 in __libc_start_main ../csu/libc-start.c:308 #22 0x555555723f39 in _start (git+0x1cff39) 0x602000002552 is located 0 bytes to the right of 2-byte region [0x602000002550,0x602000002552) allocated by thread T0 here: #0 0x7ffff768c037 in __interceptor_calloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:154 #1 0x555555d7fff7 in xcalloc wrapper.c:150 #2 0x55555598815f in parse_attr_line attr.c:384 #3 0x55555598815f in handle_attr_line attr.c:660 #4 0x555555988902 in read_attr_from_index attr.c:784 #5 0x555555988902 in read_attr_from_index attr.c:747 #6 0x555555988a1d in read_attr attr.c:800 #7 0x555555989b0c in bootstrap_attr_stack attr.c:882 #8 0x555555989b0c in prepare_attr_stack attr.c:917 #9 0x555555989b0c in collect_some_attrs attr.c:1112 #10 0x55555598b141 in git_check_attr attr.c:1126 #11 0x555555a13004 in convert_attrs convert.c:1311 #12 0x555555a95e04 in checkout_entry_ca entry.c:553 #13 0x555555d58bf6 in checkout_entry entry.h:42 #14 0x555555d58bf6 in check_updates unpack-trees.c:480 #15 0x555555d5eb55 in unpack_trees unpack-trees.c:2040 #16 0x555555785ab7 in checkout builtin/clone.c:724 #17 0x555555785ab7 in cmd_clone builtin/clone.c:1384 #18 0x55555572443c in run_builtin git.c:466 #19 0x55555572443c in handle_builtin git.c:721 #20 0x555555727872 in run_argv git.c:788 #21 0x555555727872 in cmd_main git.c:926 #22 0x555555721fa0 in main common-main.c:57 #23 0x7ffff73f1d09 in __libc_start_main ../csu/libc-start.c:308 SUMMARY: AddressSanitizer: heap-buffer-overflow attr.c:393 in parse_attr_line Shadow bytes around the buggy address: 0x0c047fff8450: fa fa 00 02 fa fa 00 07 fa fa fd fd fa fa 00 00 0x0c047fff8460: fa fa 02 fa fa fa fd fd fa fa 00 06 fa fa 05 fa 0x0c047fff8470: fa fa fd fd fa fa 00 02 fa fa 06 fa fa fa 05 fa 0x0c047fff8480: fa fa 07 fa fa fa fd fd fa fa 00 01 fa fa 00 02 0x0c047fff8490: fa fa 00 03 fa fa 00 fa fa fa 00 01 fa fa 00 03 =>0x0c047fff84a0: fa fa 00 01 fa fa 00 02 fa fa[02]fa fa fa fa fa 0x0c047fff84b0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa 0x0c047fff84c0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa 0x0c047fff84d0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa 0x0c047fff84e0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa 0x0c047fff84f0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa Shadow byte legend (one shadow byte represents 8 application bytes): Addressable: 00 Partially addressable: 01 02 03 04 05 06 07 Heap left redzone: fa Freed heap region: fd Stack left redzone: f1 Stack mid redzone: f2 Stack right redzone: f3 Stack after return: f5 Stack use after scope: f8 Global redzone: f9 Global init order: f6 Poisoned by user: f7 Container overflow: fc Array cookie: ac Intra object redzone: bb ASan internal: fe Left alloca redzone: ca Right alloca redzone: cb Shadow gap: cc ==15062==ABORTING Fix this bug by using `size_t` instead to count the number of attributes so that this value cannot reasonably overflow without running out of memory before already. Reported-by: Markus Vervier <markus.vervier@x41-dsec.de> Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>

When using a padding specifier in the pretty format passed to git-log(1) we need to calculate the string length in several places. These string lengths are stored in `int`s though, which means that these can easily overflow when the input lengths exceeds 2GB. This can ultimately lead to an out-of-bounds write when these are used in a call to memcpy(3P): ==8340==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x7f1ec62f97fe at pc 0x7f2127e5f427 bp 0x7ffd3bd63de0 sp 0x7ffd3bd63588 WRITE of size 1 at 0x7f1ec62f97fe thread T0 #0 0x7f2127e5f426 in __interceptor_memcpy /usr/src/debug/gcc/libsanitizer/sanitizer_common/sanitizer_common_interceptors.inc:827 #1 0x5628e96aa605 in format_and_pad_commit pretty.c:1762 #2 0x5628e96aa7f4 in format_commit_item pretty.c:1801 #3 0x5628e97cdb24 in strbuf_expand strbuf.c:429 #4 0x5628e96ab060 in repo_format_commit_message pretty.c:1869 #5 0x5628e96acd0f in pretty_print_commit pretty.c:2161 #6 0x5628e95a44c8 in show_log log-tree.c:781 #7 0x5628e95a76ba in log_tree_commit log-tree.c:1117 #8 0x5628e922bed5 in cmd_log_walk_no_free builtin/log.c:508 #9 0x5628e922c35b in cmd_log_walk builtin/log.c:549 #10 0x5628e922f1a2 in cmd_log builtin/log.c:883 #11 0x5628e9106993 in run_builtin git.c:466 #12 0x5628e9107397 in handle_builtin git.c:721 #13 0x5628e9107b07 in run_argv git.c:788 #14 0x5628e91088a7 in cmd_main git.c:923 #15 0x5628e939d682 in main common-main.c:57 #16 0x7f2127c3c28f (/usr/lib/libc.so.6+0x2328f) #17 0x7f2127c3c349 in __libc_start_main (/usr/lib/libc.so.6+0x23349) #18 0x5628e91020e4 in _start ../sysdeps/x86_64/start.S:115 0x7f1ec62f97fe is located 2 bytes to the left of 4831838265-byte region [0x7f1ec62f9800,0x7f1fe62f9839) allocated by thread T0 here: #0 0x7f2127ebe7ea in __interceptor_realloc /usr/src/debug/gcc/libsanitizer/asan/asan_malloc_linux.cpp:85 #1 0x5628e98774d4 in xrealloc wrapper.c:136 #2 0x5628e97cb01c in strbuf_grow strbuf.c:99 #3 0x5628e97ccd42 in strbuf_addchars strbuf.c:327 #4 0x5628e96aa55c in format_and_pad_commit pretty.c:1761 #5 0x5628e96aa7f4 in format_commit_item pretty.c:1801 #6 0x5628e97cdb24 in strbuf_expand strbuf.c:429 #7 0x5628e96ab060 in repo_format_commit_message pretty.c:1869 #8 0x5628e96acd0f in pretty_print_commit pretty.c:2161 #9 0x5628e95a44c8 in show_log log-tree.c:781 #10 0x5628e95a76ba in log_tree_commit log-tree.c:1117 #11 0x5628e922bed5 in cmd_log_walk_no_free builtin/log.c:508 #12 0x5628e922c35b in cmd_log_walk builtin/log.c:549 #13 0x5628e922f1a2 in cmd_log builtin/log.c:883 #14 0x5628e9106993 in run_builtin git.c:466 #15 0x5628e9107397 in handle_builtin git.c:721 #16 0x5628e9107b07 in run_argv git.c:788 #17 0x5628e91088a7 in cmd_main git.c:923 #18 0x5628e939d682 in main common-main.c:57 #19 0x7f2127c3c28f (/usr/lib/libc.so.6+0x2328f) #20 0x7f2127c3c349 in __libc_start_main (/usr/lib/libc.so.6+0x23349) #21 0x5628e91020e4 in _start ../sysdeps/x86_64/start.S:115 SUMMARY: AddressSanitizer: heap-buffer-overflow /usr/src/debug/gcc/libsanitizer/sanitizer_common/sanitizer_common_interceptors.inc:827 in __interceptor_memcpy Shadow bytes around the buggy address: 0x0fe458c572a0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa 0x0fe458c572b0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa 0x0fe458c572c0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa 0x0fe458c572d0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa 0x0fe458c572e0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa =>0x0fe458c572f0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa[fa] 0x0fe458c57300: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0x0fe458c57310: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0x0fe458c57320: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0x0fe458c57330: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0x0fe458c57340: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 Shadow byte legend (one shadow byte represents 8 application bytes): Addressable: 00 Partially addressable: 01 02 03 04 05 06 07 Heap left redzone: fa Freed heap region: fd Stack left redzone: f1 Stack mid redzone: f2 Stack right redzone: f3 Stack after return: f5 Stack use after scope: f8 Global redzone: f9 Global init order: f6 Poisoned by user: f7 Container overflow: fc Array cookie: ac Intra object redzone: bb ASan internal: fe Left alloca redzone: ca Right alloca redzone: cb ==8340==ABORTING The pretty format can also be used in `git archive` operations via the `export-subst` attribute. So this is what in our opinion makes this a critical issue in the context of Git forges which allow to download an archive of user supplied Git repositories. Fix this vulnerability by using `size_t` instead of `int` to track the string lengths. Add tests which detect this vulnerability when Git is compiled with the address sanitizer. Reported-by: Joern Schneeweisz <jschneeweisz@gitlab.com> Original-patch-by: Joern Schneeweisz <jschneeweisz@gitlab.com> Modified-by: Taylor Blau <me@ttalorr.com> Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>

With the `%>>(<N>)` pretty formatter, you can ask git-log(1) et al to steal spaces. To do so we need to look ahead of the next token to see whether there are spaces there. This loop takes into account ANSI sequences that end with an `m`, and if it finds any it will skip them until it finds the first space. While doing so it does not take into account the buffer's limits though and easily does an out-of-bounds read. Add a test that hits this behaviour. While we don't have an easy way to verify this, the test causes the following failure when run with `SANITIZE=address`: ==37941==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x603000000baf at pc 0x55ba6f88e0d0 bp 0x7ffc84c50d20 sp 0x7ffc84c50d10 READ of size 1 at 0x603000000baf thread T0 #0 0x55ba6f88e0cf in format_and_pad_commit pretty.c:1712 #1 0x55ba6f88e7b4 in format_commit_item pretty.c:1801 #2 0x55ba6f9b1ae4 in strbuf_expand strbuf.c:429 #3 0x55ba6f88f020 in repo_format_commit_message pretty.c:1869 #4 0x55ba6f890ccf in pretty_print_commit pretty.c:2161 #5 0x55ba6f7884c8 in show_log log-tree.c:781 #6 0x55ba6f78b6ba in log_tree_commit log-tree.c:1117 #7 0x55ba6f40fed5 in cmd_log_walk_no_free builtin/log.c:508 #8 0x55ba6f41035b in cmd_log_walk builtin/log.c:549 #9 0x55ba6f4131a2 in cmd_log builtin/log.c:883 #10 0x55ba6f2ea993 in run_builtin git.c:466 #11 0x55ba6f2eb397 in handle_builtin git.c:721 #12 0x55ba6f2ebb07 in run_argv git.c:788 #13 0x55ba6f2ec8a7 in cmd_main git.c:923 #14 0x55ba6f581682 in main common-main.c:57 #15 0x7f2d08c3c28f (/usr/lib/libc.so.6+0x2328f) #16 0x7f2d08c3c349 in __libc_start_main (/usr/lib/libc.so.6+0x23349) #17 0x55ba6f2e60e4 in _start ../sysdeps/x86_64/start.S:115 0x603000000baf is located 1 bytes to the left of 24-byte region [0x603000000bb0,0x603000000bc8) allocated by thread T0 here: #0 0x7f2d08ebe7ea in __interceptor_realloc /usr/src/debug/gcc/libsanitizer/asan/asan_malloc_linux.cpp:85 #1 0x55ba6fa5b494 in xrealloc wrapper.c:136 #2 0x55ba6f9aefdc in strbuf_grow strbuf.c:99 #3 0x55ba6f9b0a06 in strbuf_add strbuf.c:298 #4 0x55ba6f9b1a25 in strbuf_expand strbuf.c:418 #5 0x55ba6f88f020 in repo_format_commit_message pretty.c:1869 #6 0x55ba6f890ccf in pretty_print_commit pretty.c:2161 #7 0x55ba6f7884c8 in show_log log-tree.c:781 #8 0x55ba6f78b6ba in log_tree_commit log-tree.c:1117 #9 0x55ba6f40fed5 in cmd_log_walk_no_free builtin/log.c:508 #10 0x55ba6f41035b in cmd_log_walk builtin/log.c:549 #11 0x55ba6f4131a2 in cmd_log builtin/log.c:883 #12 0x55ba6f2ea993 in run_builtin git.c:466 #13 0x55ba6f2eb397 in handle_builtin git.c:721 #14 0x55ba6f2ebb07 in run_argv git.c:788 #15 0x55ba6f2ec8a7 in cmd_main git.c:923 #16 0x55ba6f581682 in main common-main.c:57 #17 0x7f2d08c3c28f (/usr/lib/libc.so.6+0x2328f) #18 0x7f2d08c3c349 in __libc_start_main (/usr/lib/libc.so.6+0x23349) #19 0x55ba6f2e60e4 in _start ../sysdeps/x86_64/start.S:115 SUMMARY: AddressSanitizer: heap-buffer-overflow pretty.c:1712 in format_and_pad_commit Shadow bytes around the buggy address: 0x0c067fff8120: fa fa fd fd fd fa fa fa fd fd fd fa fa fa fd fd 0x0c067fff8130: fd fd fa fa fd fd fd fd fa fa fd fd fd fa fa fa 0x0c067fff8140: fd fd fd fa fa fa fd fd fd fa fa fa fd fd fd fa 0x0c067fff8150: fa fa fd fd fd fd fa fa 00 00 00 fa fa fa fd fd 0x0c067fff8160: fd fa fa fa fd fd fd fa fa fa fd fd fd fa fa fa =>0x0c067fff8170: fd fd fd fa fa[fa]00 00 00 fa fa fa 00 00 00 fa 0x0c067fff8180: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa 0x0c067fff8190: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa 0x0c067fff81a0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa 0x0c067fff81b0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa 0x0c067fff81c0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa Shadow byte legend (one shadow byte represents 8 application bytes): Addressable: 00 Partially addressable: 01 02 03 04 05 06 07 Heap left redzone: fa Freed heap region: fd Stack left redzone: f1 Stack mid redzone: f2 Stack right redzone: f3 Stack after return: f5 Stack use after scope: f8 Global redzone: f9 Global init order: f6 Poisoned by user: f7 Container overflow: fc Array cookie: ac Intra object redzone: bb ASan internal: fe Left alloca redzone: ca Right alloca redzone: cb Luckily enough, this would only cause us to copy the out-of-bounds data into the formatted commit in case we really had an ANSI sequence preceding our buffer. So this bug likely has no security consequences. Fix it regardless by not traversing past the buffer's start. Reported-by: Patrick Steinhardt <ps@pks.im> Reported-by: Eric Sesterhenn <eric.sesterhenn@x41-dsec.de> Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>

The return type of both `utf8_strwidth()` and `utf8_strnwidth()` is `int`, but we operate on string lengths which are typically of type `size_t`. This means that when the string is longer than `INT_MAX`, we will overflow and thus return a negative result. This can lead to an out-of-bounds write with `--pretty=format:%<1)%B` and a commit message that is 2^31+1 bytes long: ================================================================= ==26009==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x603000001168 at pc 0x7f95c4e5f427 bp 0x7ffd8541c900 sp 0x7ffd8541c0a8 WRITE of size 2147483649 at 0x603000001168 thread T0 #0 0x7f95c4e5f426 in __interceptor_memcpy /usr/src/debug/gcc/libsanitizer/sanitizer_common/sanitizer_common_interceptors.inc:827 #1 0x5612bbb1068c in format_and_pad_commit pretty.c:1763 #2 0x5612bbb1087a in format_commit_item pretty.c:1801 #3 0x5612bbc33bab in strbuf_expand strbuf.c:429 #4 0x5612bbb110e7 in repo_format_commit_message pretty.c:1869 #5 0x5612bbb12d96 in pretty_print_commit pretty.c:2161 #6 0x5612bba0a4d5 in show_log log-tree.c:781 #7 0x5612bba0d6c7 in log_tree_commit log-tree.c:1117 #8 0x5612bb691ed5 in cmd_log_walk_no_free builtin/log.c:508 #9 0x5612bb69235b in cmd_log_walk builtin/log.c:549 #10 0x5612bb6951a2 in cmd_log builtin/log.c:883 #11 0x5612bb56c993 in run_builtin git.c:466 #12 0x5612bb56d397 in handle_builtin git.c:721 #13 0x5612bb56db07 in run_argv git.c:788 #14 0x5612bb56e8a7 in cmd_main git.c:923 #15 0x5612bb803682 in main common-main.c:57 #16 0x7f95c4c3c28f (/usr/lib/libc.so.6+0x2328f) #17 0x7f95c4c3c349 in __libc_start_main (/usr/lib/libc.so.6+0x23349) #18 0x5612bb5680e4 in _start ../sysdeps/x86_64/start.S:115 0x603000001168 is located 0 bytes to the right of 24-byte region [0x603000001150,0x603000001168) allocated by thread T0 here: #0 0x7f95c4ebe7ea in __interceptor_realloc /usr/src/debug/gcc/libsanitizer/asan/asan_malloc_linux.cpp:85 #1 0x5612bbcdd556 in xrealloc wrapper.c:136 #2 0x5612bbc310a3 in strbuf_grow strbuf.c:99 #3 0x5612bbc32acd in strbuf_add strbuf.c:298 #4 0x5612bbc33aec in strbuf_expand strbuf.c:418 #5 0x5612bbb110e7 in repo_format_commit_message pretty.c:1869 #6 0x5612bbb12d96 in pretty_print_commit pretty.c:2161 #7 0x5612bba0a4d5 in show_log log-tree.c:781 #8 0x5612bba0d6c7 in log_tree_commit log-tree.c:1117 #9 0x5612bb691ed5 in cmd_log_walk_no_free builtin/log.c:508 #10 0x5612bb69235b in cmd_log_walk builtin/log.c:549 #11 0x5612bb6951a2 in cmd_log builtin/log.c:883 #12 0x5612bb56c993 in run_builtin git.c:466 #13 0x5612bb56d397 in handle_builtin git.c:721 #14 0x5612bb56db07 in run_argv git.c:788 #15 0x5612bb56e8a7 in cmd_main git.c:923 #16 0x5612bb803682 in main common-main.c:57 #17 0x7f95c4c3c28f (/usr/lib/libc.so.6+0x2328f) SUMMARY: AddressSanitizer: heap-buffer-overflow /usr/src/debug/gcc/libsanitizer/sanitizer_common/sanitizer_common_interceptors.inc:827 in __interceptor_memcpy Shadow bytes around the buggy address: 0x0c067fff81d0: fd fd fd fa fa fa fd fd fd fa fa fa fd fd fd fa 0x0c067fff81e0: fa fa fd fd fd fd fa fa fd fd fd fd fa fa fd fd 0x0c067fff81f0: fd fa fa fa fd fd fd fa fa fa fd fd fd fa fa fa 0x0c067fff8200: fd fd fd fa fa fa fd fd fd fd fa fa 00 00 00 fa 0x0c067fff8210: fa fa fd fd fd fa fa fa fd fd fd fa fa fa fd fd =>0x0c067fff8220: fd fa fa fa fd fd fd fa fa fa 00 00 00[fa]fa fa 0x0c067fff8230: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa 0x0c067fff8240: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa 0x0c067fff8250: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa 0x0c067fff8260: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa 0x0c067fff8270: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa Shadow byte legend (one shadow byte represents 8 application bytes): Addressable: 00 Partially addressable: 01 02 03 04 05 06 07 Heap left redzone: fa Freed heap region: fd Stack left redzone: f1 Stack mid redzone: f2 Stack right redzone: f3 Stack after return: f5 Stack use after scope: f8 Global redzone: f9 Global init order: f6 Poisoned by user: f7 Container overflow: fc Array cookie: ac Intra object redzone: bb ASan internal: fe Left alloca redzone: ca Right alloca redzone: cb ==26009==ABORTING Now the proper fix for this would be to convert both functions to return an `size_t` instead of an `int`. But given that this commit may be part of a security release, let's instead do the minimal viable fix and die in case we see an overflow. Add a test that would have previously caused us to crash. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Memory sanitizer (msan) is detecting a use of an uninitialized variable (`size`) in `read_attr_from_index`: ==2268==WARNING: MemorySanitizer: use-of-uninitialized-value #0 0x5651f3416504 in read_attr_from_index git/attr.c:868:11 #1 0x5651f3415530 in read_attr git/attr.c #2 0x5651f3413d74 in bootstrap_attr_stack git/attr.c:968:6 #3 0x5651f3413d74 in prepare_attr_stack git/attr.c:1004:2 #4 0x5651f3413d74 in collect_some_attrs git/attr.c:1199:2 #5 0x5651f3413144 in git_check_attr git/attr.c:1345:2 git#6 0x5651f34728da in convert_attrs git/convert.c:1320:2 #7 0x5651f3473425 in would_convert_to_git_filter_fd git/convert.c:1373:2 git#8 0x5651f357a35e in index_fd git/object-file.c:2630:34 git#9 0x5651f357aa15 in index_path git/object-file.c:2657:7 git#10 0x5651f35db9d9 in add_to_index git/read-cache.c:766:7 git#11 0x5651f35dc170 in add_file_to_index git/read-cache.c:799:9 git#12 0x5651f321f9b2 in add_files git/builtin/add.c:346:7 git#13 0x5651f321f9b2 in cmd_add git/builtin/add.c:565:18 git#14 0x5651f321d327 in run_builtin git/git.c:474:11 git#15 0x5651f321bc9e in handle_builtin git/git.c:729:3 git#16 0x5651f321a792 in run_argv git/git.c:793:4 git#17 0x5651f321a792 in cmd_main git/git.c:928:19 git#18 0x5651f33dde1f in main git/common-main.c:62:11 The issue exists because `size` is an output parameter from `read_blob_data_from_index`, but it's only modified if `read_blob_data_from_index` returns non-NULL. The read of `size` when calling `read_attr_from_buf` unconditionally may read from an uninitialized value. `read_attr_from_buf` checks that `buf` is non-NULL before reading from `size`, but by then it's already too late: the uninitialized read will have happened already. Furthermore, there's no guarantee that the compiler won't reorder things so that it checks `size` before checking `!buf`. Make the call to `read_attr_from_buf` conditional on `buf` being non-NULL, ensuring that `size` is not read if it's never set. Signed-off-by: Kyle Lippincott <spectral@google.com>

Memory sanitizer (msan) is detecting a use of an uninitialized variable (`size`) in `read_attr_from_index`: ==2268==WARNING: MemorySanitizer: use-of-uninitialized-value #0 0x5651f3416504 in read_attr_from_index git/attr.c:868:11 #1 0x5651f3415530 in read_attr git/attr.c #2 0x5651f3413d74 in bootstrap_attr_stack git/attr.c:968:6 #3 0x5651f3413d74 in prepare_attr_stack git/attr.c:1004:2 #4 0x5651f3413d74 in collect_some_attrs git/attr.c:1199:2 #5 0x5651f3413144 in git_check_attr git/attr.c:1345:2 #6 0x5651f34728da in convert_attrs git/convert.c:1320:2 #7 0x5651f3473425 in would_convert_to_git_filter_fd git/convert.c:1373:2 #8 0x5651f357a35e in index_fd git/object-file.c:2630:34 #9 0x5651f357aa15 in index_path git/object-file.c:2657:7 #10 0x5651f35db9d9 in add_to_index git/read-cache.c:766:7 #11 0x5651f35dc170 in add_file_to_index git/read-cache.c:799:9 #12 0x5651f321f9b2 in add_files git/builtin/add.c:346:7 #13 0x5651f321f9b2 in cmd_add git/builtin/add.c:565:18 #14 0x5651f321d327 in run_builtin git/git.c:474:11 #15 0x5651f321bc9e in handle_builtin git/git.c:729:3 #16 0x5651f321a792 in run_argv git/git.c:793:4 #17 0x5651f321a792 in cmd_main git/git.c:928:19 #18 0x5651f33dde1f in main git/common-main.c:62:11 The issue exists because `size` is an output parameter from `read_blob_data_from_index`, but it's only modified if `read_blob_data_from_index` returns non-NULL. The read of `size` when calling `read_attr_from_buf` unconditionally may read from an uninitialized value. `read_attr_from_buf` checks that `buf` is non-NULL before reading from `size`, but by then it's already too late: the uninitialized read will have happened already. Furthermore, there's no guarantee that the compiler won't reorder things so that it checks `size` before checking `!buf`. Make the call to `read_attr_from_buf` conditional on `buf` being non-NULL, ensuring that `size` is not read if it's never set. Signed-off-by: Kyle Lippincott <spectral@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

It was recently reported that concurrent reads and writes may cause the reftable backend to segfault. The root cause of this is that we do not properly keep track of reftable readers across reloads. Suppose that you have a reftable iterator and then decide to reload the stack while iterating through the iterator. When the stack has been rewritten since we have created the iterator, then we would end up discarding a subset of readers that may still be in use by the iterator. The consequence is that we now try to reference deallocated memory, which of course segfaults. One way to trigger this is in t5616, where some background maintenance jobs have been leaking from one test into another. This leads to stack traces like the following one: + git -c protocol.version=0 -C pc1 fetch --filter=blob:limit=29999 --refetch origin AddressSanitizer:DEADLYSIGNAL ================================================================= ==657994==ERROR: AddressSanitizer: SEGV on unknown address 0x7fa0f0ec6089 (pc 0x55f23e52ddf9 bp 0x7ffe7bfa1700 sp 0x7ffe7bfa1700 T0) ==657994==The signal is caused by a READ memory access. #0 0x55f23e52ddf9 in get_var_int reftable/record.c:29 #1 0x55f23e53295e in reftable_decode_keylen reftable/record.c:170 #2 0x55f23e532cc0 in reftable_decode_key reftable/record.c:194 #3 0x55f23e54e72e in block_iter_next reftable/block.c:398 #4 0x55f23e5573dc in table_iter_next_in_block reftable/reader.c:240 #5 0x55f23e5573dc in table_iter_next reftable/reader.c:355 #6 0x55f23e5573dc in table_iter_next reftable/reader.c:339 #7 0x55f23e551283 in merged_iter_advance_subiter reftable/merged.c:69 #8 0x55f23e55169e in merged_iter_next_entry reftable/merged.c:123 #9 0x55f23e55169e in merged_iter_next_void reftable/merged.c:172 #10 0x55f23e537625 in reftable_iterator_next_ref reftable/generic.c:175 #11 0x55f23e2cf9c6 in reftable_ref_iterator_advance refs/reftable-backend.c:464 #12 0x55f23e2d996e in ref_iterator_advance refs/iterator.c:13 #13 0x55f23e2d996e in do_for_each_ref_iterator refs/iterator.c:452 #14 0x55f23dca6767 in get_ref_map builtin/fetch.c:623 #15 0x55f23dca6767 in do_fetch builtin/fetch.c:1659 #16 0x55f23dca6767 in fetch_one builtin/fetch.c:2133 #17 0x55f23dca6767 in cmd_fetch builtin/fetch.c:2432 #18 0x55f23dba7764 in run_builtin git.c:484 #19 0x55f23dba7764 in handle_builtin git.c:741 #20 0x55f23dbab61e in run_argv git.c:805 #21 0x55f23dbab61e in cmd_main git.c:1000 #22 0x55f23dba4781 in main common-main.c:64 #23 0x7fa0f063fc89 in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58 #24 0x7fa0f063fd44 in __libc_start_main_impl ../csu/libc-start.c:360 #25 0x55f23dba6ad0 in _start (git+0xadfad0) (BuildId: 803b2b7f59beb03d7849fb8294a8e2145dd4aa27) While it is somewhat awkward that the maintenance processes survive tests in the first place, it is totally expected that reftables should work alright with concurrent writers. Seemingly they don't. The only underlying resource that we need to care about in this context is the reftable reader, which is responsible for reading a single table from disk. These readers get discarded immediately (unless reused) when calling `reftable_stack_reload()`, which is wrong. We can only close them once we know that there are no iterators using them anymore. Prepare for a fix by converting the reftable readers to be refcounted. Reported-by: Jeff King <peff@peff.net> Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>

In 1b9e9be (csum-file.c: use unsafe SHA-1 implementation when available, 2024-09-26) we have converted our `struct hashfile` to use the unsafe SHA1 backend, which results in a significant speedup. One needs to be careful with how to use that structure now though because callers need to consistently use either the safe or unsafe variants of SHA1, as otherwise one can easily trigger corruption. As it turns out, we have one inconsistent usage in our tree because we directly initialize `struct hashfile_checkpoint::ctx` with the safe variant of SHA1, but end up writing to that context with the unsafe ones. This went unnoticed so far because our CI systems do not exercise different hash functions for these two backends, and consequently safe and unsafe variants are equivalent. But when using SHA1DC as safe and OpenSSL as unsafe backend this leads to a crash an t1050: ++ git -c core.compression=0 add large1 AddressSanitizer:DEADLYSIGNAL ================================================================= ==1367==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000040 (pc 0x7ffff7a01a99 bp 0x507000000db0 sp 0x7fffffff5690 T0) ==1367==The signal is caused by a READ memory access. ==1367==Hint: address points to the zero page. #0 0x7ffff7a01a99 in EVP_MD_CTX_copy_ex (/nix/store/h1ydpxkw9qhjdxjpic1pdc2nirggyy6f-openssl-3.3.2/lib/libcrypto.so.3+0x201a99) (BuildId: 41746a580d39075fc85e8c8065b6c07fb34e97d4) #1 0x555555ddde56 in openssl_SHA1_Clone ../sha1/openssl.h:40:2 #2 0x555555dce2fc in git_hash_sha1_clone_unsafe ../object-file.c:123:2 #3 0x555555c2d5f8 in hashfile_checkpoint ../csum-file.c:211:2 #4 0x555555b9905d in deflate_blob_to_pack ../bulk-checkin.c:286:4 #5 0x555555b98ae9 in index_blob_bulk_checkin ../bulk-checkin.c:362:15 #6 0x555555ddab62 in index_blob_stream ../object-file.c:2756:9 #7 0x555555dda420 in index_fd ../object-file.c:2778:9 #8 0x555555ddad76 in index_path ../object-file.c:2796:7 #9 0x555555e947f3 in add_to_index ../read-cache.c:771:7 #10 0x555555e954a4 in add_file_to_index ../read-cache.c:804:9 #11 0x5555558b5c39 in add_files ../builtin/add.c:355:7 #12 0x5555558b412e in cmd_add ../builtin/add.c:578:18 #13 0x555555b1f493 in run_builtin ../git.c:480:11 #14 0x555555b1bfef in handle_builtin ../git.c:740:9 #15 0x555555b1e6f4 in run_argv ../git.c:807:4 #16 0x555555b1b87a in cmd_main ../git.c:947:19 #17 0x5555561649e6 in main ../common-main.c:64:11 #18 0x7ffff742a1fb in __libc_start_call_main (/nix/store/65h17wjrrlsj2rj540igylrx7fqcd6vq-glibc-2.40-36/lib/libc.so.6+0x2a1fb) (BuildId: bf320110569c8ec2425e9a0c5e4eb7e97f1fb6e4) #19 0x7ffff742a2b8 in __libc_start_main@GLIBC_2.2.5 (/nix/store/65h17wjrrlsj2rj540igylrx7fqcd6vq-glibc-2.40-36/lib/libc.so.6+0x2a2b8) (BuildId: bf320110569c8ec2425e9a0c5e4eb7e97f1fb6e4) #20 0x555555772c84 in _start (git+0x21ec84) ==1367==Register values: rax = 0x0000511000001080 rbx = 0x0000000000000000 rcx = 0x000000000000000c rdx = 0x0000000000000000 rdi = 0x0000000000000000 rsi = 0x0000507000000db0 rbp = 0x0000507000000db0 rsp = 0x00007fffffff5690 r8 = 0x0000000000000000 r9 = 0x0000000000000000 r10 = 0x0000000000000000 r11 = 0x00007ffff7a01a30 r12 = 0x0000000000000000 r13 = 0x00007fffffff6b38 r14 = 0x00007ffff7ffd000 r15 = 0x00005555563b9910 AddressSanitizer can not provide additional info. SUMMARY: AddressSanitizer: SEGV (/nix/store/h1ydpxkw9qhjdxjpic1pdc2nirggyy6f-openssl-3.3.2/lib/libcrypto.so.3+0x201a99) (BuildId: 41746a580d39075fc85e8c8065b6c07fb34e97d4) in EVP_MD_CTX_copy_ex ==1367==ABORTING ./test-lib.sh: line 1023: 1367 Aborted git $config add large1 error: last command exited with $?=134 not ok 4 - add with -c core.compression=0 Fix the issue by using the unsafe variant instead. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Cater for cases where upstream path is one level lower than the root …

a8b8cfb

…SVN URL: e.g. https://svn.project.com/svn/Repo/one_level_down/trunk

peff closed this May 11, 2012

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Cater for cases where upstream path is one level lower than the root SVN... #18

Cater for cases where upstream path is one level lower than the root SVN... #18

Uh oh!

chernjie commented Apr 21, 2012

Uh oh!

Uh oh!

Cater for cases where upstream path is one level lower than the root SVN... #18

Cater for cases where upstream path is one level lower than the root SVN... #18

Uh oh!

Conversation

chernjie commented Apr 21, 2012

Uh oh!

Uh oh!