-
Notifications
You must be signed in to change notification settings - Fork 26.4k
Update contrib/completion/git-completion.bash #17
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Closed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
well, at top of this repo written: "This is a publish-only repository and all pull requests are ignored. Please follow Documentation/SubmittingPatches procedure for any of your improvements." So I think your try to push your pull-request are hopeless. |
ahunt
added a commit
to ahunt/git
that referenced
this pull request
Mar 5, 2021
init_db_config_path could be leaked because: 1. git_init_db_config() allocates new data into init_db_config_path on every invocation without freeing preexisting data. 2. git_init_db_config() can be called multiple times for a single git_config() invocation (see docs on git_config() for context). Furthermore, until very recently, git_config(git_init_db_config(), ...) could have been invoked twice in a single process as git_init_db_config() used to be used to handle core.* config settings. This was changed in a previous patch in this series. Freeing the existing value in git_init_db_config() would be the least intrusive fix, however switching to git_config_get_value() simplifies the code further by letting us remove the static pointer (furthermore, the returned data is owned by the config cache, saving us from having to worry about freeing it later). LSAN output from t0001: Direct leak of 73 byte(s) in 1 object(s) allocated from: #0 0x49a859 in realloc /home/abuild/rpmbuild/BUILD/llvm-11.0.0.src/build/../projects/compiler-rt/lib/asan/asan_malloc_linux.cpp:164:3 #1 0x9a7276 in xrealloc /home/ahunt/oss-fuzz/git/wrapper.c:126:8 #2 0x9362ad in strbuf_grow /home/ahunt/oss-fuzz/git/strbuf.c:98:2 #3 0x936eaa in strbuf_add /home/ahunt/oss-fuzz/git/strbuf.c:295:2 #4 0x868112 in strbuf_addstr /home/ahunt/oss-fuzz/git/./strbuf.h:304:2 #5 0x86a8ad in expand_user_path /home/ahunt/oss-fuzz/git/path.c:758:2 #6 0x720bb1 in git_config_pathname /home/ahunt/oss-fuzz/git/config.c:1287:10 #7 0x5960e2 in git_init_db_config /home/ahunt/oss-fuzz/git/builtin/init-db.c:161:11 git#8 0x7255b8 in configset_iter /home/ahunt/oss-fuzz/git/config.c:1982:7 git#9 0x7253fc in repo_config /home/ahunt/oss-fuzz/git/config.c:2311:2 git#10 0x725ca7 in git_config /home/ahunt/oss-fuzz/git/config.c:2399:2 git#11 0x593e8d in create_default_files /home/ahunt/oss-fuzz/git/builtin/init-db.c:225:2 git#12 0x5935c6 in init_db /home/ahunt/oss-fuzz/git/builtin/init-db.c:449:11 git#13 0x59588e in cmd_init_db /home/ahunt/oss-fuzz/git/builtin/init-db.c:714:9 git#14 0x4cd60d in run_builtin /home/ahunt/oss-fuzz/git/git.c:453:11 git#15 0x4cb2da in handle_builtin /home/ahunt/oss-fuzz/git/git.c:704:3 git#16 0x4ccc37 in run_argv /home/ahunt/oss-fuzz/git/git.c:771:4 git#17 0x4cac29 in cmd_main /home/ahunt/oss-fuzz/git/git.c:902:19 git#18 0x69c4de in main /home/ahunt/oss-fuzz/git/common-main.c:52:11 git#19 0x7f23552d6349 in __libc_start_main (/lib64/libc.so.6+0x24349)
ahunt
added a commit
to ahunt/git
that referenced
this pull request
Mar 5, 2021
init_db_config_path could be leaked because: 1. git_init_db_config() allocates new data into init_db_config_path on every invocation without freeing preexisting data. 2. git_init_db_config() can be called multiple times for a single git_config() invocation (see docs on git_config() for context). Furthermore, until very recently, git_config(git_init_db_config(), ...) could have been invoked twice in a single process as git_init_db_config() used to be used to handle core.* config settings. This was changed in a previous patch in this series. Freeing the existing value in git_init_db_config() would be the least intrusive fix, however switching to git_config_get_value() simplifies the code further by letting us remove the static pointer (furthermore, the returned data is owned by the config cache, saving us from having to worry about freeing it later). LSAN output from t0001: Direct leak of 73 byte(s) in 1 object(s) allocated from: #0 0x49a859 in realloc /home/abuild/rpmbuild/BUILD/llvm-11.0.0.src/build/../projects/compiler-rt/lib/asan/asan_malloc_linux.cpp:164:3 #1 0x9a7276 in xrealloc /home/ahunt/oss-fuzz/git/wrapper.c:126:8 #2 0x9362ad in strbuf_grow /home/ahunt/oss-fuzz/git/strbuf.c:98:2 #3 0x936eaa in strbuf_add /home/ahunt/oss-fuzz/git/strbuf.c:295:2 #4 0x868112 in strbuf_addstr /home/ahunt/oss-fuzz/git/./strbuf.h:304:2 #5 0x86a8ad in expand_user_path /home/ahunt/oss-fuzz/git/path.c:758:2 #6 0x720bb1 in git_config_pathname /home/ahunt/oss-fuzz/git/config.c:1287:10 #7 0x5960e2 in git_init_db_config /home/ahunt/oss-fuzz/git/builtin/init-db.c:161:11 git#8 0x7255b8 in configset_iter /home/ahunt/oss-fuzz/git/config.c:1982:7 git#9 0x7253fc in repo_config /home/ahunt/oss-fuzz/git/config.c:2311:2 git#10 0x725ca7 in git_config /home/ahunt/oss-fuzz/git/config.c:2399:2 git#11 0x593e8d in create_default_files /home/ahunt/oss-fuzz/git/builtin/init-db.c:225:2 git#12 0x5935c6 in init_db /home/ahunt/oss-fuzz/git/builtin/init-db.c:449:11 git#13 0x59588e in cmd_init_db /home/ahunt/oss-fuzz/git/builtin/init-db.c:714:9 git#14 0x4cd60d in run_builtin /home/ahunt/oss-fuzz/git/git.c:453:11 git#15 0x4cb2da in handle_builtin /home/ahunt/oss-fuzz/git/git.c:704:3 git#16 0x4ccc37 in run_argv /home/ahunt/oss-fuzz/git/git.c:771:4 git#17 0x4cac29 in cmd_main /home/ahunt/oss-fuzz/git/git.c:902:19 git#18 0x69c4de in main /home/ahunt/oss-fuzz/git/common-main.c:52:11 git#19 0x7f23552d6349 in __libc_start_main (/lib64/libc.so.6+0x24349)
ahunt
added a commit
to ahunt/git
that referenced
this pull request
Mar 5, 2021
init_db_config_path could be leaked because: 1. git_init_db_config() allocates new data into init_db_config_path on every invocation without freeing preexisting data. 2. git_init_db_config() can be called multiple times for a single git_config() invocation (see docs on git_config() for context). Furthermore, until very recently, git_config(git_init_db_config(), ...) could have been invoked twice in a single process as git_init_db_config() used to be used to handle core.* config settings. This was changed in a previous patch in this series. Freeing the existing value in git_init_db_config() would be the least intrusive fix, however switching to git_config_get_value() simplifies the code further by letting us remove the static pointer (furthermore, the returned data is owned by the config cache, saving us from having to worry about freeing it later). LSAN output from t0001: Direct leak of 73 byte(s) in 1 object(s) allocated from: #0 0x49a859 in realloc /home/abuild/rpmbuild/BUILD/llvm-11.0.0.src/build/../projects/compiler-rt/lib/asan/asan_malloc_linux.cpp:164:3 #1 0x9a7276 in xrealloc /home/ahunt/oss-fuzz/git/wrapper.c:126:8 #2 0x9362ad in strbuf_grow /home/ahunt/oss-fuzz/git/strbuf.c:98:2 #3 0x936eaa in strbuf_add /home/ahunt/oss-fuzz/git/strbuf.c:295:2 #4 0x868112 in strbuf_addstr /home/ahunt/oss-fuzz/git/./strbuf.h:304:2 #5 0x86a8ad in expand_user_path /home/ahunt/oss-fuzz/git/path.c:758:2 #6 0x720bb1 in git_config_pathname /home/ahunt/oss-fuzz/git/config.c:1287:10 #7 0x5960e2 in git_init_db_config /home/ahunt/oss-fuzz/git/builtin/init-db.c:161:11 git#8 0x7255b8 in configset_iter /home/ahunt/oss-fuzz/git/config.c:1982:7 git#9 0x7253fc in repo_config /home/ahunt/oss-fuzz/git/config.c:2311:2 git#10 0x725ca7 in git_config /home/ahunt/oss-fuzz/git/config.c:2399:2 git#11 0x593e8d in create_default_files /home/ahunt/oss-fuzz/git/builtin/init-db.c:225:2 git#12 0x5935c6 in init_db /home/ahunt/oss-fuzz/git/builtin/init-db.c:449:11 git#13 0x59588e in cmd_init_db /home/ahunt/oss-fuzz/git/builtin/init-db.c:714:9 git#14 0x4cd60d in run_builtin /home/ahunt/oss-fuzz/git/git.c:453:11 git#15 0x4cb2da in handle_builtin /home/ahunt/oss-fuzz/git/git.c:704:3 git#16 0x4ccc37 in run_argv /home/ahunt/oss-fuzz/git/git.c:771:4 git#17 0x4cac29 in cmd_main /home/ahunt/oss-fuzz/git/git.c:902:19 git#18 0x69c4de in main /home/ahunt/oss-fuzz/git/common-main.c:52:11 git#19 0x7f23552d6349 in __libc_start_main (/lib64/libc.so.6+0x24349)
ahunt
added a commit
to ahunt/git
that referenced
this pull request
Mar 6, 2021
init_db_config_path could be leaked because: 1. git_init_db_config() allocates new data into init_db_config_path on every invocation without freeing preexisting data. 2. git_init_db_config() can be called multiple times for a single git_config() invocation (see docs on git_config() for context). Furthermore, until very recently, git_config(git_init_db_config(), ...) could have been invoked twice in a single process as git_init_db_config() used to be used to handle core.* config settings. This was changed in a previous patch in this series - that change alone might fix the leak in some scenarios, but is not enough to guarantee that we never leak. Freeing the existing value in git_init_db_config() would be the least intrusive fix, however switching to git_config_get_value() simplifies the code further by letting us remove the static pointer (furthermore, the returned data is owned by the config cache, saving us from having to worry about freeing it later). The following commit is when we first started using git_init_db_config() for 2 different purposes: 2878533 (mingw: respect core.hidedotfiles = false in git-init again, 2019-03-11) Although I suspect the potential for a leak existed since the original implementation in: 90b4518 (Add `init.templatedir` configuration variable., 2010-02-17) LSAN output from t0001: Direct leak of 73 byte(s) in 1 object(s) allocated from: #0 0x49a859 in realloc /home/abuild/rpmbuild/BUILD/llvm-11.0.0.src/build/../projects/compiler-rt/lib/asan/asan_malloc_linux.cpp:164:3 #1 0x9a7276 in xrealloc /home/ahunt/oss-fuzz/git/wrapper.c:126:8 #2 0x9362ad in strbuf_grow /home/ahunt/oss-fuzz/git/strbuf.c:98:2 #3 0x936eaa in strbuf_add /home/ahunt/oss-fuzz/git/strbuf.c:295:2 #4 0x868112 in strbuf_addstr /home/ahunt/oss-fuzz/git/./strbuf.h:304:2 #5 0x86a8ad in expand_user_path /home/ahunt/oss-fuzz/git/path.c:758:2 #6 0x720bb1 in git_config_pathname /home/ahunt/oss-fuzz/git/config.c:1287:10 #7 0x5960e2 in git_init_db_config /home/ahunt/oss-fuzz/git/builtin/init-db.c:161:11 git#8 0x7255b8 in configset_iter /home/ahunt/oss-fuzz/git/config.c:1982:7 git#9 0x7253fc in repo_config /home/ahunt/oss-fuzz/git/config.c:2311:2 git#10 0x725ca7 in git_config /home/ahunt/oss-fuzz/git/config.c:2399:2 git#11 0x593e8d in create_default_files /home/ahunt/oss-fuzz/git/builtin/init-db.c:225:2 git#12 0x5935c6 in init_db /home/ahunt/oss-fuzz/git/builtin/init-db.c:449:11 git#13 0x59588e in cmd_init_db /home/ahunt/oss-fuzz/git/builtin/init-db.c:714:9 git#14 0x4cd60d in run_builtin /home/ahunt/oss-fuzz/git/git.c:453:11 git#15 0x4cb2da in handle_builtin /home/ahunt/oss-fuzz/git/git.c:704:3 git#16 0x4ccc37 in run_argv /home/ahunt/oss-fuzz/git/git.c:771:4 git#17 0x4cac29 in cmd_main /home/ahunt/oss-fuzz/git/git.c:902:19 git#18 0x69c4de in main /home/ahunt/oss-fuzz/git/common-main.c:52:11 git#19 0x7f23552d6349 in __libc_start_main (/lib64/libc.so.6+0x24349)
ahunt
added a commit
to ahunt/git
that referenced
this pull request
Mar 6, 2021
init_db_config_path could be leaked because: 1. git_init_db_config() allocates new data into init_db_config_path on every invocation without freeing preexisting data. 2. git_init_db_config() can be called multiple times for a single git_config() invocation (see docs on git_config() for context). Furthermore, until very recently, git_config(git_init_db_config(), ...) could have been invoked twice in a single process as git_init_db_config() used to be used to handle core.* config settings. This was changed in a previous patch in this series - that change alone might fix the leak in some scenarios, but is not enough to guarantee that we never leak. Freeing the existing value in git_init_db_config() would be the least intrusive fix, however switching to git_config_get_value() simplifies the code further by letting us remove the static pointer (furthermore, the returned data is owned by the config cache, saving us from having to worry about freeing it later). The following commit is when we first started using git_init_db_config() for 2 different purposes: 2878533 (mingw: respect core.hidedotfiles = false in git-init again, 2019-03-11) Although I suspect the potential for a leak existed since the original implementation in: 90b4518 (Add `init.templatedir` configuration variable., 2010-02-17) LSAN output from t0001: Direct leak of 73 byte(s) in 1 object(s) allocated from: #0 0x49a859 in realloc /home/abuild/rpmbuild/BUILD/llvm-11.0.0.src/build/../projects/compiler-rt/lib/asan/asan_malloc_linux.cpp:164:3 #1 0x9a7276 in xrealloc /home/ahunt/oss-fuzz/git/wrapper.c:126:8 #2 0x9362ad in strbuf_grow /home/ahunt/oss-fuzz/git/strbuf.c:98:2 #3 0x936eaa in strbuf_add /home/ahunt/oss-fuzz/git/strbuf.c:295:2 #4 0x868112 in strbuf_addstr /home/ahunt/oss-fuzz/git/./strbuf.h:304:2 #5 0x86a8ad in expand_user_path /home/ahunt/oss-fuzz/git/path.c:758:2 #6 0x720bb1 in git_config_pathname /home/ahunt/oss-fuzz/git/config.c:1287:10 #7 0x5960e2 in git_init_db_config /home/ahunt/oss-fuzz/git/builtin/init-db.c:161:11 git#8 0x7255b8 in configset_iter /home/ahunt/oss-fuzz/git/config.c:1982:7 git#9 0x7253fc in repo_config /home/ahunt/oss-fuzz/git/config.c:2311:2 git#10 0x725ca7 in git_config /home/ahunt/oss-fuzz/git/config.c:2399:2 git#11 0x593e8d in create_default_files /home/ahunt/oss-fuzz/git/builtin/init-db.c:225:2 git#12 0x5935c6 in init_db /home/ahunt/oss-fuzz/git/builtin/init-db.c:449:11 git#13 0x59588e in cmd_init_db /home/ahunt/oss-fuzz/git/builtin/init-db.c:714:9 git#14 0x4cd60d in run_builtin /home/ahunt/oss-fuzz/git/git.c:453:11 git#15 0x4cb2da in handle_builtin /home/ahunt/oss-fuzz/git/git.c:704:3 git#16 0x4ccc37 in run_argv /home/ahunt/oss-fuzz/git/git.c:771:4 git#17 0x4cac29 in cmd_main /home/ahunt/oss-fuzz/git/git.c:902:19 git#18 0x69c4de in main /home/ahunt/oss-fuzz/git/common-main.c:52:11 git#19 0x7f23552d6349 in __libc_start_main (/lib64/libc.so.6+0x24349)
ahunt
added a commit
to ahunt/git
that referenced
this pull request
Mar 6, 2021
Also UNLEAK template_dir in cmd_init_db(), as we are passing it into init_db() and cannot easily free it without making more intrusive changes to execution flow. init_db_config_path could be leaked because: 1. git_init_db_config() allocates new data into init_db_config_path on every invocation without freeing preexisting data. 2. git_init_db_config() can be called multiple times for a single git_config() invocation (see docs on git_config() for context). Furthermore, until very recently, git_config(git_init_db_config(), ...) could have been invoked twice in a single process as git_init_db_config() used to be used to handle core.* config settings. This was changed in a previous patch in this series - that change alone might fix the leak in some scenarios, but is not enough to guarantee that we never leak. Freeing the existing value in git_init_db_config() would be the least intrusive fix, however switching to git_config_get_value() simplifies the code further by letting us remove the static pointer (furthermore, the returned data is owned by the config cache, saving us from having to worry about freeing it later). The following commit is when we first started using git_init_db_config() for 2 different purposes: 2878533 (mingw: respect core.hidedotfiles = false in git-init again, 2019-03-11) Although I suspect the potential for a leak existed since the original implementation in: 90b4518 (Add `init.templatedir` configuration variable., 2010-02-17) LSAN output from t0001: Direct leak of 73 byte(s) in 1 object(s) allocated from: #0 0x49a859 in realloc /home/abuild/rpmbuild/BUILD/llvm-11.0.0.src/build/../projects/compiler-rt/lib/asan/asan_malloc_linux.cpp:164:3 #1 0x9a7276 in xrealloc /home/ahunt/oss-fuzz/git/wrapper.c:126:8 #2 0x9362ad in strbuf_grow /home/ahunt/oss-fuzz/git/strbuf.c:98:2 #3 0x936eaa in strbuf_add /home/ahunt/oss-fuzz/git/strbuf.c:295:2 #4 0x868112 in strbuf_addstr /home/ahunt/oss-fuzz/git/./strbuf.h:304:2 #5 0x86a8ad in expand_user_path /home/ahunt/oss-fuzz/git/path.c:758:2 #6 0x720bb1 in git_config_pathname /home/ahunt/oss-fuzz/git/config.c:1287:10 #7 0x5960e2 in git_init_db_config /home/ahunt/oss-fuzz/git/builtin/init-db.c:161:11 git#8 0x7255b8 in configset_iter /home/ahunt/oss-fuzz/git/config.c:1982:7 git#9 0x7253fc in repo_config /home/ahunt/oss-fuzz/git/config.c:2311:2 git#10 0x725ca7 in git_config /home/ahunt/oss-fuzz/git/config.c:2399:2 git#11 0x593e8d in create_default_files /home/ahunt/oss-fuzz/git/builtin/init-db.c:225:2 git#12 0x5935c6 in init_db /home/ahunt/oss-fuzz/git/builtin/init-db.c:449:11 git#13 0x59588e in cmd_init_db /home/ahunt/oss-fuzz/git/builtin/init-db.c:714:9 git#14 0x4cd60d in run_builtin /home/ahunt/oss-fuzz/git/git.c:453:11 git#15 0x4cb2da in handle_builtin /home/ahunt/oss-fuzz/git/git.c:704:3 git#16 0x4ccc37 in run_argv /home/ahunt/oss-fuzz/git/git.c:771:4 git#17 0x4cac29 in cmd_main /home/ahunt/oss-fuzz/git/git.c:902:19 git#18 0x69c4de in main /home/ahunt/oss-fuzz/git/common-main.c:52:11 git#19 0x7f23552d6349 in __libc_start_main (/lib64/libc.so.6+0x24349)
ahunt
added a commit
to ahunt/git
that referenced
this pull request
Mar 6, 2021
Also UNLEAK template_dir in cmd_init_db(), as we are passing it into init_db() and cannot easily free it without making more intrusive changes to execution flow. init_db_config_path could be leaked because: 1. git_init_db_config() allocates new data into init_db_config_path on every invocation without freeing preexisting data. 2. git_init_db_config() can be called multiple times for a single git_config() invocation (see docs on git_config() for context). Furthermore, until very recently, git_config(git_init_db_config(), ...) could have been invoked twice in a single process as git_init_db_config() used to be used to handle core.* config settings. This was changed in a previous patch in this series - that change alone might fix the leak in some scenarios, but is not enough to guarantee that we never leak. Freeing the existing value in git_init_db_config() would be the least intrusive fix, however switching to git_config_get_value() simplifies the code further by letting us remove the static pointer (furthermore, the returned data is owned by the config cache, saving us from having to worry about freeing it later). The following commit is when we first started using git_init_db_config() for 2 different purposes: 2878533 (mingw: respect core.hidedotfiles = false in git-init again, 2019-03-11) Although I suspect the potential for a leak existed since the original implementation in: 90b4518 (Add `init.templatedir` configuration variable., 2010-02-17) LSAN output from t0001: Direct leak of 73 byte(s) in 1 object(s) allocated from: #0 0x49a859 in realloc /home/abuild/rpmbuild/BUILD/llvm-11.0.0.src/build/../projects/compiler-rt/lib/asan/asan_malloc_linux.cpp:164:3 #1 0x9a7276 in xrealloc /home/ahunt/oss-fuzz/git/wrapper.c:126:8 #2 0x9362ad in strbuf_grow /home/ahunt/oss-fuzz/git/strbuf.c:98:2 #3 0x936eaa in strbuf_add /home/ahunt/oss-fuzz/git/strbuf.c:295:2 #4 0x868112 in strbuf_addstr /home/ahunt/oss-fuzz/git/./strbuf.h:304:2 #5 0x86a8ad in expand_user_path /home/ahunt/oss-fuzz/git/path.c:758:2 #6 0x720bb1 in git_config_pathname /home/ahunt/oss-fuzz/git/config.c:1287:10 #7 0x5960e2 in git_init_db_config /home/ahunt/oss-fuzz/git/builtin/init-db.c:161:11 git#8 0x7255b8 in configset_iter /home/ahunt/oss-fuzz/git/config.c:1982:7 git#9 0x7253fc in repo_config /home/ahunt/oss-fuzz/git/config.c:2311:2 git#10 0x725ca7 in git_config /home/ahunt/oss-fuzz/git/config.c:2399:2 git#11 0x593e8d in create_default_files /home/ahunt/oss-fuzz/git/builtin/init-db.c:225:2 git#12 0x5935c6 in init_db /home/ahunt/oss-fuzz/git/builtin/init-db.c:449:11 git#13 0x59588e in cmd_init_db /home/ahunt/oss-fuzz/git/builtin/init-db.c:714:9 git#14 0x4cd60d in run_builtin /home/ahunt/oss-fuzz/git/git.c:453:11 git#15 0x4cb2da in handle_builtin /home/ahunt/oss-fuzz/git/git.c:704:3 git#16 0x4ccc37 in run_argv /home/ahunt/oss-fuzz/git/git.c:771:4 git#17 0x4cac29 in cmd_main /home/ahunt/oss-fuzz/git/git.c:902:19 git#18 0x69c4de in main /home/ahunt/oss-fuzz/git/common-main.c:52:11 git#19 0x7f23552d6349 in __libc_start_main (/lib64/libc.so.6+0x24349) Signed-off-by: Andrzej Hunt <ajrhunt@google.com>
ahunt
added a commit
to ahunt/git
that referenced
this pull request
Mar 8, 2021
The primary goal of this change is to stop leaking init_db_template_dir. This leak can happen because: 1. git_init_db_config() allocates new memory into init_db_template_dir without first freeing. 2. init_db_template_dir might already contain data, either because: 2.1 git_config() can be invoked twice with this callback in a single process - at least 2 allocations are likely. 2.2 A single git_config() allocation can invoke the callback multiple times for a given key (see further explanation in the function docs) - each of those calls will trigger another leak. The simplest fix for the leak would be to free(init_db_template_dir) before overwriting it. Instead we convert to git_config_get_value() as that is more explicit, seemingly more efficient, AND avoids allocations (the returned result is owned by the config cache, so we aren't responsible for freeing it). By removing init_db_template_dir, git_init_db_config() ends up only being responsible for forwarding core.* config values to platform_core_config(). However platform_core_config() already ignores non-core.* config values, so we can safely remove git_init_db_config() and invoke git_config() directly on platform_core_config(). In addition to those changes, we squash another leak by UNLEAK'ing template_dir in cmd_init_db(). We are already passing it into init_db() and cannot easily free it without making more intrusive changes to execution flow. The platform_core_config forwarding was originally added in: 2878533 (mingw: respect core.hidedotfiles = false in git-init again, 2019-03-11 I suspect the potential for a leak existed since the original implementation of git_init_db_config in: 90b4518 (Add `init.templatedir` configuration variable., 2010-02-17) LSAN output from t0001: Direct leak of 73 byte(s) in 1 object(s) allocated from: #0 0x49a859 in realloc /home/abuild/rpmbuild/BUILD/llvm-11.0.0.src/build/../projects/compiler-rt/lib/asan/asan_malloc_linux.cpp:164:3 #1 0x9a7276 in xrealloc /home/ahunt/oss-fuzz/git/wrapper.c:126:8 #2 0x9362ad in strbuf_grow /home/ahunt/oss-fuzz/git/strbuf.c:98:2 #3 0x936eaa in strbuf_add /home/ahunt/oss-fuzz/git/strbuf.c:295:2 #4 0x868112 in strbuf_addstr /home/ahunt/oss-fuzz/git/./strbuf.h:304:2 #5 0x86a8ad in expand_user_path /home/ahunt/oss-fuzz/git/path.c:758:2 #6 0x720bb1 in git_config_pathname /home/ahunt/oss-fuzz/git/config.c:1287:10 #7 0x5960e2 in git_init_db_config /home/ahunt/oss-fuzz/git/builtin/init-db.c:161:11 git#8 0x7255b8 in configset_iter /home/ahunt/oss-fuzz/git/config.c:1982:7 git#9 0x7253fc in repo_config /home/ahunt/oss-fuzz/git/config.c:2311:2 git#10 0x725ca7 in git_config /home/ahunt/oss-fuzz/git/config.c:2399:2 git#11 0x593e8d in create_default_files /home/ahunt/oss-fuzz/git/builtin/init-db.c:225:2 git#12 0x5935c6 in init_db /home/ahunt/oss-fuzz/git/builtin/init-db.c:449:11 git#13 0x59588e in cmd_init_db /home/ahunt/oss-fuzz/git/builtin/init-db.c:714:9 git#14 0x4cd60d in run_builtin /home/ahunt/oss-fuzz/git/git.c:453:11 git#15 0x4cb2da in handle_builtin /home/ahunt/oss-fuzz/git/git.c:704:3 git#16 0x4ccc37 in run_argv /home/ahunt/oss-fuzz/git/git.c:771:4 git#17 0x4cac29 in cmd_main /home/ahunt/oss-fuzz/git/git.c:902:19 git#18 0x69c4de in main /home/ahunt/oss-fuzz/git/common-main.c:52:11 git#19 0x7f23552d6349 in __libc_start_main (/lib64/libc.so.6+0x24349) Signed-off-by: Andrzej Hunt <ajrhunt@google.com>
ahunt
added a commit
to ahunt/git
that referenced
this pull request
Mar 8, 2021
The primary goal of this change is to stop leaking init_db_template_dir. This leak can happen because: 1. git_init_db_config() allocates new memory into init_db_template_dir without first freeing. 2. init_db_template_dir might already contain data, either because: 2.1 git_config() can be invoked twice with this callback in a single process - at least 2 allocations are likely. 2.2 A single git_config() allocation can invoke the callback multiple times for a given key (see further explanation in the function docs) - each of those calls will trigger another leak. The simplest fix for the leak would be to free(init_db_template_dir) before overwriting it. Instead we convert to git_config_get_value() as that is more explicit, seemingly more efficient, AND avoids allocations (the returned result is owned by the config cache, so we aren't responsible for freeing it). By removing init_db_template_dir, git_init_db_config() ends up only being responsible for forwarding core.* config values to platform_core_config(). However platform_core_config() already ignores non-core.* config values, so we can safely remove git_init_db_config() and invoke git_config() directly on platform_core_config(). The platform_core_config forwarding was originally added in: 2878533 (mingw: respect core.hidedotfiles = false in git-init again, 2019-03-11 I suspect the potential for a leak existed since the original implementation of git_init_db_config in: 90b4518 (Add `init.templatedir` configuration variable., 2010-02-17) LSAN output from t0001: Direct leak of 73 byte(s) in 1 object(s) allocated from: #0 0x49a859 in realloc /home/abuild/rpmbuild/BUILD/llvm-11.0.0.src/build/../projects/compiler-rt/lib/asan/asan_malloc_linux.cpp:164:3 #1 0x9a7276 in xrealloc /home/ahunt/oss-fuzz/git/wrapper.c:126:8 #2 0x9362ad in strbuf_grow /home/ahunt/oss-fuzz/git/strbuf.c:98:2 #3 0x936eaa in strbuf_add /home/ahunt/oss-fuzz/git/strbuf.c:295:2 #4 0x868112 in strbuf_addstr /home/ahunt/oss-fuzz/git/./strbuf.h:304:2 #5 0x86a8ad in expand_user_path /home/ahunt/oss-fuzz/git/path.c:758:2 #6 0x720bb1 in git_config_pathname /home/ahunt/oss-fuzz/git/config.c:1287:10 #7 0x5960e2 in git_init_db_config /home/ahunt/oss-fuzz/git/builtin/init-db.c:161:11 git#8 0x7255b8 in configset_iter /home/ahunt/oss-fuzz/git/config.c:1982:7 git#9 0x7253fc in repo_config /home/ahunt/oss-fuzz/git/config.c:2311:2 git#10 0x725ca7 in git_config /home/ahunt/oss-fuzz/git/config.c:2399:2 git#11 0x593e8d in create_default_files /home/ahunt/oss-fuzz/git/builtin/init-db.c:225:2 git#12 0x5935c6 in init_db /home/ahunt/oss-fuzz/git/builtin/init-db.c:449:11 git#13 0x59588e in cmd_init_db /home/ahunt/oss-fuzz/git/builtin/init-db.c:714:9 git#14 0x4cd60d in run_builtin /home/ahunt/oss-fuzz/git/git.c:453:11 git#15 0x4cb2da in handle_builtin /home/ahunt/oss-fuzz/git/git.c:704:3 git#16 0x4ccc37 in run_argv /home/ahunt/oss-fuzz/git/git.c:771:4 git#17 0x4cac29 in cmd_main /home/ahunt/oss-fuzz/git/git.c:902:19 git#18 0x69c4de in main /home/ahunt/oss-fuzz/git/common-main.c:52:11 git#19 0x7f23552d6349 in __libc_start_main (/lib64/libc.so.6+0x24349) Signed-off-by: Andrzej Hunt <ajrhunt@google.com>
ahunt
added a commit
to ahunt/git
that referenced
this pull request
Mar 8, 2021
The primary goal of this change is to stop leaking init_db_template_dir. This leak can happen because: 1. git_init_db_config() allocates new memory into init_db_template_dir without first freeing. 2. init_db_template_dir might already contain data, either because: 2.1 git_config() can be invoked twice with this callback in a single process - at least 2 allocations are likely. 2.2 A single git_config() allocation can invoke the callback multiple times for a given key (see further explanation in the function docs) - each of those calls will trigger another leak. The simplest fix for the leak would be to free(init_db_template_dir) before overwriting it. Instead we convert to git_config_get_value() as that is more explicit, seemingly more efficient, AND avoids allocations (the returned result is owned by the config cache, so we aren't responsible for freeing it). By removing init_db_template_dir, git_init_db_config() ends up only being responsible for forwarding core.* config values to platform_core_config(). However platform_core_config() already ignores non-core.* config values, so we can safely remove git_init_db_config() and invoke git_config() directly on platform_core_config(). The platform_core_config forwarding was originally added in: 2878533 (mingw: respect core.hidedotfiles = false in git-init again, 2019-03-11 I suspect the potential for a leak existed since the original implementation of git_init_db_config in: 90b4518 (Add `init.templatedir` configuration variable., 2010-02-17) LSAN output from t0001: Direct leak of 73 byte(s) in 1 object(s) allocated from: #0 0x49a859 in realloc /home/abuild/rpmbuild/BUILD/llvm-11.0.0.src/build/../projects/compiler-rt/lib/asan/asan_malloc_linux.cpp:164:3 #1 0x9a7276 in xrealloc /home/ahunt/oss-fuzz/git/wrapper.c:126:8 #2 0x9362ad in strbuf_grow /home/ahunt/oss-fuzz/git/strbuf.c:98:2 #3 0x936eaa in strbuf_add /home/ahunt/oss-fuzz/git/strbuf.c:295:2 #4 0x868112 in strbuf_addstr /home/ahunt/oss-fuzz/git/./strbuf.h:304:2 #5 0x86a8ad in expand_user_path /home/ahunt/oss-fuzz/git/path.c:758:2 #6 0x720bb1 in git_config_pathname /home/ahunt/oss-fuzz/git/config.c:1287:10 #7 0x5960e2 in git_init_db_config /home/ahunt/oss-fuzz/git/builtin/init-db.c:161:11 git#8 0x7255b8 in configset_iter /home/ahunt/oss-fuzz/git/config.c:1982:7 git#9 0x7253fc in repo_config /home/ahunt/oss-fuzz/git/config.c:2311:2 git#10 0x725ca7 in git_config /home/ahunt/oss-fuzz/git/config.c:2399:2 git#11 0x593e8d in create_default_files /home/ahunt/oss-fuzz/git/builtin/init-db.c:225:2 git#12 0x5935c6 in init_db /home/ahunt/oss-fuzz/git/builtin/init-db.c:449:11 git#13 0x59588e in cmd_init_db /home/ahunt/oss-fuzz/git/builtin/init-db.c:714:9 git#14 0x4cd60d in run_builtin /home/ahunt/oss-fuzz/git/git.c:453:11 git#15 0x4cb2da in handle_builtin /home/ahunt/oss-fuzz/git/git.c:704:3 git#16 0x4ccc37 in run_argv /home/ahunt/oss-fuzz/git/git.c:771:4 git#17 0x4cac29 in cmd_main /home/ahunt/oss-fuzz/git/git.c:902:19 git#18 0x69c4de in main /home/ahunt/oss-fuzz/git/common-main.c:52:11 git#19 0x7f23552d6349 in __libc_start_main (/lib64/libc.so.6+0x24349) Signed-off-by: Andrzej Hunt <ajrhunt@google.com>
ahunt
added a commit
to ahunt/git
that referenced
this pull request
Mar 8, 2021
The primary goal of this change is to stop leaking init_db_template_dir. This leak can happen because: 1. git_init_db_config() allocates new memory into init_db_template_dir without first freeing. 2. init_db_template_dir might already contain data, either because: 2.1 git_config() can be invoked twice with this callback in a single process - at least 2 allocations are likely. 2.2 A single git_config() allocation can invoke the callback multiple times for a given key (see further explanation in the function docs) - each of those calls will trigger another leak. The simplest fix for the leak would be to free(init_db_template_dir) before overwriting it. Instead we choose to convert to fetching init.templatedir via git_config_get_value() as that is more explicit, more efficient, and avoids allocations (the returned result is owned by the config cache, so we aren't responsible for freeing it). If we remove init_db_template_dir, git_init_db_config() ends up being responsible only for forwarding core.* config values to platform_core_config(). However platform_core_config() already ignores non-core.* config values, so we can safely remove git_init_db_config() and invoke git_config() directly with platform_core_config() as the callback. The platform_core_config forwarding was originally added in: 2878533 (mingw: respect core.hidedotfiles = false in git-init again, 2019-03-11 And I suspect the potential for a leak existed since the original implementation of git_init_db_config in: 90b4518 (Add `init.templatedir` configuration variable., 2010-02-17) LSAN output from t0001: Direct leak of 73 byte(s) in 1 object(s) allocated from: #0 0x49a859 in realloc /home/abuild/rpmbuild/BUILD/llvm-11.0.0.src/build/../projects/compiler-rt/lib/asan/asan_malloc_linux.cpp:164:3 #1 0x9a7276 in xrealloc /home/ahunt/oss-fuzz/git/wrapper.c:126:8 #2 0x9362ad in strbuf_grow /home/ahunt/oss-fuzz/git/strbuf.c:98:2 #3 0x936eaa in strbuf_add /home/ahunt/oss-fuzz/git/strbuf.c:295:2 #4 0x868112 in strbuf_addstr /home/ahunt/oss-fuzz/git/./strbuf.h:304:2 #5 0x86a8ad in expand_user_path /home/ahunt/oss-fuzz/git/path.c:758:2 #6 0x720bb1 in git_config_pathname /home/ahunt/oss-fuzz/git/config.c:1287:10 #7 0x5960e2 in git_init_db_config /home/ahunt/oss-fuzz/git/builtin/init-db.c:161:11 git#8 0x7255b8 in configset_iter /home/ahunt/oss-fuzz/git/config.c:1982:7 git#9 0x7253fc in repo_config /home/ahunt/oss-fuzz/git/config.c:2311:2 git#10 0x725ca7 in git_config /home/ahunt/oss-fuzz/git/config.c:2399:2 git#11 0x593e8d in create_default_files /home/ahunt/oss-fuzz/git/builtin/init-db.c:225:2 git#12 0x5935c6 in init_db /home/ahunt/oss-fuzz/git/builtin/init-db.c:449:11 git#13 0x59588e in cmd_init_db /home/ahunt/oss-fuzz/git/builtin/init-db.c:714:9 git#14 0x4cd60d in run_builtin /home/ahunt/oss-fuzz/git/git.c:453:11 git#15 0x4cb2da in handle_builtin /home/ahunt/oss-fuzz/git/git.c:704:3 git#16 0x4ccc37 in run_argv /home/ahunt/oss-fuzz/git/git.c:771:4 git#17 0x4cac29 in cmd_main /home/ahunt/oss-fuzz/git/git.c:902:19 git#18 0x69c4de in main /home/ahunt/oss-fuzz/git/common-main.c:52:11 git#19 0x7f23552d6349 in __libc_start_main (/lib64/libc.so.6+0x24349) Signed-off-by: Andrzej Hunt <ajrhunt@google.com>
ahunt
added a commit
to ahunt/git
that referenced
this pull request
Mar 8, 2021
The primary goal of this change is to stop leaking init_db_template_dir. This leak can happen because: 1. git_init_db_config() allocates new memory into init_db_template_dir without first freeing the existing value. 2. init_db_template_dir might already contain data, either because: 2.1 git_config() can be invoked twice with this callback in a single process - at least 2 allocations are likely. 2.2 A single git_config() allocation can invoke the callback multiple times for a given key (see further explanation in the function docs) - each of those calls will trigger another leak. The simplest fix for the leak would be to free(init_db_template_dir) before overwriting it. Instead we choose to convert to fetching init.templatedir via git_config_get_value() as that is more explicit, more efficient, and avoids allocations (the returned result is owned by the config cache, so we aren't responsible for freeing it). If we remove init_db_template_dir, git_init_db_config() ends up being responsible only for forwarding core.* config values to platform_core_config(). However platform_core_config() already ignores non-core.* config values, so we can safely remove git_init_db_config() and invoke git_config() directly with platform_core_config() as the callback. The platform_core_config forwarding was originally added in: 2878533 (mingw: respect core.hidedotfiles = false in git-init again, 2019-03-11 And I suspect the potential for a leak existed since the original implementation of git_init_db_config in: 90b4518 (Add `init.templatedir` configuration variable., 2010-02-17) LSAN output from t0001: Direct leak of 73 byte(s) in 1 object(s) allocated from: #0 0x49a859 in realloc /home/abuild/rpmbuild/BUILD/llvm-11.0.0.src/build/../projects/compiler-rt/lib/asan/asan_malloc_linux.cpp:164:3 #1 0x9a7276 in xrealloc /home/ahunt/oss-fuzz/git/wrapper.c:126:8 #2 0x9362ad in strbuf_grow /home/ahunt/oss-fuzz/git/strbuf.c:98:2 #3 0x936eaa in strbuf_add /home/ahunt/oss-fuzz/git/strbuf.c:295:2 #4 0x868112 in strbuf_addstr /home/ahunt/oss-fuzz/git/./strbuf.h:304:2 #5 0x86a8ad in expand_user_path /home/ahunt/oss-fuzz/git/path.c:758:2 #6 0x720bb1 in git_config_pathname /home/ahunt/oss-fuzz/git/config.c:1287:10 #7 0x5960e2 in git_init_db_config /home/ahunt/oss-fuzz/git/builtin/init-db.c:161:11 git#8 0x7255b8 in configset_iter /home/ahunt/oss-fuzz/git/config.c:1982:7 git#9 0x7253fc in repo_config /home/ahunt/oss-fuzz/git/config.c:2311:2 git#10 0x725ca7 in git_config /home/ahunt/oss-fuzz/git/config.c:2399:2 git#11 0x593e8d in create_default_files /home/ahunt/oss-fuzz/git/builtin/init-db.c:225:2 git#12 0x5935c6 in init_db /home/ahunt/oss-fuzz/git/builtin/init-db.c:449:11 git#13 0x59588e in cmd_init_db /home/ahunt/oss-fuzz/git/builtin/init-db.c:714:9 git#14 0x4cd60d in run_builtin /home/ahunt/oss-fuzz/git/git.c:453:11 git#15 0x4cb2da in handle_builtin /home/ahunt/oss-fuzz/git/git.c:704:3 git#16 0x4ccc37 in run_argv /home/ahunt/oss-fuzz/git/git.c:771:4 git#17 0x4cac29 in cmd_main /home/ahunt/oss-fuzz/git/git.c:902:19 git#18 0x69c4de in main /home/ahunt/oss-fuzz/git/common-main.c:52:11 git#19 0x7f23552d6349 in __libc_start_main (/lib64/libc.so.6+0x24349) Signed-off-by: Andrzej Hunt <ajrhunt@google.com>
gitster
pushed a commit
that referenced
this pull request
Mar 8, 2021
The primary goal of this change is to stop leaking init_db_template_dir. This leak can happen because: 1. git_init_db_config() allocates new memory into init_db_template_dir without first freeing the existing value. 2. init_db_template_dir might already contain data, either because: 2.1 git_config() can be invoked twice with this callback in a single process - at least 2 allocations are likely. 2.2 A single git_config() allocation can invoke the callback multiple times for a given key (see further explanation in the function docs) - each of those calls will trigger another leak. The simplest fix for the leak would be to free(init_db_template_dir) before overwriting it. Instead we choose to convert to fetching init.templatedir via git_config_get_value() as that is more explicit, more efficient, and avoids allocations (the returned result is owned by the config cache, so we aren't responsible for freeing it). If we remove init_db_template_dir, git_init_db_config() ends up being responsible only for forwarding core.* config values to platform_core_config(). However platform_core_config() already ignores non-core.* config values, so we can safely remove git_init_db_config() and invoke git_config() directly with platform_core_config() as the callback. The platform_core_config forwarding was originally added in: 2878533 (mingw: respect core.hidedotfiles = false in git-init again, 2019-03-11 And I suspect the potential for a leak existed since the original implementation of git_init_db_config in: 90b4518 (Add `init.templatedir` configuration variable., 2010-02-17) LSAN output from t0001: Direct leak of 73 byte(s) in 1 object(s) allocated from: #0 0x49a859 in realloc /home/abuild/rpmbuild/BUILD/llvm-11.0.0.src/build/../projects/compiler-rt/lib/asan/asan_malloc_linux.cpp:164:3 #1 0x9a7276 in xrealloc /home/ahunt/oss-fuzz/git/wrapper.c:126:8 #2 0x9362ad in strbuf_grow /home/ahunt/oss-fuzz/git/strbuf.c:98:2 #3 0x936eaa in strbuf_add /home/ahunt/oss-fuzz/git/strbuf.c:295:2 #4 0x868112 in strbuf_addstr /home/ahunt/oss-fuzz/git/./strbuf.h:304:2 #5 0x86a8ad in expand_user_path /home/ahunt/oss-fuzz/git/path.c:758:2 #6 0x720bb1 in git_config_pathname /home/ahunt/oss-fuzz/git/config.c:1287:10 #7 0x5960e2 in git_init_db_config /home/ahunt/oss-fuzz/git/builtin/init-db.c:161:11 #8 0x7255b8 in configset_iter /home/ahunt/oss-fuzz/git/config.c:1982:7 #9 0x7253fc in repo_config /home/ahunt/oss-fuzz/git/config.c:2311:2 #10 0x725ca7 in git_config /home/ahunt/oss-fuzz/git/config.c:2399:2 #11 0x593e8d in create_default_files /home/ahunt/oss-fuzz/git/builtin/init-db.c:225:2 #12 0x5935c6 in init_db /home/ahunt/oss-fuzz/git/builtin/init-db.c:449:11 #13 0x59588e in cmd_init_db /home/ahunt/oss-fuzz/git/builtin/init-db.c:714:9 #14 0x4cd60d in run_builtin /home/ahunt/oss-fuzz/git/git.c:453:11 #15 0x4cb2da in handle_builtin /home/ahunt/oss-fuzz/git/git.c:704:3 #16 0x4ccc37 in run_argv /home/ahunt/oss-fuzz/git/git.c:771:4 #17 0x4cac29 in cmd_main /home/ahunt/oss-fuzz/git/git.c:902:19 #18 0x69c4de in main /home/ahunt/oss-fuzz/git/common-main.c:52:11 #19 0x7f23552d6349 in __libc_start_main (/lib64/libc.so.6+0x24349) Signed-off-by: Andrzej Hunt <ajrhunt@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
ahunt
added a commit
to ahunt/git
that referenced
this pull request
Mar 14, 2021
The primary goal of this change is to stop leaking init_db_template_dir. This leak can happen because: 1. git_init_db_config() allocates new memory into init_db_template_dir without first freeing the existing value. 2. init_db_template_dir might already contain data, either because: 2.1 git_config() can be invoked twice with this callback in a single process - at least 2 allocations are likely. 2.2 A single git_config() allocation can invoke the callback multiple times for a given key (see further explanation in the function docs) - each of those calls will trigger another leak. The simplest fix for the leak would be to free(init_db_template_dir) before overwriting it. Instead we choose to convert to fetching init.templatedir via git_config_get_value() as that is more explicit, more efficient, and avoids allocations (the returned result is owned by the config cache, so we aren't responsible for freeing it). If we remove init_db_template_dir, git_init_db_config() ends up being responsible only for forwarding core.* config values to platform_core_config(). However platform_core_config() already ignores non-core.* config values, so we can safely remove git_init_db_config() and invoke git_config() directly with platform_core_config() as the callback. The platform_core_config forwarding was originally added in: 2878533 (mingw: respect core.hidedotfiles = false in git-init again, 2019-03-11 And I suspect the potential for a leak existed since the original implementation of git_init_db_config in: 90b4518 (Add `init.templatedir` configuration variable., 2010-02-17) LSAN output from t0001: Direct leak of 73 byte(s) in 1 object(s) allocated from: #0 0x49a859 in realloc /home/abuild/rpmbuild/BUILD/llvm-11.0.0.src/build/../projects/compiler-rt/lib/asan/asan_malloc_linux.cpp:164:3 #1 0x9a7276 in xrealloc /home/ahunt/oss-fuzz/git/wrapper.c:126:8 #2 0x9362ad in strbuf_grow /home/ahunt/oss-fuzz/git/strbuf.c:98:2 #3 0x936eaa in strbuf_add /home/ahunt/oss-fuzz/git/strbuf.c:295:2 #4 0x868112 in strbuf_addstr /home/ahunt/oss-fuzz/git/./strbuf.h:304:2 #5 0x86a8ad in expand_user_path /home/ahunt/oss-fuzz/git/path.c:758:2 #6 0x720bb1 in git_config_pathname /home/ahunt/oss-fuzz/git/config.c:1287:10 #7 0x5960e2 in git_init_db_config /home/ahunt/oss-fuzz/git/builtin/init-db.c:161:11 git#8 0x7255b8 in configset_iter /home/ahunt/oss-fuzz/git/config.c:1982:7 git#9 0x7253fc in repo_config /home/ahunt/oss-fuzz/git/config.c:2311:2 git#10 0x725ca7 in git_config /home/ahunt/oss-fuzz/git/config.c:2399:2 git#11 0x593e8d in create_default_files /home/ahunt/oss-fuzz/git/builtin/init-db.c:225:2 git#12 0x5935c6 in init_db /home/ahunt/oss-fuzz/git/builtin/init-db.c:449:11 git#13 0x59588e in cmd_init_db /home/ahunt/oss-fuzz/git/builtin/init-db.c:714:9 git#14 0x4cd60d in run_builtin /home/ahunt/oss-fuzz/git/git.c:453:11 git#15 0x4cb2da in handle_builtin /home/ahunt/oss-fuzz/git/git.c:704:3 git#16 0x4ccc37 in run_argv /home/ahunt/oss-fuzz/git/git.c:771:4 git#17 0x4cac29 in cmd_main /home/ahunt/oss-fuzz/git/git.c:902:19 git#18 0x69c4de in main /home/ahunt/oss-fuzz/git/common-main.c:52:11 git#19 0x7f23552d6349 in __libc_start_main (/lib64/libc.so.6+0x24349) Signed-off-by: Andrzej Hunt <ajrhunt@google.com>
ahunt
added a commit
to ahunt/git
that referenced
this pull request
Mar 14, 2021
The primary goal of this change is to stop leaking init_db_template_dir. This leak can happen because: 1. git_init_db_config() allocates new memory into init_db_template_dir without first freeing the existing value. 2. init_db_template_dir might already contain data, either because: 2.1 git_config() can be invoked twice with this callback in a single process - at least 2 allocations are likely. 2.2 A single git_config() allocation can invoke the callback multiple times for a given key (see further explanation in the function docs) - each of those calls will trigger another leak. The simplest fix for the leak would be to free(init_db_template_dir) before overwriting it. Instead we choose to convert to fetching init.templatedir via git_config_get_value() as that is more explicit, more efficient, and avoids allocations (the returned result is owned by the config cache, so we aren't responsible for freeing it). If we remove init_db_template_dir, git_init_db_config() ends up being responsible only for forwarding core.* config values to platform_core_config(). However platform_core_config() already ignores non-core.* config values, so we can safely remove git_init_db_config() and invoke git_config() directly with platform_core_config() as the callback. The platform_core_config forwarding was originally added in: 2878533 (mingw: respect core.hidedotfiles = false in git-init again, 2019-03-11 And I suspect the potential for a leak existed since the original implementation of git_init_db_config in: 90b4518 (Add `init.templatedir` configuration variable., 2010-02-17) LSAN output from t0001: Direct leak of 73 byte(s) in 1 object(s) allocated from: #0 0x49a859 in realloc /home/abuild/rpmbuild/BUILD/llvm-11.0.0.src/build/../projects/compiler-rt/lib/asan/asan_malloc_linux.cpp:164:3 #1 0x9a7276 in xrealloc /home/ahunt/oss-fuzz/git/wrapper.c:126:8 #2 0x9362ad in strbuf_grow /home/ahunt/oss-fuzz/git/strbuf.c:98:2 #3 0x936eaa in strbuf_add /home/ahunt/oss-fuzz/git/strbuf.c:295:2 #4 0x868112 in strbuf_addstr /home/ahunt/oss-fuzz/git/./strbuf.h:304:2 #5 0x86a8ad in expand_user_path /home/ahunt/oss-fuzz/git/path.c:758:2 #6 0x720bb1 in git_config_pathname /home/ahunt/oss-fuzz/git/config.c:1287:10 #7 0x5960e2 in git_init_db_config /home/ahunt/oss-fuzz/git/builtin/init-db.c:161:11 git#8 0x7255b8 in configset_iter /home/ahunt/oss-fuzz/git/config.c:1982:7 git#9 0x7253fc in repo_config /home/ahunt/oss-fuzz/git/config.c:2311:2 git#10 0x725ca7 in git_config /home/ahunt/oss-fuzz/git/config.c:2399:2 git#11 0x593e8d in create_default_files /home/ahunt/oss-fuzz/git/builtin/init-db.c:225:2 git#12 0x5935c6 in init_db /home/ahunt/oss-fuzz/git/builtin/init-db.c:449:11 git#13 0x59588e in cmd_init_db /home/ahunt/oss-fuzz/git/builtin/init-db.c:714:9 git#14 0x4cd60d in run_builtin /home/ahunt/oss-fuzz/git/git.c:453:11 git#15 0x4cb2da in handle_builtin /home/ahunt/oss-fuzz/git/git.c:704:3 git#16 0x4ccc37 in run_argv /home/ahunt/oss-fuzz/git/git.c:771:4 git#17 0x4cac29 in cmd_main /home/ahunt/oss-fuzz/git/git.c:902:19 git#18 0x69c4de in main /home/ahunt/oss-fuzz/git/common-main.c:52:11 git#19 0x7f23552d6349 in __libc_start_main (/lib64/libc.so.6+0x24349) Signed-off-by: Andrzej Hunt <ajrhunt@google.com>
ahunt
added a commit
to ahunt/git
that referenced
this pull request
Mar 14, 2021
The primary goal of this change is to stop leaking init_db_template_dir. This leak can happen because: 1. git_init_db_config() allocates new memory into init_db_template_dir without first freeing the existing value. 2. init_db_template_dir might already contain data, either because: 2.1 git_config() can be invoked twice with this callback in a single process - at least 2 allocations are likely. 2.2 A single git_config() allocation can invoke the callback multiple times for a given key (see further explanation in the function docs) - each of those calls will trigger another leak. The simplest fix for the leak would be to free(init_db_template_dir) before overwriting it. Instead we choose to convert to fetching init.templatedir via git_config_get_value() as that is more explicit, more efficient, and avoids allocations (the returned result is owned by the config cache, so we aren't responsible for freeing it). If we remove init_db_template_dir, git_init_db_config() ends up being responsible only for forwarding core.* config values to platform_core_config(). However platform_core_config() already ignores non-core.* config values, so we can safely remove git_init_db_config() and invoke git_config() directly with platform_core_config() as the callback. The platform_core_config forwarding was originally added in: 2878533 (mingw: respect core.hidedotfiles = false in git-init again, 2019-03-11 And I suspect the potential for a leak existed since the original implementation of git_init_db_config in: 90b4518 (Add `init.templatedir` configuration variable., 2010-02-17) LSAN output from t0001: Direct leak of 73 byte(s) in 1 object(s) allocated from: #0 0x49a859 in realloc /home/abuild/rpmbuild/BUILD/llvm-11.0.0.src/build/../projects/compiler-rt/lib/asan/asan_malloc_linux.cpp:164:3 #1 0x9a7276 in xrealloc /home/ahunt/oss-fuzz/git/wrapper.c:126:8 #2 0x9362ad in strbuf_grow /home/ahunt/oss-fuzz/git/strbuf.c:98:2 #3 0x936eaa in strbuf_add /home/ahunt/oss-fuzz/git/strbuf.c:295:2 #4 0x868112 in strbuf_addstr /home/ahunt/oss-fuzz/git/./strbuf.h:304:2 #5 0x86a8ad in expand_user_path /home/ahunt/oss-fuzz/git/path.c:758:2 #6 0x720bb1 in git_config_pathname /home/ahunt/oss-fuzz/git/config.c:1287:10 #7 0x5960e2 in git_init_db_config /home/ahunt/oss-fuzz/git/builtin/init-db.c:161:11 git#8 0x7255b8 in configset_iter /home/ahunt/oss-fuzz/git/config.c:1982:7 git#9 0x7253fc in repo_config /home/ahunt/oss-fuzz/git/config.c:2311:2 git#10 0x725ca7 in git_config /home/ahunt/oss-fuzz/git/config.c:2399:2 git#11 0x593e8d in create_default_files /home/ahunt/oss-fuzz/git/builtin/init-db.c:225:2 git#12 0x5935c6 in init_db /home/ahunt/oss-fuzz/git/builtin/init-db.c:449:11 git#13 0x59588e in cmd_init_db /home/ahunt/oss-fuzz/git/builtin/init-db.c:714:9 git#14 0x4cd60d in run_builtin /home/ahunt/oss-fuzz/git/git.c:453:11 git#15 0x4cb2da in handle_builtin /home/ahunt/oss-fuzz/git/git.c:704:3 git#16 0x4ccc37 in run_argv /home/ahunt/oss-fuzz/git/git.c:771:4 git#17 0x4cac29 in cmd_main /home/ahunt/oss-fuzz/git/git.c:902:19 git#18 0x69c4de in main /home/ahunt/oss-fuzz/git/common-main.c:52:11 git#19 0x7f23552d6349 in __libc_start_main (/lib64/libc.so.6+0x24349) Signed-off-by: Andrzej Hunt <ajrhunt@google.com>
gitster
pushed a commit
that referenced
this pull request
Mar 15, 2021
The primary goal of this change is to stop leaking init_db_template_dir. This leak can happen because: 1. git_init_db_config() allocates new memory into init_db_template_dir without first freeing the existing value. 2. init_db_template_dir might already contain data, either because: 2.1 git_config() can be invoked twice with this callback in a single process - at least 2 allocations are likely. 2.2 A single git_config() allocation can invoke the callback multiple times for a given key (see further explanation in the function docs) - each of those calls will trigger another leak. The simplest fix for the leak would be to free(init_db_template_dir) before overwriting it. Instead we choose to convert to fetching init.templatedir via git_config_get_value() as that is more explicit, more efficient, and avoids allocations (the returned result is owned by the config cache, so we aren't responsible for freeing it). If we remove init_db_template_dir, git_init_db_config() ends up being responsible only for forwarding core.* config values to platform_core_config(). However platform_core_config() already ignores non-core.* config values, so we can safely remove git_init_db_config() and invoke git_config() directly with platform_core_config() as the callback. The platform_core_config forwarding was originally added in: 2878533 (mingw: respect core.hidedotfiles = false in git-init again, 2019-03-11 And I suspect the potential for a leak existed since the original implementation of git_init_db_config in: 90b4518 (Add `init.templatedir` configuration variable., 2010-02-17) LSAN output from t0001: Direct leak of 73 byte(s) in 1 object(s) allocated from: #0 0x49a859 in realloc /home/abuild/rpmbuild/BUILD/llvm-11.0.0.src/build/../projects/compiler-rt/lib/asan/asan_malloc_linux.cpp:164:3 #1 0x9a7276 in xrealloc /home/ahunt/oss-fuzz/git/wrapper.c:126:8 #2 0x9362ad in strbuf_grow /home/ahunt/oss-fuzz/git/strbuf.c:98:2 #3 0x936eaa in strbuf_add /home/ahunt/oss-fuzz/git/strbuf.c:295:2 #4 0x868112 in strbuf_addstr /home/ahunt/oss-fuzz/git/./strbuf.h:304:2 #5 0x86a8ad in expand_user_path /home/ahunt/oss-fuzz/git/path.c:758:2 #6 0x720bb1 in git_config_pathname /home/ahunt/oss-fuzz/git/config.c:1287:10 #7 0x5960e2 in git_init_db_config /home/ahunt/oss-fuzz/git/builtin/init-db.c:161:11 #8 0x7255b8 in configset_iter /home/ahunt/oss-fuzz/git/config.c:1982:7 #9 0x7253fc in repo_config /home/ahunt/oss-fuzz/git/config.c:2311:2 #10 0x725ca7 in git_config /home/ahunt/oss-fuzz/git/config.c:2399:2 #11 0x593e8d in create_default_files /home/ahunt/oss-fuzz/git/builtin/init-db.c:225:2 #12 0x5935c6 in init_db /home/ahunt/oss-fuzz/git/builtin/init-db.c:449:11 #13 0x59588e in cmd_init_db /home/ahunt/oss-fuzz/git/builtin/init-db.c:714:9 #14 0x4cd60d in run_builtin /home/ahunt/oss-fuzz/git/git.c:453:11 #15 0x4cb2da in handle_builtin /home/ahunt/oss-fuzz/git/git.c:704:3 #16 0x4ccc37 in run_argv /home/ahunt/oss-fuzz/git/git.c:771:4 #17 0x4cac29 in cmd_main /home/ahunt/oss-fuzz/git/git.c:902:19 #18 0x69c4de in main /home/ahunt/oss-fuzz/git/common-main.c:52:11 #19 0x7f23552d6349 in __libc_start_main (/lib64/libc.so.6+0x24349) Signed-off-by: Andrzej Hunt <ajrhunt@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
ahunt
added a commit
to ahunt/git
that referenced
this pull request
Mar 16, 2021
The primary goal of this change is to stop leaking init_db_template_dir. This leak can happen because: 1. git_init_db_config() allocates new memory into init_db_template_dir without first freeing the existing value. 2. init_db_template_dir might already contain data, either because: 2.1 git_config() can be invoked twice with this callback in a single process - at least 2 allocations are likely. 2.2 A single git_config() allocation can invoke the callback multiple times for a given key (see further explanation in the function docs) - each of those calls will trigger another leak. The simplest fix for the leak would be to free(init_db_template_dir) before overwriting it. Instead we choose to convert to fetching init.templatedir via git_config_get_value() as that is more explicit, more efficient, and avoids allocations (the returned result is owned by the config cache, so we aren't responsible for freeing it). If we remove init_db_template_dir, git_init_db_config() ends up being responsible only for forwarding core.* config values to platform_core_config(). However platform_core_config() already ignores non-core.* config values, so we can safely remove git_init_db_config() and invoke git_config() directly with platform_core_config() as the callback. The platform_core_config forwarding was originally added in: 2878533 (mingw: respect core.hidedotfiles = false in git-init again, 2019-03-11 And I suspect the potential for a leak existed since the original implementation of git_init_db_config in: 90b4518 (Add `init.templatedir` configuration variable., 2010-02-17) LSAN output from t0001: Direct leak of 73 byte(s) in 1 object(s) allocated from: #0 0x49a859 in realloc /home/abuild/rpmbuild/BUILD/llvm-11.0.0.src/build/../projects/compiler-rt/lib/asan/asan_malloc_linux.cpp:164:3 #1 0x9a7276 in xrealloc /home/ahunt/oss-fuzz/git/wrapper.c:126:8 #2 0x9362ad in strbuf_grow /home/ahunt/oss-fuzz/git/strbuf.c:98:2 #3 0x936eaa in strbuf_add /home/ahunt/oss-fuzz/git/strbuf.c:295:2 #4 0x868112 in strbuf_addstr /home/ahunt/oss-fuzz/git/./strbuf.h:304:2 #5 0x86a8ad in expand_user_path /home/ahunt/oss-fuzz/git/path.c:758:2 #6 0x720bb1 in git_config_pathname /home/ahunt/oss-fuzz/git/config.c:1287:10 #7 0x5960e2 in git_init_db_config /home/ahunt/oss-fuzz/git/builtin/init-db.c:161:11 git#8 0x7255b8 in configset_iter /home/ahunt/oss-fuzz/git/config.c:1982:7 git#9 0x7253fc in repo_config /home/ahunt/oss-fuzz/git/config.c:2311:2 git#10 0x725ca7 in git_config /home/ahunt/oss-fuzz/git/config.c:2399:2 git#11 0x593e8d in create_default_files /home/ahunt/oss-fuzz/git/builtin/init-db.c:225:2 git#12 0x5935c6 in init_db /home/ahunt/oss-fuzz/git/builtin/init-db.c:449:11 git#13 0x59588e in cmd_init_db /home/ahunt/oss-fuzz/git/builtin/init-db.c:714:9 git#14 0x4cd60d in run_builtin /home/ahunt/oss-fuzz/git/git.c:453:11 git#15 0x4cb2da in handle_builtin /home/ahunt/oss-fuzz/git/git.c:704:3 git#16 0x4ccc37 in run_argv /home/ahunt/oss-fuzz/git/git.c:771:4 git#17 0x4cac29 in cmd_main /home/ahunt/oss-fuzz/git/git.c:902:19 git#18 0x69c4de in main /home/ahunt/oss-fuzz/git/common-main.c:52:11 git#19 0x7f23552d6349 in __libc_start_main (/lib64/libc.so.6+0x24349) Signed-off-by: Andrzej Hunt <ajrhunt@google.com>
ahunt
added a commit
to ahunt/git
that referenced
this pull request
Mar 21, 2021
The primary goal of this change is to stop leaking init_db_template_dir. This leak can happen because: 1. git_init_db_config() allocates new memory into init_db_template_dir without first freeing the existing value. 2. init_db_template_dir might already contain data, either because: 2.1 git_config() can be invoked twice with this callback in a single process - at least 2 allocations are likely. 2.2 A single git_config() allocation can invoke the callback multiple times for a given key (see further explanation in the function docs) - each of those calls will trigger another leak. The simplest fix for the leak would be to free(init_db_template_dir) before overwriting it. Instead we choose to convert to fetching init.templatedir via git_config_get_value() as that is more explicit, more efficient, and avoids allocations (the returned result is owned by the config cache, so we aren't responsible for freeing it). If we remove init_db_template_dir, git_init_db_config() ends up being responsible only for forwarding core.* config values to platform_core_config(). However platform_core_config() already ignores non-core.* config values, so we can safely remove git_init_db_config() and invoke git_config() directly with platform_core_config() as the callback. The platform_core_config forwarding was originally added in: 2878533 (mingw: respect core.hidedotfiles = false in git-init again, 2019-03-11 And I suspect the potential for a leak existed since the original implementation of git_init_db_config in: 90b4518 (Add `init.templatedir` configuration variable., 2010-02-17) LSAN output from t0001: Direct leak of 73 byte(s) in 1 object(s) allocated from: #0 0x49a859 in realloc /home/abuild/rpmbuild/BUILD/llvm-11.0.0.src/build/../projects/compiler-rt/lib/asan/asan_malloc_linux.cpp:164:3 #1 0x9a7276 in xrealloc /home/ahunt/oss-fuzz/git/wrapper.c:126:8 #2 0x9362ad in strbuf_grow /home/ahunt/oss-fuzz/git/strbuf.c:98:2 #3 0x936eaa in strbuf_add /home/ahunt/oss-fuzz/git/strbuf.c:295:2 #4 0x868112 in strbuf_addstr /home/ahunt/oss-fuzz/git/./strbuf.h:304:2 #5 0x86a8ad in expand_user_path /home/ahunt/oss-fuzz/git/path.c:758:2 #6 0x720bb1 in git_config_pathname /home/ahunt/oss-fuzz/git/config.c:1287:10 #7 0x5960e2 in git_init_db_config /home/ahunt/oss-fuzz/git/builtin/init-db.c:161:11 git#8 0x7255b8 in configset_iter /home/ahunt/oss-fuzz/git/config.c:1982:7 git#9 0x7253fc in repo_config /home/ahunt/oss-fuzz/git/config.c:2311:2 git#10 0x725ca7 in git_config /home/ahunt/oss-fuzz/git/config.c:2399:2 git#11 0x593e8d in create_default_files /home/ahunt/oss-fuzz/git/builtin/init-db.c:225:2 git#12 0x5935c6 in init_db /home/ahunt/oss-fuzz/git/builtin/init-db.c:449:11 git#13 0x59588e in cmd_init_db /home/ahunt/oss-fuzz/git/builtin/init-db.c:714:9 git#14 0x4cd60d in run_builtin /home/ahunt/oss-fuzz/git/git.c:453:11 git#15 0x4cb2da in handle_builtin /home/ahunt/oss-fuzz/git/git.c:704:3 git#16 0x4ccc37 in run_argv /home/ahunt/oss-fuzz/git/git.c:771:4 git#17 0x4cac29 in cmd_main /home/ahunt/oss-fuzz/git/git.c:902:19 git#18 0x69c4de in main /home/ahunt/oss-fuzz/git/common-main.c:52:11 git#19 0x7f23552d6349 in __libc_start_main (/lib64/libc.so.6+0x24349) Signed-off-by: Andrzej Hunt <ajrhunt@google.com>
ahunt
added a commit
to ahunt/git
that referenced
this pull request
Apr 9, 2021
limit_list() iterates over the original revs->commits list, and consumes many of its entries via pop_commit. However we might stop iterating over the list early (e.g. if we realise that the rest of the list is uninteresting). If we do stop iterating early, list will be pointing to the unconsumed portion of revs->commits - and we need to free this list to avoid a leak. (revs->commits itself will be an invalid pointer: it will have been free'd during the first pop_commit.) This leak was found while running t0090. It's not likely to be very impactful, but it can happen quite early during some checkout invocations, and hence seems to be worth fixing: Direct leak of 16 byte(s) in 1 object(s) allocated from: #0 0x49a85d in malloc ../projects/compiler-rt/lib/asan/asan_malloc_linux.cpp:145:3 #1 0x9ac084 in do_xmalloc wrapper.c:41:8 #2 0x9ac05a in xmalloc wrapper.c:62:9 #3 0x7175d6 in commit_list_insert commit.c:540:33 #4 0x71800f in commit_list_insert_by_date commit.c:604:9 #5 0x8f8d2e in process_parents revision.c:1128:5 #6 0x8f2f2c in limit_list revision.c:1418:7 #7 0x8f210e in prepare_revision_walk revision.c:3577:7 git#8 0x514170 in orphaned_commit_warning builtin/checkout.c:1185:6 git#9 0x512f05 in switch_branches builtin/checkout.c:1250:3 git#10 0x50f8de in checkout_branch builtin/checkout.c:1646:9 git#11 0x50ba12 in checkout_main builtin/checkout.c:2003:9 git#12 0x5086c0 in cmd_checkout builtin/checkout.c:2055:8 git#13 0x4cd91d in run_builtin git.c:467:11 git#14 0x4cb5f3 in handle_builtin git.c:719:3 git#15 0x4ccf47 in run_argv git.c:808:4 git#16 0x4caf49 in cmd_main git.c:939:19 git#17 0x69dc0e in main common-main.c:52:11 git#18 0x7faaabd0e349 in __libc_start_main (/lib64/libc.so.6+0x24349) Indirect leak of 48 byte(s) in 3 object(s) allocated from: #0 0x49a85d in malloc ../projects/compiler-rt/lib/asan/asan_malloc_linux.cpp:145:3 #1 0x9ac084 in do_xmalloc wrapper.c:41:8 #2 0x9ac05a in xmalloc wrapper.c:62:9 #3 0x717de6 in commit_list_append commit.c:1609:35 #4 0x8f1f9b in prepare_revision_walk revision.c:3554:12 #5 0x514170 in orphaned_commit_warning builtin/checkout.c:1185:6 #6 0x512f05 in switch_branches builtin/checkout.c:1250:3 #7 0x50f8de in checkout_branch builtin/checkout.c:1646:9 git#8 0x50ba12 in checkout_main builtin/checkout.c:2003:9 git#9 0x5086c0 in cmd_checkout builtin/checkout.c:2055:8 git#10 0x4cd91d in run_builtin git.c:467:11 git#11 0x4cb5f3 in handle_builtin git.c:719:3 git#12 0x4ccf47 in run_argv git.c:808:4 git#13 0x4caf49 in cmd_main git.c:939:19 git#14 0x69dc0e in main common-main.c:52:11 git#15 0x7faaabd0e349 in __libc_start_main (/lib64/libc.so.6+0x24349) Signed-off-by: Andrzej Hunt <ajrhunt@google.com>
ahunt
added a commit
to ahunt/git
that referenced
this pull request
Apr 9, 2021
add_pending_object() populates rev.pending, we need to take care of clearing it once we're done. This code is run close to the end of a checkout, therefore this leak seems like it would have very little impact. See also LSAN output from t0020 below: Direct leak of 2048 byte(s) in 1 object(s) allocated from: #0 0x49ab79 in realloc ../projects/compiler-rt/lib/asan/asan_malloc_linux.cpp:164:3 #1 0x9acc46 in xrealloc wrapper.c:126:8 #2 0x83e3a3 in add_object_array_with_path object.c:337:3 #3 0x8f672a in add_pending_object_with_path revision.c:329:2 #4 0x8eaeab in add_pending_object_with_mode revision.c:336:2 #5 0x8eae9d in add_pending_object revision.c:342:2 #6 0x5154a0 in show_local_changes builtin/checkout.c:602:2 #7 0x513b00 in merge_working_tree builtin/checkout.c:979:3 git#8 0x512cb3 in switch_branches builtin/checkout.c:1242:9 git#9 0x50f8de in checkout_branch builtin/checkout.c:1646:9 git#10 0x50ba12 in checkout_main builtin/checkout.c:2003:9 git#11 0x5086c0 in cmd_checkout builtin/checkout.c:2055:8 git#12 0x4cd91d in run_builtin git.c:467:11 git#13 0x4cb5f3 in handle_builtin git.c:719:3 git#14 0x4ccf47 in run_argv git.c:808:4 git#15 0x4caf49 in cmd_main git.c:939:19 git#16 0x69e43e in main common-main.c:52:11 git#17 0x7f5dd1d50349 in __libc_start_main (/lib64/libc.so.6+0x24349) SUMMARY: AddressSanitizer: 2048 byte(s) leaked in 1 allocation(s). Signed-off-by: Andrzej Hunt <ajrhunt@google.com>
ahunt
added a commit
to ahunt/git
that referenced
this pull request
Apr 9, 2021
add_pending_object() populates rev.pending, we need to take care of clearing it once we're done. This code is run close to the end of a checkout, therefore this leak seems like it would have very little impact. See also LSAN output from t0020 below: Direct leak of 2048 byte(s) in 1 object(s) allocated from: #0 0x49ab79 in realloc ../projects/compiler-rt/lib/asan/asan_malloc_linux.cpp:164:3 #1 0x9acc46 in xrealloc wrapper.c:126:8 #2 0x83e3a3 in add_object_array_with_path object.c:337:3 #3 0x8f672a in add_pending_object_with_path revision.c:329:2 #4 0x8eaeab in add_pending_object_with_mode revision.c:336:2 #5 0x8eae9d in add_pending_object revision.c:342:2 #6 0x5154a0 in show_local_changes builtin/checkout.c:602:2 #7 0x513b00 in merge_working_tree builtin/checkout.c:979:3 git#8 0x512cb3 in switch_branches builtin/checkout.c:1242:9 git#9 0x50f8de in checkout_branch builtin/checkout.c:1646:9 git#10 0x50ba12 in checkout_main builtin/checkout.c:2003:9 git#11 0x5086c0 in cmd_checkout builtin/checkout.c:2055:8 git#12 0x4cd91d in run_builtin git.c:467:11 git#13 0x4cb5f3 in handle_builtin git.c:719:3 git#14 0x4ccf47 in run_argv git.c:808:4 git#15 0x4caf49 in cmd_main git.c:939:19 git#16 0x69e43e in main common-main.c:52:11 git#17 0x7f5dd1d50349 in __libc_start_main (/lib64/libc.so.6+0x24349) SUMMARY: AddressSanitizer: 2048 byte(s) leaked in 1 allocation(s). Signed-off-by: Andrzej Hunt <ajrhunt@google.com>
ahunt
added a commit
to ahunt/git
that referenced
this pull request
Apr 9, 2021
add_pending_object() populates rev.pending, we need to take care of clearing it once we're done. This code is run close to the end of a checkout, therefore this leak seems like it would have very little impact. See also LSAN output from t0020 below: Direct leak of 2048 byte(s) in 1 object(s) allocated from: #0 0x49ab79 in realloc ../projects/compiler-rt/lib/asan/asan_malloc_linux.cpp:164:3 #1 0x9acc46 in xrealloc wrapper.c:126:8 #2 0x83e3a3 in add_object_array_with_path object.c:337:3 #3 0x8f672a in add_pending_object_with_path revision.c:329:2 #4 0x8eaeab in add_pending_object_with_mode revision.c:336:2 #5 0x8eae9d in add_pending_object revision.c:342:2 #6 0x5154a0 in show_local_changes builtin/checkout.c:602:2 #7 0x513b00 in merge_working_tree builtin/checkout.c:979:3 git#8 0x512cb3 in switch_branches builtin/checkout.c:1242:9 git#9 0x50f8de in checkout_branch builtin/checkout.c:1646:9 git#10 0x50ba12 in checkout_main builtin/checkout.c:2003:9 git#11 0x5086c0 in cmd_checkout builtin/checkout.c:2055:8 git#12 0x4cd91d in run_builtin git.c:467:11 git#13 0x4cb5f3 in handle_builtin git.c:719:3 git#14 0x4ccf47 in run_argv git.c:808:4 git#15 0x4caf49 in cmd_main git.c:939:19 git#16 0x69e43e in main common-main.c:52:11 git#17 0x7f5dd1d50349 in __libc_start_main (/lib64/libc.so.6+0x24349) SUMMARY: AddressSanitizer: 2048 byte(s) leaked in 1 allocation(s). Signed-off-by: Andrzej Hunt <ajrhunt@google.com>
ahunt
added a commit
to ahunt/git
that referenced
this pull request
Apr 9, 2021
add_pending_object() populates rev.pending, we need to take care of clearing it once we're done. This code is run close to the end of a checkout, therefore this leak seems like it would have very little impact. See also LSAN output from t0020 below: Direct leak of 2048 byte(s) in 1 object(s) allocated from: #0 0x49ab79 in realloc ../projects/compiler-rt/lib/asan/asan_malloc_linux.cpp:164:3 #1 0x9acc46 in xrealloc wrapper.c:126:8 #2 0x83e3a3 in add_object_array_with_path object.c:337:3 #3 0x8f672a in add_pending_object_with_path revision.c:329:2 #4 0x8eaeab in add_pending_object_with_mode revision.c:336:2 #5 0x8eae9d in add_pending_object revision.c:342:2 #6 0x5154a0 in show_local_changes builtin/checkout.c:602:2 #7 0x513b00 in merge_working_tree builtin/checkout.c:979:3 git#8 0x512cb3 in switch_branches builtin/checkout.c:1242:9 git#9 0x50f8de in checkout_branch builtin/checkout.c:1646:9 git#10 0x50ba12 in checkout_main builtin/checkout.c:2003:9 git#11 0x5086c0 in cmd_checkout builtin/checkout.c:2055:8 git#12 0x4cd91d in run_builtin git.c:467:11 git#13 0x4cb5f3 in handle_builtin git.c:719:3 git#14 0x4ccf47 in run_argv git.c:808:4 git#15 0x4caf49 in cmd_main git.c:939:19 git#16 0x69e43e in main common-main.c:52:11 git#17 0x7f5dd1d50349 in __libc_start_main (/lib64/libc.so.6+0x24349) SUMMARY: AddressSanitizer: 2048 byte(s) leaked in 1 allocation(s). Signed-off-by: Andrzej Hunt <ajrhunt@google.com>
gitster
pushed a commit
that referenced
this pull request
Apr 11, 2021
add_pending_object() populates rev.pending, we need to take care of clearing it once we're done. This code is run close to the end of a checkout, therefore this leak seems like it would have very little impact. See also LSAN output from t0020 below: Direct leak of 2048 byte(s) in 1 object(s) allocated from: #0 0x49ab79 in realloc ../projects/compiler-rt/lib/asan/asan_malloc_linux.cpp:164:3 #1 0x9acc46 in xrealloc wrapper.c:126:8 #2 0x83e3a3 in add_object_array_with_path object.c:337:3 #3 0x8f672a in add_pending_object_with_path revision.c:329:2 #4 0x8eaeab in add_pending_object_with_mode revision.c:336:2 #5 0x8eae9d in add_pending_object revision.c:342:2 #6 0x5154a0 in show_local_changes builtin/checkout.c:602:2 #7 0x513b00 in merge_working_tree builtin/checkout.c:979:3 #8 0x512cb3 in switch_branches builtin/checkout.c:1242:9 #9 0x50f8de in checkout_branch builtin/checkout.c:1646:9 #10 0x50ba12 in checkout_main builtin/checkout.c:2003:9 #11 0x5086c0 in cmd_checkout builtin/checkout.c:2055:8 #12 0x4cd91d in run_builtin git.c:467:11 #13 0x4cb5f3 in handle_builtin git.c:719:3 #14 0x4ccf47 in run_argv git.c:808:4 #15 0x4caf49 in cmd_main git.c:939:19 #16 0x69e43e in main common-main.c:52:11 #17 0x7f5dd1d50349 in __libc_start_main (/lib64/libc.so.6+0x24349) SUMMARY: AddressSanitizer: 2048 byte(s) leaked in 1 allocation(s). Signed-off-by: Andrzej Hunt <ajrhunt@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
ahunt
added a commit
to ahunt/git
that referenced
this pull request
Apr 25, 2021
limit_list() iterates over the original revs->commits list, and consumes many of its entries via pop_commit. However we might stop iterating over the list early (e.g. if we realise that the rest of the list is uninteresting). If we do stop iterating early, list will be pointing to the unconsumed portion of revs->commits - and we need to free this list to avoid a leak. (revs->commits itself will be an invalid pointer: it will have been free'd during the first pop_commit.) This leak was found while running t0090. It's not likely to be very impactful, but it can happen quite early during some checkout invocations, and hence seems to be worth fixing: Direct leak of 16 byte(s) in 1 object(s) allocated from: #0 0x49a85d in malloc ../projects/compiler-rt/lib/asan/asan_malloc_linux.cpp:145:3 #1 0x9ac084 in do_xmalloc wrapper.c:41:8 #2 0x9ac05a in xmalloc wrapper.c:62:9 #3 0x7175d6 in commit_list_insert commit.c:540:33 #4 0x71800f in commit_list_insert_by_date commit.c:604:9 #5 0x8f8d2e in process_parents revision.c:1128:5 #6 0x8f2f2c in limit_list revision.c:1418:7 #7 0x8f210e in prepare_revision_walk revision.c:3577:7 git#8 0x514170 in orphaned_commit_warning builtin/checkout.c:1185:6 git#9 0x512f05 in switch_branches builtin/checkout.c:1250:3 git#10 0x50f8de in checkout_branch builtin/checkout.c:1646:9 git#11 0x50ba12 in checkout_main builtin/checkout.c:2003:9 git#12 0x5086c0 in cmd_checkout builtin/checkout.c:2055:8 git#13 0x4cd91d in run_builtin git.c:467:11 git#14 0x4cb5f3 in handle_builtin git.c:719:3 git#15 0x4ccf47 in run_argv git.c:808:4 git#16 0x4caf49 in cmd_main git.c:939:19 git#17 0x69dc0e in main common-main.c:52:11 git#18 0x7faaabd0e349 in __libc_start_main (/lib64/libc.so.6+0x24349) Indirect leak of 48 byte(s) in 3 object(s) allocated from: #0 0x49a85d in malloc ../projects/compiler-rt/lib/asan/asan_malloc_linux.cpp:145:3 #1 0x9ac084 in do_xmalloc wrapper.c:41:8 #2 0x9ac05a in xmalloc wrapper.c:62:9 #3 0x717de6 in commit_list_append commit.c:1609:35 #4 0x8f1f9b in prepare_revision_walk revision.c:3554:12 #5 0x514170 in orphaned_commit_warning builtin/checkout.c:1185:6 #6 0x512f05 in switch_branches builtin/checkout.c:1250:3 #7 0x50f8de in checkout_branch builtin/checkout.c:1646:9 git#8 0x50ba12 in checkout_main builtin/checkout.c:2003:9 git#9 0x5086c0 in cmd_checkout builtin/checkout.c:2055:8 git#10 0x4cd91d in run_builtin git.c:467:11 git#11 0x4cb5f3 in handle_builtin git.c:719:3 git#12 0x4ccf47 in run_argv git.c:808:4 git#13 0x4caf49 in cmd_main git.c:939:19 git#14 0x69dc0e in main common-main.c:52:11 git#15 0x7faaabd0e349 in __libc_start_main (/lib64/libc.so.6+0x24349) Signed-off-by: Andrzej Hunt <ajrhunt@google.com>
avar
added a commit
to avar/git
that referenced
this pull request
Feb 3, 2023
* avar/nuke-test_i18ngrep-again: tests with git#17 test_i18ngrep: replace it with 'grep' tests with git#16 test_i18ngrep: replace it with 'grep' tests with git#15 test_i18ngrep: replace it with 'grep' tests with git#14 test_i18ngrep: replace it with 'grep' tests with git#13 test_i18ngrep: replace it with 'grep' tests with git#12 test_i18ngrep: replace it with 'grep' tests with git#11 test_i18ngrep: replace it with 'grep'
avar
added a commit
to avar/git
that referenced
this pull request
Feb 3, 2023
* avar/nuke-test_i18ngrep-again: tests with git#17 test_i18ngrep: replace it with 'grep' tests with git#16 test_i18ngrep: replace it with 'grep' tests with git#15 test_i18ngrep: replace it with 'grep' tests with git#14 test_i18ngrep: replace it with 'grep' tests with git#13 test_i18ngrep: replace it with 'grep' tests with git#12 test_i18ngrep: replace it with 'grep' tests with git#11 test_i18ngrep: replace it with 'grep'
avar
added a commit
to avar/git
that referenced
this pull request
Feb 6, 2023
* avar/nuke-test_i18ngrep-again: tests with git#17 test_i18ngrep: replace it with 'grep' tests with git#16 test_i18ngrep: replace it with 'grep' tests with git#15 test_i18ngrep: replace it with 'grep' tests with git#14 test_i18ngrep: replace it with 'grep' tests with git#13 test_i18ngrep: replace it with 'grep' tests with git#12 test_i18ngrep: replace it with 'grep' tests with git#11 test_i18ngrep: replace it with 'grep'
avar
added a commit
to avar/git
that referenced
this pull request
Feb 6, 2023
* avar/nuke-test_i18ngrep-again: tests with git#17 test_i18ngrep: replace it with 'grep' tests with git#16 test_i18ngrep: replace it with 'grep' tests with git#15 test_i18ngrep: replace it with 'grep' tests with git#14 test_i18ngrep: replace it with 'grep' tests with git#13 test_i18ngrep: replace it with 'grep' tests with git#12 test_i18ngrep: replace it with 'grep' tests with git#11 test_i18ngrep: replace it with 'grep'
avar
added a commit
to avar/git
that referenced
this pull request
Feb 6, 2023
* avar/nuke-test_i18ngrep-again: tests with git#17 test_i18ngrep: replace it with 'grep' tests with git#16 test_i18ngrep: replace it with 'grep' tests with git#15 test_i18ngrep: replace it with 'grep' tests with git#14 test_i18ngrep: replace it with 'grep' tests with git#13 test_i18ngrep: replace it with 'grep' tests with git#12 test_i18ngrep: replace it with 'grep' tests with git#11 test_i18ngrep: replace it with 'grep'
avar
added a commit
to avar/git
that referenced
this pull request
Feb 7, 2023
* avar/nuke-test_i18ngrep-again: tests with git#17 test_i18ngrep: replace it with 'grep' tests with git#16 test_i18ngrep: replace it with 'grep' tests with git#15 test_i18ngrep: replace it with 'grep' tests with git#14 test_i18ngrep: replace it with 'grep' tests with git#13 test_i18ngrep: replace it with 'grep' tests with git#12 test_i18ngrep: replace it with 'grep' tests with git#11 test_i18ngrep: replace it with 'grep'
avar
added a commit
to avar/git
that referenced
this pull request
Feb 7, 2023
* avar/nuke-test_i18ngrep-again: tests with git#17 test_i18ngrep: replace it with 'grep' tests with git#16 test_i18ngrep: replace it with 'grep' tests with git#15 test_i18ngrep: replace it with 'grep' tests with git#14 test_i18ngrep: replace it with 'grep' tests with git#13 test_i18ngrep: replace it with 'grep' tests with git#12 test_i18ngrep: replace it with 'grep' tests with git#11 test_i18ngrep: replace it with 'grep'
avar
added a commit
to avar/git
that referenced
this pull request
Feb 8, 2023
* avar/nuke-test_i18ngrep-again: tests with git#17 test_i18ngrep: replace it with 'grep' tests with git#16 test_i18ngrep: replace it with 'grep' tests with git#15 test_i18ngrep: replace it with 'grep' tests with git#14 test_i18ngrep: replace it with 'grep' tests with git#13 test_i18ngrep: replace it with 'grep' tests with git#12 test_i18ngrep: replace it with 'grep' tests with git#11 test_i18ngrep: replace it with 'grep'
avar
added a commit
to avar/git
that referenced
this pull request
Feb 8, 2023
* avar/nuke-test_i18ngrep-again: tests with git#17 test_i18ngrep: replace it with 'grep' tests with git#16 test_i18ngrep: replace it with 'grep' tests with git#15 test_i18ngrep: replace it with 'grep' tests with git#14 test_i18ngrep: replace it with 'grep' tests with git#13 test_i18ngrep: replace it with 'grep' tests with git#12 test_i18ngrep: replace it with 'grep' tests with git#11 test_i18ngrep: replace it with 'grep'
avar
added a commit
to avar/git
that referenced
this pull request
Feb 10, 2023
* avar/nuke-test_i18ngrep-again: tests with git#17 test_i18ngrep: replace it with 'grep' tests with git#16 test_i18ngrep: replace it with 'grep' tests with git#15 test_i18ngrep: replace it with 'grep' tests with git#14 test_i18ngrep: replace it with 'grep' tests with git#13 test_i18ngrep: replace it with 'grep' tests with git#12 test_i18ngrep: replace it with 'grep' tests with git#11 test_i18ngrep: replace it with 'grep'
avar
added a commit
to avar/git
that referenced
this pull request
Mar 6, 2023
* avar/nuke-test_i18ngrep-again: tests with git#17 test_i18ngrep: replace it with 'grep' tests with git#16 test_i18ngrep: replace it with 'grep' tests with git#15 test_i18ngrep: replace it with 'grep' tests with git#14 test_i18ngrep: replace it with 'grep' tests with git#13 test_i18ngrep: replace it with 'grep' tests with git#12 test_i18ngrep: replace it with 'grep' tests with git#11 test_i18ngrep: replace it with 'grep'
avar
added a commit
to avar/git
that referenced
this pull request
Mar 7, 2023
* avar/nuke-test_i18ngrep-again: tests with git#17 test_i18ngrep: replace it with 'grep' tests with git#16 test_i18ngrep: replace it with 'grep' tests with git#15 test_i18ngrep: replace it with 'grep' tests with git#14 test_i18ngrep: replace it with 'grep' tests with git#13 test_i18ngrep: replace it with 'grep' tests with git#12 test_i18ngrep: replace it with 'grep' tests with git#11 test_i18ngrep: replace it with 'grep'
avar
added a commit
to avar/git
that referenced
this pull request
Mar 7, 2023
* avar/nuke-test_i18ngrep-again: tests with git#17 test_i18ngrep: replace it with 'grep' tests with git#16 test_i18ngrep: replace it with 'grep' tests with git#15 test_i18ngrep: replace it with 'grep' tests with git#14 test_i18ngrep: replace it with 'grep' tests with git#13 test_i18ngrep: replace it with 'grep' tests with git#12 test_i18ngrep: replace it with 'grep' tests with git#11 test_i18ngrep: replace it with 'grep'
avar
added a commit
to avar/git
that referenced
this pull request
May 1, 2023
For those tests that have 17 occurances of 'test_i18ngrep', replace it with 'grep'. Splitting this incremental conversion by number of occurances in the file is arbitrary, but help so keep the commit size down. The 'test_i18ngrep' wrapper has been deprecated since d162b25 (tests: remove support for GIT_TEST_GETTEXT_POISON, 2021-01-20). Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
ttaylorr
added a commit
to ttaylorr/git
that referenced
this pull request
Aug 23, 2023
When t5583-push-branches.sh was originally introduced via 425b4d7 (push: introduce '--branches' option, 2023-05-06), it was not leak-free. In fact, the test did not even run correctly until 022fbb6 (t5583: fix shebang line, 2023-05-12), but after applying that patch, we see a failure at t5583.8: ==2529087==ERROR: LeakSanitizer: detected memory leaks Direct leak of 384 byte(s) in 1 object(s) allocated from: #0 0x7fb536330986 in __interceptor_realloc ../../../../src/libsanitizer/lsan/lsan_interceptors.cpp:98 #1 0x55e07606cbf9 in xrealloc wrapper.c:140 #2 0x55e075fb6cb3 in prio_queue_put prio-queue.c:42 #3 0x55e075ec81cb in get_reachable_subset commit-reach.c:917 #4 0x55e075fe9cce in add_missing_tags remote.c:1518 #5 0x55e075fea1e4 in match_push_refs remote.c:1665 git#6 0x55e076050a8e in transport_push transport.c:1378 #7 0x55e075e2eb74 in push_with_options builtin/push.c:401 git#8 0x55e075e2edb0 in do_push builtin/push.c:458 git#9 0x55e075e2ff7a in cmd_push builtin/push.c:702 git#10 0x55e075d8aaf0 in run_builtin git.c:452 git#11 0x55e075d8af08 in handle_builtin git.c:706 git#12 0x55e075d8b12c in run_argv git.c:770 git#13 0x55e075d8b6a0 in cmd_main git.c:905 git#14 0x55e075e81f07 in main common-main.c:60 git#15 0x7fb5360ab6c9 in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58 git#16 0x7fb5360ab784 in __libc_start_main_impl ../csu/libc-start.c:360 git#17 0x55e075d88f40 in _start (git+0x1ff40) (BuildId: 38ad998b85a535e786129979443630d025ec2453) SUMMARY: LeakSanitizer: 384 byte(s) leaked in 1 allocation(s). This leak was addressed independently via 68b5117 (commit-reach: fix memory leak in get_reachable_subset(), 2023-06-03), which makes t5583 leak-free. But t5583 was not in the tree when 68b5117 was written, and the two only met after the latter was merged back in via 693bde4 (Merge branch 'mh/commit-reach-get-reachable-plug-leak', 2023-06-20). At that point, t5583 was leak-free. Let's mark it as such accordingly. Signed-off-by: Taylor Blau <me@ttaylorr.com>
ttaylorr
added a commit
to ttaylorr/git
that referenced
this pull request
Aug 23, 2023
When t5583-push-branches.sh was originally introduced via 425b4d7 (push: introduce '--branches' option, 2023-05-06), it was not leak-free. In fact, the test did not even run correctly until 022fbb6 (t5583: fix shebang line, 2023-05-12), but after applying that patch, we see a failure at t5583.8: ==2529087==ERROR: LeakSanitizer: detected memory leaks Direct leak of 384 byte(s) in 1 object(s) allocated from: #0 0x7fb536330986 in __interceptor_realloc ../../../../src/libsanitizer/lsan/lsan_interceptors.cpp:98 #1 0x55e07606cbf9 in xrealloc wrapper.c:140 #2 0x55e075fb6cb3 in prio_queue_put prio-queue.c:42 #3 0x55e075ec81cb in get_reachable_subset commit-reach.c:917 #4 0x55e075fe9cce in add_missing_tags remote.c:1518 #5 0x55e075fea1e4 in match_push_refs remote.c:1665 git#6 0x55e076050a8e in transport_push transport.c:1378 #7 0x55e075e2eb74 in push_with_options builtin/push.c:401 git#8 0x55e075e2edb0 in do_push builtin/push.c:458 git#9 0x55e075e2ff7a in cmd_push builtin/push.c:702 git#10 0x55e075d8aaf0 in run_builtin git.c:452 git#11 0x55e075d8af08 in handle_builtin git.c:706 git#12 0x55e075d8b12c in run_argv git.c:770 git#13 0x55e075d8b6a0 in cmd_main git.c:905 git#14 0x55e075e81f07 in main common-main.c:60 git#15 0x7fb5360ab6c9 in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58 git#16 0x7fb5360ab784 in __libc_start_main_impl ../csu/libc-start.c:360 git#17 0x55e075d88f40 in _start (git+0x1ff40) (BuildId: 38ad998b85a535e786129979443630d025ec2453) SUMMARY: LeakSanitizer: 384 byte(s) leaked in 1 allocation(s). This leak was addressed independently via 68b5117 (commit-reach: fix memory leak in get_reachable_subset(), 2023-06-03), which makes t5583 leak-free. But t5583 was not in the tree when 68b5117 was written, and the two only met after the latter was merged back in via 693bde4 (Merge branch 'mh/commit-reach-get-reachable-plug-leak', 2023-06-20). At that point, t5583 was leak-free. Let's mark it as such accordingly. Signed-off-by: Taylor Blau <me@ttaylorr.com>
ttaylorr
added a commit
to ttaylorr/git
that referenced
this pull request
Aug 23, 2023
When t5583-push-branches.sh was originally introduced via 425b4d7 (push: introduce '--branches' option, 2023-05-06), it was not leak-free. In fact, the test did not even run correctly until 022fbb6 (t5583: fix shebang line, 2023-05-12), but after applying that patch, we see a failure at t5583.8: ==2529087==ERROR: LeakSanitizer: detected memory leaks Direct leak of 384 byte(s) in 1 object(s) allocated from: #0 0x7fb536330986 in __interceptor_realloc ../../../../src/libsanitizer/lsan/lsan_interceptors.cpp:98 #1 0x55e07606cbf9 in xrealloc wrapper.c:140 #2 0x55e075fb6cb3 in prio_queue_put prio-queue.c:42 #3 0x55e075ec81cb in get_reachable_subset commit-reach.c:917 #4 0x55e075fe9cce in add_missing_tags remote.c:1518 #5 0x55e075fea1e4 in match_push_refs remote.c:1665 git#6 0x55e076050a8e in transport_push transport.c:1378 #7 0x55e075e2eb74 in push_with_options builtin/push.c:401 git#8 0x55e075e2edb0 in do_push builtin/push.c:458 git#9 0x55e075e2ff7a in cmd_push builtin/push.c:702 git#10 0x55e075d8aaf0 in run_builtin git.c:452 git#11 0x55e075d8af08 in handle_builtin git.c:706 git#12 0x55e075d8b12c in run_argv git.c:770 git#13 0x55e075d8b6a0 in cmd_main git.c:905 git#14 0x55e075e81f07 in main common-main.c:60 git#15 0x7fb5360ab6c9 in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58 git#16 0x7fb5360ab784 in __libc_start_main_impl ../csu/libc-start.c:360 git#17 0x55e075d88f40 in _start (git+0x1ff40) (BuildId: 38ad998b85a535e786129979443630d025ec2453) SUMMARY: LeakSanitizer: 384 byte(s) leaked in 1 allocation(s). This leak was addressed independently via 68b5117 (commit-reach: fix memory leak in get_reachable_subset(), 2023-06-03), which makes t5583 leak-free. But t5583 was not in the tree when 68b5117 was written, and the two only met after the latter was merged back in via 693bde4 (Merge branch 'mh/commit-reach-get-reachable-plug-leak', 2023-06-20). At that point, t5583 was leak-free. Let's mark it as such accordingly. Signed-off-by: Taylor Blau <me@ttaylorr.com>
ttaylorr
added a commit
to ttaylorr/git
that referenced
this pull request
Aug 24, 2023
When t5583-push-branches.sh was originally introduced via 425b4d7 (push: introduce '--branches' option, 2023-05-06), it was not leak-free. In fact, the test did not even run correctly until 022fbb6 (t5583: fix shebang line, 2023-05-12), but after applying that patch, we see a failure at t5583.8: ==2529087==ERROR: LeakSanitizer: detected memory leaks Direct leak of 384 byte(s) in 1 object(s) allocated from: #0 0x7fb536330986 in __interceptor_realloc ../../../../src/libsanitizer/lsan/lsan_interceptors.cpp:98 #1 0x55e07606cbf9 in xrealloc wrapper.c:140 #2 0x55e075fb6cb3 in prio_queue_put prio-queue.c:42 #3 0x55e075ec81cb in get_reachable_subset commit-reach.c:917 #4 0x55e075fe9cce in add_missing_tags remote.c:1518 #5 0x55e075fea1e4 in match_push_refs remote.c:1665 git#6 0x55e076050a8e in transport_push transport.c:1378 #7 0x55e075e2eb74 in push_with_options builtin/push.c:401 git#8 0x55e075e2edb0 in do_push builtin/push.c:458 git#9 0x55e075e2ff7a in cmd_push builtin/push.c:702 git#10 0x55e075d8aaf0 in run_builtin git.c:452 git#11 0x55e075d8af08 in handle_builtin git.c:706 git#12 0x55e075d8b12c in run_argv git.c:770 git#13 0x55e075d8b6a0 in cmd_main git.c:905 git#14 0x55e075e81f07 in main common-main.c:60 git#15 0x7fb5360ab6c9 in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58 git#16 0x7fb5360ab784 in __libc_start_main_impl ../csu/libc-start.c:360 git#17 0x55e075d88f40 in _start (git+0x1ff40) (BuildId: 38ad998b85a535e786129979443630d025ec2453) SUMMARY: LeakSanitizer: 384 byte(s) leaked in 1 allocation(s). This leak was addressed independently via 68b5117 (commit-reach: fix memory leak in get_reachable_subset(), 2023-06-03), which makes t5583 leak-free. But t5583 was not in the tree when 68b5117 was written, and the two only met after the latter was merged back in via 693bde4 (Merge branch 'mh/commit-reach-get-reachable-plug-leak', 2023-06-20). At that point, t5583 was leak-free. Let's mark it as such accordingly. Signed-off-by: Taylor Blau <me@ttaylorr.com>
ttaylorr
added a commit
to ttaylorr/git
that referenced
this pull request
Aug 24, 2023
When t5583-push-branches.sh was originally introduced via 425b4d7 (push: introduce '--branches' option, 2023-05-06), it was not leak-free. In fact, the test did not even run correctly until 022fbb6 (t5583: fix shebang line, 2023-05-12), but after applying that patch, we see a failure at t5583.8: ==2529087==ERROR: LeakSanitizer: detected memory leaks Direct leak of 384 byte(s) in 1 object(s) allocated from: #0 0x7fb536330986 in __interceptor_realloc ../../../../src/libsanitizer/lsan/lsan_interceptors.cpp:98 #1 0x55e07606cbf9 in xrealloc wrapper.c:140 #2 0x55e075fb6cb3 in prio_queue_put prio-queue.c:42 #3 0x55e075ec81cb in get_reachable_subset commit-reach.c:917 #4 0x55e075fe9cce in add_missing_tags remote.c:1518 #5 0x55e075fea1e4 in match_push_refs remote.c:1665 git#6 0x55e076050a8e in transport_push transport.c:1378 #7 0x55e075e2eb74 in push_with_options builtin/push.c:401 git#8 0x55e075e2edb0 in do_push builtin/push.c:458 git#9 0x55e075e2ff7a in cmd_push builtin/push.c:702 git#10 0x55e075d8aaf0 in run_builtin git.c:452 git#11 0x55e075d8af08 in handle_builtin git.c:706 git#12 0x55e075d8b12c in run_argv git.c:770 git#13 0x55e075d8b6a0 in cmd_main git.c:905 git#14 0x55e075e81f07 in main common-main.c:60 git#15 0x7fb5360ab6c9 in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58 git#16 0x7fb5360ab784 in __libc_start_main_impl ../csu/libc-start.c:360 git#17 0x55e075d88f40 in _start (git+0x1ff40) (BuildId: 38ad998b85a535e786129979443630d025ec2453) SUMMARY: LeakSanitizer: 384 byte(s) leaked in 1 allocation(s). This leak was addressed independently via 68b5117 (commit-reach: fix memory leak in get_reachable_subset(), 2023-06-03), which makes t5583 leak-free. But t5583 was not in the tree when 68b5117 was written, and the two only met after the latter was merged back in via 693bde4 (Merge branch 'mh/commit-reach-get-reachable-plug-leak', 2023-06-20). At that point, t5583 was leak-free. Let's mark it as such accordingly. Signed-off-by: Taylor Blau <me@ttaylorr.com>
ttaylorr
added a commit
to ttaylorr/git
that referenced
this pull request
Aug 24, 2023
When t5583-push-branches.sh was originally introduced via 425b4d7 (push: introduce '--branches' option, 2023-05-06), it was not leak-free. In fact, the test did not even run correctly until 022fbb6 (t5583: fix shebang line, 2023-05-12), but after applying that patch, we see a failure at t5583.8: ==2529087==ERROR: LeakSanitizer: detected memory leaks Direct leak of 384 byte(s) in 1 object(s) allocated from: #0 0x7fb536330986 in __interceptor_realloc ../../../../src/libsanitizer/lsan/lsan_interceptors.cpp:98 #1 0x55e07606cbf9 in xrealloc wrapper.c:140 #2 0x55e075fb6cb3 in prio_queue_put prio-queue.c:42 #3 0x55e075ec81cb in get_reachable_subset commit-reach.c:917 #4 0x55e075fe9cce in add_missing_tags remote.c:1518 #5 0x55e075fea1e4 in match_push_refs remote.c:1665 git#6 0x55e076050a8e in transport_push transport.c:1378 #7 0x55e075e2eb74 in push_with_options builtin/push.c:401 git#8 0x55e075e2edb0 in do_push builtin/push.c:458 git#9 0x55e075e2ff7a in cmd_push builtin/push.c:702 git#10 0x55e075d8aaf0 in run_builtin git.c:452 git#11 0x55e075d8af08 in handle_builtin git.c:706 git#12 0x55e075d8b12c in run_argv git.c:770 git#13 0x55e075d8b6a0 in cmd_main git.c:905 git#14 0x55e075e81f07 in main common-main.c:60 git#15 0x7fb5360ab6c9 in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58 git#16 0x7fb5360ab784 in __libc_start_main_impl ../csu/libc-start.c:360 git#17 0x55e075d88f40 in _start (git+0x1ff40) (BuildId: 38ad998b85a535e786129979443630d025ec2453) SUMMARY: LeakSanitizer: 384 byte(s) leaked in 1 allocation(s). This leak was addressed independently via 68b5117 (commit-reach: fix memory leak in get_reachable_subset(), 2023-06-03), which makes t5583 leak-free. But t5583 was not in the tree when 68b5117 was written, and the two only met after the latter was merged back in via 693bde4 (Merge branch 'mh/commit-reach-get-reachable-plug-leak', 2023-06-20). At that point, t5583 was leak-free. Let's mark it as such accordingly. Signed-off-by: Taylor Blau <me@ttaylorr.com>
gitster
pushed a commit
that referenced
this pull request
Aug 24, 2023
When t5583-push-branches.sh was originally introduced via 425b4d7 (push: introduce '--branches' option, 2023-05-06), it was not leak-free. In fact, the test did not even run correctly until 022fbb6 (t5583: fix shebang line, 2023-05-12), but after applying that patch, we see a failure at t5583.8: ==2529087==ERROR: LeakSanitizer: detected memory leaks Direct leak of 384 byte(s) in 1 object(s) allocated from: #0 0x7fb536330986 in __interceptor_realloc ../../../../src/libsanitizer/lsan/lsan_interceptors.cpp:98 #1 0x55e07606cbf9 in xrealloc wrapper.c:140 #2 0x55e075fb6cb3 in prio_queue_put prio-queue.c:42 #3 0x55e075ec81cb in get_reachable_subset commit-reach.c:917 #4 0x55e075fe9cce in add_missing_tags remote.c:1518 #5 0x55e075fea1e4 in match_push_refs remote.c:1665 #6 0x55e076050a8e in transport_push transport.c:1378 #7 0x55e075e2eb74 in push_with_options builtin/push.c:401 #8 0x55e075e2edb0 in do_push builtin/push.c:458 #9 0x55e075e2ff7a in cmd_push builtin/push.c:702 #10 0x55e075d8aaf0 in run_builtin git.c:452 #11 0x55e075d8af08 in handle_builtin git.c:706 #12 0x55e075d8b12c in run_argv git.c:770 #13 0x55e075d8b6a0 in cmd_main git.c:905 #14 0x55e075e81f07 in main common-main.c:60 #15 0x7fb5360ab6c9 in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58 #16 0x7fb5360ab784 in __libc_start_main_impl ../csu/libc-start.c:360 #17 0x55e075d88f40 in _start (git+0x1ff40) (BuildId: 38ad998b85a535e786129979443630d025ec2453) SUMMARY: LeakSanitizer: 384 byte(s) leaked in 1 allocation(s). This leak was addressed independently via 68b5117 (commit-reach: fix memory leak in get_reachable_subset(), 2023-06-03), which makes t5583 leak-free. But t5583 was not in the tree when 68b5117 was written, and the two only met after the latter was merged back in via 693bde4 (Merge branch 'mh/commit-reach-get-reachable-plug-leak', 2023-06-20). At that point, t5583 was leak-free. Let's mark it as such accordingly. Signed-off-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
ttaylorr
added a commit
to ttaylorr/git
that referenced
this pull request
Aug 25, 2023
When t5583-push-branches.sh was originally introduced via 425b4d7 (push: introduce '--branches' option, 2023-05-06), it was not leak-free. In fact, the test did not even run correctly until 022fbb6 (t5583: fix shebang line, 2023-05-12), but after applying that patch, we see a failure at t5583.8: ==2529087==ERROR: LeakSanitizer: detected memory leaks Direct leak of 384 byte(s) in 1 object(s) allocated from: #0 0x7fb536330986 in __interceptor_realloc ../../../../src/libsanitizer/lsan/lsan_interceptors.cpp:98 #1 0x55e07606cbf9 in xrealloc wrapper.c:140 #2 0x55e075fb6cb3 in prio_queue_put prio-queue.c:42 #3 0x55e075ec81cb in get_reachable_subset commit-reach.c:917 #4 0x55e075fe9cce in add_missing_tags remote.c:1518 #5 0x55e075fea1e4 in match_push_refs remote.c:1665 git#6 0x55e076050a8e in transport_push transport.c:1378 #7 0x55e075e2eb74 in push_with_options builtin/push.c:401 git#8 0x55e075e2edb0 in do_push builtin/push.c:458 git#9 0x55e075e2ff7a in cmd_push builtin/push.c:702 git#10 0x55e075d8aaf0 in run_builtin git.c:452 git#11 0x55e075d8af08 in handle_builtin git.c:706 git#12 0x55e075d8b12c in run_argv git.c:770 git#13 0x55e075d8b6a0 in cmd_main git.c:905 git#14 0x55e075e81f07 in main common-main.c:60 git#15 0x7fb5360ab6c9 in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58 git#16 0x7fb5360ab784 in __libc_start_main_impl ../csu/libc-start.c:360 git#17 0x55e075d88f40 in _start (git+0x1ff40) (BuildId: 38ad998b85a535e786129979443630d025ec2453) SUMMARY: LeakSanitizer: 384 byte(s) leaked in 1 allocation(s). This leak was addressed independently via 68b5117 (commit-reach: fix memory leak in get_reachable_subset(), 2023-06-03), which makes t5583 leak-free. But t5583 was not in the tree when 68b5117 was written, and the two only met after the latter was merged back in via 693bde4 (Merge branch 'mh/commit-reach-get-reachable-plug-leak', 2023-06-20). At that point, t5583 was leak-free. Let's mark it as such accordingly. Signed-off-by: Taylor Blau <me@ttaylorr.com>
ttaylorr
added a commit
to ttaylorr/git
that referenced
this pull request
Aug 28, 2023
When t5583-push-branches.sh was originally introduced via 425b4d7 (push: introduce '--branches' option, 2023-05-06), it was not leak-free. In fact, the test did not even run correctly until 022fbb6 (t5583: fix shebang line, 2023-05-12), but after applying that patch, we see a failure at t5583.8: ==2529087==ERROR: LeakSanitizer: detected memory leaks Direct leak of 384 byte(s) in 1 object(s) allocated from: #0 0x7fb536330986 in __interceptor_realloc ../../../../src/libsanitizer/lsan/lsan_interceptors.cpp:98 #1 0x55e07606cbf9 in xrealloc wrapper.c:140 #2 0x55e075fb6cb3 in prio_queue_put prio-queue.c:42 #3 0x55e075ec81cb in get_reachable_subset commit-reach.c:917 #4 0x55e075fe9cce in add_missing_tags remote.c:1518 #5 0x55e075fea1e4 in match_push_refs remote.c:1665 git#6 0x55e076050a8e in transport_push transport.c:1378 #7 0x55e075e2eb74 in push_with_options builtin/push.c:401 git#8 0x55e075e2edb0 in do_push builtin/push.c:458 git#9 0x55e075e2ff7a in cmd_push builtin/push.c:702 git#10 0x55e075d8aaf0 in run_builtin git.c:452 git#11 0x55e075d8af08 in handle_builtin git.c:706 git#12 0x55e075d8b12c in run_argv git.c:770 git#13 0x55e075d8b6a0 in cmd_main git.c:905 git#14 0x55e075e81f07 in main common-main.c:60 git#15 0x7fb5360ab6c9 in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58 git#16 0x7fb5360ab784 in __libc_start_main_impl ../csu/libc-start.c:360 git#17 0x55e075d88f40 in _start (git+0x1ff40) (BuildId: 38ad998b85a535e786129979443630d025ec2453) SUMMARY: LeakSanitizer: 384 byte(s) leaked in 1 allocation(s). This leak was addressed independently via 68b5117 (commit-reach: fix memory leak in get_reachable_subset(), 2023-06-03), which makes t5583 leak-free. But t5583 was not in the tree when 68b5117 was written, and the two only met after the latter was merged back in via 693bde4 (Merge branch 'mh/commit-reach-get-reachable-plug-leak', 2023-06-20). At that point, t5583 was leak-free. Let's mark it as such accordingly. Signed-off-by: Taylor Blau <me@ttaylorr.com>
gitster
pushed a commit
that referenced
this pull request
Aug 29, 2023
When t5583-push-branches.sh was originally introduced via 425b4d7 (push: introduce '--branches' option, 2023-05-06), it was not leak-free. In fact, the test did not even run correctly until 022fbb6 (t5583: fix shebang line, 2023-05-12), but after applying that patch, we see a failure at t5583.8: ==2529087==ERROR: LeakSanitizer: detected memory leaks Direct leak of 384 byte(s) in 1 object(s) allocated from: #0 0x7fb536330986 in __interceptor_realloc ../../../../src/libsanitizer/lsan/lsan_interceptors.cpp:98 #1 0x55e07606cbf9 in xrealloc wrapper.c:140 #2 0x55e075fb6cb3 in prio_queue_put prio-queue.c:42 #3 0x55e075ec81cb in get_reachable_subset commit-reach.c:917 #4 0x55e075fe9cce in add_missing_tags remote.c:1518 #5 0x55e075fea1e4 in match_push_refs remote.c:1665 #6 0x55e076050a8e in transport_push transport.c:1378 #7 0x55e075e2eb74 in push_with_options builtin/push.c:401 #8 0x55e075e2edb0 in do_push builtin/push.c:458 #9 0x55e075e2ff7a in cmd_push builtin/push.c:702 #10 0x55e075d8aaf0 in run_builtin git.c:452 #11 0x55e075d8af08 in handle_builtin git.c:706 #12 0x55e075d8b12c in run_argv git.c:770 #13 0x55e075d8b6a0 in cmd_main git.c:905 #14 0x55e075e81f07 in main common-main.c:60 #15 0x7fb5360ab6c9 in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58 #16 0x7fb5360ab784 in __libc_start_main_impl ../csu/libc-start.c:360 #17 0x55e075d88f40 in _start (git+0x1ff40) (BuildId: 38ad998b85a535e786129979443630d025ec2453) SUMMARY: LeakSanitizer: 384 byte(s) leaked in 1 allocation(s). This leak was addressed independently via 68b5117 (commit-reach: fix memory leak in get_reachable_subset(), 2023-06-03), which makes t5583 leak-free. But t5583 was not in the tree when 68b5117 was written, and the two only met after the latter was merged back in via 693bde4 (Merge branch 'mh/commit-reach-get-reachable-plug-leak', 2023-06-20). At that point, t5583 was leak-free. Let's mark it as such accordingly. Signed-off-by: Taylor Blau <me@ttaylorr.com> Acked-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
spectral54
added a commit
to spectral54/git
that referenced
this pull request
Jun 12, 2024
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>
gitster
pushed a commit
that referenced
this pull request
Jun 17, 2024
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>
gitster
pushed a commit
that referenced
this pull request
Aug 19, 2024
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>
gitster
pushed a commit
that referenced
this pull request
Aug 22, 2024
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>
gitster
pushed a commit
that referenced
this pull request
Aug 23, 2024
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>
gitster
pushed a commit
that referenced
this pull request
Dec 30, 2024
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>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.