Fixes for the new SDK #1

t-b · 2014-09-03T21:26:15Z

The commits which are in sschuberth/git and not in here are (pasted from private mail from @sschuberth):

75e415e Makefile: Do not install built-ins
ffb434c MinGW: Use MakeMaker to build the Perl libraries
617433c Documentation: make AsciiDoc links always point to HTML files (*)
b543717 MSVC: fix stat definition hell (*)
296f510 MinGW: Fix stat definitions to work with MinGW runtime version 4.0 (*)
72631c9 mailmap: work around implementations with pure inline strcasecmp (*)
ab94cf6 Windows: do not redefine _WIN32_WINNT (*)
d892bec t0061: Work around a line endings issue with newer versions of
cat on MSYS
adfa3c9 Makefile: Set htmldir to match the default HTML docs location under MSYS
159ea1a t7800: Use "test_cmp_text" in all places where "echo" is used

The starred ones (*) are supposedly already upstream.
I picked the necessary ones here.

The omission of 75e415e is on purpose as that feature is not ready yet.

This way the libraries get properly installed into the "site_perl" directory and we just have to move them out of the "mingw" directory. Signed-off-by: Sebastian Schuberth <sschuberth@gmail.com>

Signed-off-by: Sebastian Schuberth <sschuberth@gmail.com>

While "echo", being a Bash built-in, produces Unix line-endings, "git" and "cat" produce DOS line-endings on MSYS, so ignore line-ending differences. Signed-off-by: Sebastian Schuberth <sschuberth@gmail.com>

… MSYS While msysgit uses cat 2.0, mingwGitDevEnv uses cat 5.97, currently. A change part of the latter version [1] makes cat use the "same rules as other programs to decide whether to use binary I/O". As a result, cat 5.97 behaves like e.g. awk with respect to which line endings are used if stdout is redirected to a file, and those are DOS line endings. Ignore line endings when comparing the test result to work around this. [1] http://git.savannah.gnu.org/gitweb/?p=coreutils.git;a=commit;h=8770c00ef45e3c6c4dd3d5ce1e55a6fb1adb13dc Signed-off-by: Thomas Braun <thomas.braun@virtuell-zuhause.de> Signed-off-by: Sebastian Schuberth <sschuberth@gmail.com>

dscho · 2014-09-04T12:46:39Z

Looks good to me! @sschuberth would you mind if I merged?

sschuberth · 2014-09-04T12:53:47Z

Just a question to double check: Did you actually cherry-pick the commits that are supposed to be upstream, to verify that they really are (they should either result in a no-op or minor conflict, if upstream has a slightly reworked patch applied)?

sschuberth · 2014-09-04T12:57:32Z

Nevermind, I just verified that the starred commits are indeed upstream.

Fixes for the new SDK

t-b · 2014-09-04T16:00:02Z

Thanks for merging.
@sschuberth Got me! No I did not let cherry-pick discover that for. Will do that next time :)

Fixes for the new SDK

The main loop in strbuf_utf8_replace() could summed up as: while ('src' is still valid) { 1) advance 'src' to copy ANSI escape sequences 2) advance 'src' to copy/replace visible characters } The problem is after #1, 'src' may have reached the end of the string (so 'src' points to NUL) and #2 will continue to copy that NUL as if it's a normal character. Because the output is stored in a strbuf, this NUL accounted in the 'len' field as well. Check after #1 and break the loop if necessary. The test does not look obvious, but the combination of %>>() should make a call trace like this show_log() pretty_print_commit() format_commit_message() strbuf_expand() format_commit_item() format_and_pad_commit() strbuf_utf8_replace() where %C(auto)%d would insert a color reset escape sequence in the end of the string given to strbuf_utf8_replace() and show_log() uses fwrite() to send everything to stdout (including the incorrect NUL inserted by strbuf_utf8_replace) Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Don't chop test_expect_success line into pieces and concatenate with '\'. That's so 2005. Signed-off-by: Junio C Hamano <gitster@pobox.com>

* jc/t9001-modernise: t9001: style modernisation phase #5 t9001: style modernisation phase #4 t9001: style modernisation phase #3 t9001: style modernisation phase #2 t9001: style modernisation phase #1

Signed-off-by: Junio C Hamano <gitster@pobox.com>

The collect_parents() function now is responsible for 1. parsing the commits given on the command line into a list of commits to be merged; 2. filtering these parents into independent ones; and 3. optionally calling fmt_merge_msg() via prepare_merge_message() to prepare an auto-generated merge log message, using fake contents that FETCH_HEAD would have had if these commits were fetched from the current repository with "git pull . $args..." Make "git merge FETCH_HEAD" to be the same as the traditional git merge "$(git fmt-merge-msg <.git/FETCH_HEAD)" $commits invocation of the command in "git pull", where $commits are the ones that appear in FETCH_HEAD that are not marked as not-for-merge, by making it do a bit more, specifically: - noticing "FETCH_HEAD" is the only "commit" on the command line and picking the commits that are not marked as not-for-merge as the list of commits to be merged (substitute for step #1 above); - letting the resulting list fed to step #2 above; - doing the step #3 above, using the contents of the FETCH_HEAD instead of fake contents crafted from the list of commits parsed in the step #1 above. Note that this changes the semantics. "git merge FETCH_HEAD" has always behaved as if the first commit in the FETCH_HEAD file were directly specified on the command line, creating a two-way merge whose auto-generated merge log said "merge commit xyz". With this change, if the previous fetch was to grab multiple branches (e.g. "git fetch $there topic-a topic-b"), the new world order is to create an octopus, behaving as if "git pull $there topic-a topic-b" were run. This is a deliberate change to make that happen, and can be seen in the changes to t3033 tests. Signed-off-by: Junio C Hamano <gitster@pobox.com>

When ac49f5c (rerere "remaining", 2011-02-16) split out a new helper function check_one_conflict() out of find_conflict() function, so that the latter will use the returned value from the new helper to update the loop control variable that is an index into active_cache[], the new variable incremented the index by one too many when it found a path with only stage #1 entry at the very end of active_cache[]. This "strange" return value does not have any effect on the loop control of two callers of this function, as they all notice that active_nr+2 is larger than active_nr just like active_nr+1 is, but nevertheless it puzzles the readers when they are trying to figure out what the function is trying to do. In fact, there is no need to do an early return. The code that follows after skipping the stage #1 entry is fully prepared to handle a case where the entry is at the very end of active_cache[]. Help future readers from unnecessary confusion by dropping an early return. We skip the stage #1 entry, and if there are stage #2 and stage git-for-windows#3 entries for the same path, we diagnose the path as THREE_STAGED (otherwise we say PUNTED), and then we skip all entries for the same path. Signed-off-by: Junio C Hamano <gitster@pobox.com>

A conflicted index can have multiple stage #1 entries when dealing with a criss-cross merge and using the "resolve" merge strategy. Signed-off-by: Junio C Hamano <gitster@pobox.com>

A conflicted index can have multiple stage #1 entries when dealing with a criss-cross merge and using the "resolve" merge strategy. Plug the leak by reading only the first one of the same stage entries. Strictly speaking, this fix does change the semantics, in that we used to use the last stage #1 entry as the common ancestor when doing the plain-vanilla three-way merge, but with the leak fix, we will use the first stage #1 entry. But it is not a grave backward compatibility breakage. Either way, we are arbitrarily picking one of multiple stage #1 entries and using it, ignoring others, and there is no meaning in the ordering of these stage #1 entries. Signed-off-by: Junio C Hamano <gitster@pobox.com>

Code clean-up and minor fixes. * jc/rerere: (21 commits) rerere: un-nest merge() further rerere: use "struct rerere_id" instead of "char *" for conflict ID rerere: call conflict-ids IDs rerere: further clarify do_rerere_one_path() rerere: further de-dent do_plain_rerere() rerere: refactor "replay" part of do_plain_rerere() rerere: explain the remainder rerere: explain "rerere forget" codepath rerere: explain the primary codepath rerere: explain MERGE_RR management helpers rerere: fix benign off-by-one non-bug and clarify code rerere: explain the rerere I/O abstraction rerere: do not leak mmfile[] for a path with multiple stage #1 entries rerere: stop looping unnecessarily rerere: drop want_sp parameter from is_cmarker() rerere: report autoupdated paths only after actually updating them rerere: write out each record of MERGE_RR in one go rerere: lift PATH_MAX limitation rerere: plug conflict ID leaks rerere: handle conflicts with multiple stage #1 entries ...

We generate the squash commit message incrementally running a sed script once for each commit. It parses "This is a combination of <N> commits" from the first line of the existing message, adds one to <N>, and uses the result as the number of our current message. Since f2d1706 (i18n: rebase-interactive: mark comments of squash for translation, 2016-06-17), the first line may be localized, and sed uses a pretty liberal regex, looking for: /^#.*([0-9][0-9]*)/ The "[0-9][0-9]*" tries to match double digits, but it doesn't quite work. The first ".*" is greedy, so if you have: This is a combination of 10 commits. it will eat up "This is a combination of 1", leaving "0" to match the first "[0-9]" digit, and then skipping the optional match of "[0-9]*". As a result, the count resets every 10 commits, and a 15-commit squash would end up as: # This is a combination of 5 commits. # This is the 1st commit message: ... # This is the commit message #2: ... and so on .. # This is the commit message #10: ... # This is the commit message #1: ... # This is the commit message #2: ... etc, up to 5 ... We can fix this by making the ".*" less greedy. Instead of depending on ".*?" working portably, we can just limit the match to non-digit characters, which accomplishes the same thing. Reported-by: Brandon Tolsch <btolsch@gmail.com> Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Amend my change earlier in this series ("grep: add support for the PCRE v1 JIT API", 2017-04-11) to un-break the build on PCRE v1 versions later than 8.31 compiled without --enable-jit. As explained in that change and a later compatibility change in this series ("grep: un-break building with PCRE < 8.32", 2017-05-10) the pcre_jit_exec() function is a faster path to execute the JIT. Unfortunately there's no compatibility stub for that function compiled into the library if pcre_config(PCRE_CONFIG_JIT, &ret) would return 0, and no macro that can be used to check for it, so the only portable option to support builds without --enable-jit is via a new NO_LIBPCRE1_JIT=UnfortunatelyYes Makefile option[1]. Another option would be to make the JIT opt-in via USE_LIBPCRE1_JIT=YesPlease, after all it's not a default option of PCRE v1. I think it makes more sense to make it opt-out since even though it's not a default option, most packagers of PCRE seem to turn it on by default, with the notable exception of the MinGW package. Make the MinGW platform work by default by changing the build defaults to turn on NO_LIBPCRE1_JIT=UnfortunatelyYes. It is the only platform that turns on USE_LIBPCRE=YesPlease by default, see commit df5218b ("config.mak.uname: support MSys2", 2016-01-13) for that change. 1. "How do I support pcre1 JIT on all versions?" (https://lists.exim.org/lurker/thread/20170601.103148.10253788.en.html) 2. https://github.com/Alexpux/MINGW-packages/blob/master/mingw-w64-pcre/PKGBUILD (referenced from "Re: PCRE v2 compile error, was Re: What's cooking in git.git (May 2017, #1; Mon, 1)"; <alpine.DEB.2.20.1705021756530.3480@virtualbox>) Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

We turn off ASan's leak detection by default in the test suite because it's too noisy. But we don't do so until part-way through test-lib. This is before we've run any tests, but after we do our initial "./git" to see if the binary has even been built. When built with clang, this seems to work fine. However, using "gcc -fsanitize=address", the leak checker seems to complain more aggressively: $ ./git ... ==5352==ERROR: LeakSanitizer: detected memory leaks Direct leak of 2 byte(s) in 1 object(s) allocated from: #0 0x7f120e7afcf8 in malloc (/usr/lib/x86_64-linux-gnu/libasan.so.3+0xc1cf8) #1 0x559fc2a3ce41 in do_xmalloc /home/peff/compile/git/wrapper.c:60 #2 0x559fc2a3cf1a in do_xmallocz /home/peff/compile/git/wrapper.c:100 #3 0x559fc2a3d0ad in xmallocz /home/peff/compile/git/wrapper.c:108 #4 0x559fc2a3d0ad in xmemdupz /home/peff/compile/git/wrapper.c:124 #5 0x559fc2a3d0ad in xstrndup /home/peff/compile/git/wrapper.c:130 #6 0x559fc274535a in main /home/peff/compile/git/common-main.c:39 #7 0x7f120dabd2b0 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x202b0) This is a leak in the sense that we never free it, but it's in a global that is meant to last the whole program. So it's not really interesting or in need of fixing. And at any rate, mentioning leaks outside of the test_expect blocks is certainly unwelcome, as it pollutes stderr. Let's bump the setting of ASAN_OPTIONS higher in test-lib.sh to catch our initial "can we even run git?" test. While we're at it, we can add a comment to make it a bit less inscrutable. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>

In some setups, old-style submodules (i.e. the ones with .git directory within theirs worktrees) with commondir can be of tremendous help. For example, commondir link can be used to avoid duplication of objects and also to keep branches in sync with multiple copies of the repo's worktree, while keeping the .git directory inside the worktree can be (ab?-)used to exploit the sharing of the same submodule worktree across different projects (this at least works on Windows with submodule directory being a directory junction, but having a junction is not relevant for reproducing the bug described below). Unfortunately, at the moment, when `git status` is run in the root repo of such a setup, it gives an output akin to this: ```sh fatal: unable to access '�??\1?/config': Invalid argument fatal: 'git status --porcelain=2' failed in submodule commonlibs ``` where `�??\1?` part of '�??\1?/config' varies from run to run, and `commonlibs` is the name of submodule's directory. Currently, when Git discovers old-style submodule , it spawns subprocess to get its status, like this one: ```sh cd commonlibs; unset GIT_PREFIX; GIT_DIR=.git git status --porcelain=2 ``` Unsurprisingly, the following output is also quite unexpected: ``` fatal: unable to access '`??L&?/config': Invalid argument ``` The core reason for these is that global repository field for commondir is not being cleared to `NULL` after being `free()`'d in `repo_set_commondir()`, which is precisely what this commit fixes. Regarding the further details of the case of investigation, this value of struct pointed by the global `the_repository` pointer is checked for being not-NULL down in the callstack in compatibility layer for MinGW in a function that is called by `repo_set_commondir()` before the `free()`'d value gets assigned in its body (i.e. the body of `repo_set_commondir()`). Backtrace from the check is: ``` #0 mingw_open (filename=0x<address-25> ".git/commondir", oflags=0) at compat/mingw.c:784 git-for-windows#1 0x<address-27> in strbuf_read_file (sb=0x<address-26>, path=0x<address-25> ".git/commondir", hint=0) at strbuf.c:758 git-for-windows#2 0x<address-24> in get_common_dir_noenv (sb=0x<address-23>, gitdir=0x<address-22> ".git") at setup.c:313 git-for-windows#3 0x<address-21> in repo_set_commondir (repo=0x<address-19> <the_repo>, commondir=0x0) at repository.c:57 git-for-windows#4 0x<address-20> in repo_set_gitdir (repo=0x<address-19> <the_repo>, root=0x<address-15> ".git", o=0x<address-18>) at repository.c:76 git-for-windows#5 0x<address-17> in setup_git_env (git_dir=0x<address-15> ".git") at environment.c:179 git-for-windows#6 0x<address-16> in set_git_dir_1 (path=0x<address-15> ".git") at environment.c:334 git-for-windows#7 0x<address-14> in update_relative_gitdir (name=0x0, old_cwd=0x<address-13> "C:/Users/%username%/<root-repo-name>/commonlibs", new_cwd=0x<address-11> "C:/Users/%username%/<root-repo-name>/commonlibs", data=0x0) at environment.c:348 git-for-windows#8 0x<address-12> in chdir_notify ( new_cwd=0x<address-11> "C:/Users/%username%/<root-repo-name>/commonlibs") at chdir-notify.c:72 git-for-windows#9 0x<address-10> in setup_work_tree () at setup.c:428 git-for-windows#10 0x<address-9> in run_builtin (p=0x<address-8> <commands+2856>, argc=2, argv=0x<address-2>) at git.c:458 git-for-windows#11 0x<address-7> in handle_builtin (argc=2, argv=0x<address-2>) at git.c:721 git-for-windows#12 0x<address-6> in run_argv (argcp=0x<address-5>, argv=0x<address-4>) at git.c:788 git-for-windows#13 0x<address-3> in cmd_main (argc=2, argv=0x<address-2>) at git.c:921 git-for-windows#14 0x<address-1> in main (argc=6, argv=0x<address-0>) at common-main.c:56 ``` Backtrace from the death is: ``` #0 die_errno (fmt=0x<address-42> <result_type+2002> "unable to access '%s'") at usage.c:210 git-for-windows#1 0x<address-41> in access_or_die ( path=0x<address-40> "`\001\r��\004/config", mode=4, flag=0) at wrapper.c:667 git-for-windows#2 0x<address-39> in do_git_config_sequence (opts=0x<address-35>, fn=0x<address-37> <git_config_include>, data=0x<address-36>) at config.c:2142 git-for-windows#3 0x<address-38> in config_with_options ( fn=0x<address-37> <git_config_include>, data=0x<address-36>, config_source=0x0, opts=0x<address-35>) at config.c:2198 git-for-windows#4 0x<address-34> in repo_read_config (repo=0x<address-19> <the_repo>) at config.c:2524 git-for-windows#5 0x<address-33> in git_config_check_init ( repo=0x<address-19> <the_repo>) at config.c:2543 git-for-windows#6 0x<address-32> in repo_config_get_bool ( repo=0x<address-19> <the_repo>, key=0x<address-30> <pad+3116> "windows.appendatomically", dest=0x<address-29> <append_atomically>) at config.c:2612 git-for-windows#7 0x<address-31> in git_config_get_bool ( key=0x<address-30> <pad+3116> "windows.appendatomically", dest=0x<address-29> <append_atomically>) at config.c:2714 git-for-windows#8 0x<address-28> in mingw_open ( filename=0x<address-25> ".git/commondir", oflags=0) at compat/mingw.c:785 git-for-windows#9 0x<address-27> in strbuf_read_file (sb=0x<address-26>, path=0x<address-25> ".git/commondir", hint=0) at strbuf.c:758 git-for-windows#10 0x<address-24> in get_common_dir_noenv (sb=0x<address-23>, gitdir=0x<address-22> ".git") at setup.c:313 git-for-windows#11 0x<address-21> in repo_set_commondir (repo=0x<address-19> <the_repo>, commondir=0x0) at repository.c:57 git-for-windows#12 0x<address-20> in repo_set_gitdir (repo=0x<address-19> <the_repo>, root=0x<address-15> ".git", o=0x<address-18>) at repository.c:76 git-for-windows#13 0x<address-17> in setup_git_env (git_dir=0x<address-15> ".git") at environment.c:179 git-for-windows#14 0x<address-16> in set_git_dir_1 (path=0x<address-15> ".git") at environment.c:334 git-for-windows#15 0x<address-14> in update_relative_gitdir (name=0x0, old_cwd=0x<address-13> "C:/Users/%username%/<root-repo-name>/commonlibs", new_cwd=0x<address-11> "C:/Users/%username%/<root-repo-name>/commonlibs", data=0x0) at environment.c:348 git-for-windows#16 0x<address-12> in chdir_notify ( new_cwd=0x<address-11> "C:/Users/%username%/<root-repo-name>/commonlibs") at chdir-notify.c:72 git-for-windows#17 0x<address-10> in setup_work_tree () at setup.c:428 git-for-windows#18 0x<address-9> in run_builtin (p=0x<address-8> <commands+2856>, argc=2, argv=0x<address-2>) at git.c:458 git-for-windows#19 0x<address-7> in handle_builtin (argc=2, argv=0x<address-2>) at git.c:721 git-for-windows#20 0x<address-6> in run_argv (argcp=0x<address-5>, argv=0x<address-4>) at git.c:788 git-for-windows#21 0x<address-3> in cmd_main (argc=2, argv=0x<address-2>) at git.c:921 git-for-windows#22 0x<address-1> in main (argc=6, argv=0x<address-0>) at common-main.c:56 ``` Signed-off-by: Andrey Zabavnikov <zabavnikov@gmail.com>

Add config option `windows.appendAtomically` Atomic append on windows is only supported on local disk files, and it may cause errors in other situations, e.g. network file system. If that is the case, this config option should be used to turn atomic append off. With these edits, status for old-style submodules with commondir needs to be fixed, due to the following. In some setups, old-style submodules (i.e. the ones with .git directory within theirs worktrees) with commondir can be of tremendous help. For example, commondir link can be used to avoid duplication of objects and also to keep branches in sync with multiple copies of the repo's worktree, while keeping the .git directory inside the worktree can be (ab?-)used to exploit the sharing of the same submodule worktree across different projects (this at least works on Windows with submodule directory being a directory junction, but having a junction is not relevant for reproducing the bug described below). Unfortunately, at the moment, when `git status` is run in the root repo of such a setup, it gives an output akin to this: ```sh fatal: unable to access '�??\1?/config': Invalid argument fatal: 'git status --porcelain=2' failed in submodule commonlibs ``` where `�??\1?` part of '�??\1?/config' varies from run to run, and `commonlibs` is the name of submodule's directory. Currently, when Git discovers old-style submodule , it spawns subprocess to get its status, like this one: ```sh cd commonlibs; unset GIT_PREFIX; GIT_DIR=.git git status --porcelain=2 ``` Unsurprisingly, the following output is also quite unexpected: ``` fatal: unable to access '`??L&?/config': Invalid argument ``` The core reason for these is that global repository field for commondir is not being cleared to `NULL` after being `free()`'d in `repo_set_commondir()`, which is precisely what this commit fixes. Regarding the further details of the case of investigation, this value of struct pointed by the global `the_repository` pointer is checked for being not-NULL down in the callstack in compatibility layer for MinGW in a function that is called by `repo_set_commondir()` before the `free()`'d value gets assigned in its body (i.e. the body of `repo_set_commondir()`). Backtrace from the check is: ``` #0 mingw_open (filename=0x<address-25> ".git/commondir", oflags=0) at compat/mingw.c:784 git-for-windows#1 0x<address-27> in strbuf_read_file (sb=0x<address-26>, path=0x<address-25> ".git/commondir", hint=0) at strbuf.c:758 git-for-windows#2 0x<address-24> in get_common_dir_noenv (sb=0x<address-23>, gitdir=0x<address-22> ".git") at setup.c:313 git-for-windows#3 0x<address-21> in repo_set_commondir (repo=0x<address-19> <the_repo>, commondir=0x0) at repository.c:57 git-for-windows#4 0x<address-20> in repo_set_gitdir (repo=0x<address-19> <the_repo>, root=0x<address-15> ".git", o=0x<address-18>) at repository.c:76 git-for-windows#5 0x<address-17> in setup_git_env (git_dir=0x<address-15> ".git") at environment.c:179 git-for-windows#6 0x<address-16> in set_git_dir_1 (path=0x<address-15> ".git") at environment.c:334 git-for-windows#7 0x<address-14> in update_relative_gitdir (name=0x0, old_cwd=0x<address-13> "C:/Users/%username%/<root-repo-name>/commonlibs", new_cwd=0x<address-11> "C:/Users/%username%/<root-repo-name>/commonlibs", data=0x0) at environment.c:348 git-for-windows#8 0x<address-12> in chdir_notify ( new_cwd=0x<address-11> "C:/Users/%username%/<root-repo-name>/commonlibs") at chdir-notify.c:72 git-for-windows#9 0x<address-10> in setup_work_tree () at setup.c:428 git-for-windows#10 0x<address-9> in run_builtin (p=0x<address-8> <commands+2856>, argc=2, argv=0x<address-2>) at git.c:458 git-for-windows#11 0x<address-7> in handle_builtin (argc=2, argv=0x<address-2>) at git.c:721 git-for-windows#12 0x<address-6> in run_argv (argcp=0x<address-5>, argv=0x<address-4>) at git.c:788 git-for-windows#13 0x<address-3> in cmd_main (argc=2, argv=0x<address-2>) at git.c:921 git-for-windows#14 0x<address-1> in main (argc=6, argv=0x<address-0>) at common-main.c:56 ``` Backtrace from the death is: ``` #0 die_errno (fmt=0x<address-42> <result_type+2002> "unable to access '%s'") at usage.c:210 git-for-windows#1 0x<address-41> in access_or_die ( path=0x<address-40> "`\001\r��\004/config", mode=4, flag=0) at wrapper.c:667 git-for-windows#2 0x<address-39> in do_git_config_sequence (opts=0x<address-35>, fn=0x<address-37> <git_config_include>, data=0x<address-36>) at config.c:2142 git-for-windows#3 0x<address-38> in config_with_options ( fn=0x<address-37> <git_config_include>, data=0x<address-36>, config_source=0x0, opts=0x<address-35>) at config.c:2198 git-for-windows#4 0x<address-34> in repo_read_config (repo=0x<address-19> <the_repo>) at config.c:2524 git-for-windows#5 0x<address-33> in git_config_check_init ( repo=0x<address-19> <the_repo>) at config.c:2543 git-for-windows#6 0x<address-32> in repo_config_get_bool ( repo=0x<address-19> <the_repo>, key=0x<address-30> <pad+3116> "windows.appendatomically", dest=0x<address-29> <append_atomically>) at config.c:2612 git-for-windows#7 0x<address-31> in git_config_get_bool ( key=0x<address-30> <pad+3116> "windows.appendatomically", dest=0x<address-29> <append_atomically>) at config.c:2714 git-for-windows#8 0x<address-28> in mingw_open ( filename=0x<address-25> ".git/commondir", oflags=0) at compat/mingw.c:785 git-for-windows#9 0x<address-27> in strbuf_read_file (sb=0x<address-26>, path=0x<address-25> ".git/commondir", hint=0) at strbuf.c:758 git-for-windows#10 0x<address-24> in get_common_dir_noenv (sb=0x<address-23>, gitdir=0x<address-22> ".git") at setup.c:313 git-for-windows#11 0x<address-21> in repo_set_commondir (repo=0x<address-19> <the_repo>, commondir=0x0) at repository.c:57 git-for-windows#12 0x<address-20> in repo_set_gitdir (repo=0x<address-19> <the_repo>, root=0x<address-15> ".git", o=0x<address-18>) at repository.c:76 git-for-windows#13 0x<address-17> in setup_git_env (git_dir=0x<address-15> ".git") at environment.c:179 git-for-windows#14 0x<address-16> in set_git_dir_1 (path=0x<address-15> ".git") at environment.c:334 git-for-windows#15 0x<address-14> in update_relative_gitdir (name=0x0, old_cwd=0x<address-13> "C:/Users/%username%/<root-repo-name>/commonlibs", new_cwd=0x<address-11> "C:/Users/%username%/<root-repo-name>/commonlibs", data=0x0) at environment.c:348 git-for-windows#16 0x<address-12> in chdir_notify ( new_cwd=0x<address-11> "C:/Users/%username%/<root-repo-name>/commonlibs") at chdir-notify.c:72 git-for-windows#17 0x<address-10> in setup_work_tree () at setup.c:428 git-for-windows#18 0x<address-9> in run_builtin (p=0x<address-8> <commands+2856>, argc=2, argv=0x<address-2>) at git.c:458 git-for-windows#19 0x<address-7> in handle_builtin (argc=2, argv=0x<address-2>) at git.c:721 git-for-windows#20 0x<address-6> in run_argv (argcp=0x<address-5>, argv=0x<address-4>) at git.c:788 git-for-windows#21 0x<address-3> in cmd_main (argc=2, argv=0x<address-2>) at git.c:921 git-for-windows#22 0x<address-1> in main (argc=6, argv=0x<address-0>) at common-main.c:56 ``` Co-Authored-By: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: 孙卓识 <sunzhuoshi@gmail.com> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Andrey Zabavnikov <zabavnikov@gmail.com>

Add config option `windows.appendAtomically` Atomic append on windows is only supported on local disk files, and it may cause errors in other situations, e.g. network file system. If that is the case, this config option should be used to turn atomic append off. With these edits, the status command for old-style submodules with commondir needs to be fixed, due to the following. In some setups, old-style submodules (i.e. the ones with .git directory within theirs worktrees) with commondir can be of tremendous help. For example, commondir link can be used to avoid duplication of objects and also to keep branches in sync with multiple copies of the repo's worktree, while keeping the .git directory inside the worktree can be (ab?-)used to exploit the sharing of the same submodule worktree across different projects (this at least works on Windows with submodule directory being a directory junction, but having a junction is not relevant for reproducing the bug described below). Unfortunately, after the addition of the new config option, when `git status` is run in the root repo of such a setup, it gives an output akin to this: ```sh $ git status fatal: unable to access '�??\1?/config': Invalid argument fatal: 'git status --porcelain=2' failed in submodule commonlibs ``` where `�??\1?` part of '�??\1?/config' varies from run to run, and `commonlibs` is the name of submodule's directory. Currently, when Git discovers old-style submodule , it spawns subprocess to get its status, like this one: ```sh cd commonlibs; unset GIT_PREFIX; GIT_DIR=.git git status --porcelain=2 ``` Unsurprisingly, the following output is also quite unexpected: ``` $ GIT_DIR=.git git -C commonlibs/ status --porcelain=2 fatal: unable to access '`??L&?/config': Invalid argument ``` The core reason for these is that global repository field for commondir is not being cleared to `NULL` after being `free()`'d in `repo_set_commondir()`, which is precisely what this commit fixes. Regarding the further details of the case of investigation, this value of struct pointed by the global `the_repository` pointer is checked for being not-NULL down in the callstack in compatibility layer for MinGW in a function that is called by `repo_set_commondir()` before the `free()`'d value gets assigned in its body (i.e. the body of `repo_set_commondir()`). Backtrace from the check is: ``` #0 mingw_open (filename=0x<address-25> ".git/commondir", oflags=0) at compat/mingw.c:784 git-for-windows#1 0x<address-27> in strbuf_read_file (sb=0x<address-26>, path=0x<address-25> ".git/commondir", hint=0) at strbuf.c:758 git-for-windows#2 0x<address-24> in get_common_dir_noenv (sb=0x<address-23>, gitdir=0x<address-22> ".git") at setup.c:313 git-for-windows#3 0x<address-21> in repo_set_commondir (repo=0x<address-19> <the_repo>, commondir=0x0) at repository.c:57 git-for-windows#4 0x<address-20> in repo_set_gitdir (repo=0x<address-19> <the_repo>, root=0x<address-15> ".git", o=0x<address-18>) at repository.c:76 git-for-windows#5 0x<address-17> in setup_git_env (git_dir=0x<address-15> ".git") at environment.c:179 git-for-windows#6 0x<address-16> in set_git_dir_1 (path=0x<address-15> ".git") at environment.c:334 git-for-windows#7 0x<address-14> in update_relative_gitdir (name=0x0, old_cwd=0x<address-13> "C:/Users/%username%/<root-repo-name>/commonlibs", new_cwd=0x<address-11> "C:/Users/%username%/<root-repo-name>/commonlibs", data=0x0) at environment.c:348 git-for-windows#8 0x<address-12> in chdir_notify ( new_cwd=0x<address-11> "C:/Users/%username%/<root-repo-name>/commonlibs") at chdir-notify.c:72 git-for-windows#9 0x<address-10> in setup_work_tree () at setup.c:428 git-for-windows#10 0x<address-9> in run_builtin (p=0x<address-8> <commands+2856>, argc=2, argv=0x<address-2>) at git.c:458 git-for-windows#11 0x<address-7> in handle_builtin (argc=2, argv=0x<address-2>) at git.c:721 git-for-windows#12 0x<address-6> in run_argv (argcp=0x<address-5>, argv=0x<address-4>) at git.c:788 git-for-windows#13 0x<address-3> in cmd_main (argc=2, argv=0x<address-2>) at git.c:921 git-for-windows#14 0x<address-1> in main (argc=6, argv=0x<address-0>) at common-main.c:56 ``` Backtrace from the death is: ``` #0 die_errno (fmt=0x<address-42> <result_type+2002> "unable to access '%s'") at usage.c:210 git-for-windows#1 0x<address-41> in access_or_die ( path=0x<address-40> "`\001\r��\004/config", mode=4, flag=0) at wrapper.c:667 git-for-windows#2 0x<address-39> in do_git_config_sequence (opts=0x<address-35>, fn=0x<address-37> <git_config_include>, data=0x<address-36>) at config.c:2142 git-for-windows#3 0x<address-38> in config_with_options ( fn=0x<address-37> <git_config_include>, data=0x<address-36>, config_source=0x0, opts=0x<address-35>) at config.c:2198 git-for-windows#4 0x<address-34> in repo_read_config (repo=0x<address-19> <the_repo>) at config.c:2524 git-for-windows#5 0x<address-33> in git_config_check_init ( repo=0x<address-19> <the_repo>) at config.c:2543 git-for-windows#6 0x<address-32> in repo_config_get_bool ( repo=0x<address-19> <the_repo>, key=0x<address-30> <pad+3116> "windows.appendatomically", dest=0x<address-29> <append_atomically>) at config.c:2612 git-for-windows#7 0x<address-31> in git_config_get_bool ( key=0x<address-30> <pad+3116> "windows.appendatomically", dest=0x<address-29> <append_atomically>) at config.c:2714 git-for-windows#8 0x<address-28> in mingw_open ( filename=0x<address-25> ".git/commondir", oflags=0) at compat/mingw.c:785 git-for-windows#9 0x<address-27> in strbuf_read_file (sb=0x<address-26>, path=0x<address-25> ".git/commondir", hint=0) at strbuf.c:758 git-for-windows#10 0x<address-24> in get_common_dir_noenv (sb=0x<address-23>, gitdir=0x<address-22> ".git") at setup.c:313 git-for-windows#11 0x<address-21> in repo_set_commondir (repo=0x<address-19> <the_repo>, commondir=0x0) at repository.c:57 git-for-windows#12 0x<address-20> in repo_set_gitdir (repo=0x<address-19> <the_repo>, root=0x<address-15> ".git", o=0x<address-18>) at repository.c:76 git-for-windows#13 0x<address-17> in setup_git_env (git_dir=0x<address-15> ".git") at environment.c:179 git-for-windows#14 0x<address-16> in set_git_dir_1 (path=0x<address-15> ".git") at environment.c:334 git-for-windows#15 0x<address-14> in update_relative_gitdir (name=0x0, old_cwd=0x<address-13> "C:/Users/%username%/<root-repo-name>/commonlibs", new_cwd=0x<address-11> "C:/Users/%username%/<root-repo-name>/commonlibs", data=0x0) at environment.c:348 git-for-windows#16 0x<address-12> in chdir_notify ( new_cwd=0x<address-11> "C:/Users/%username%/<root-repo-name>/commonlibs") at chdir-notify.c:72 git-for-windows#17 0x<address-10> in setup_work_tree () at setup.c:428 git-for-windows#18 0x<address-9> in run_builtin (p=0x<address-8> <commands+2856>, argc=2, argv=0x<address-2>) at git.c:458 git-for-windows#19 0x<address-7> in handle_builtin (argc=2, argv=0x<address-2>) at git.c:721 git-for-windows#20 0x<address-6> in run_argv (argcp=0x<address-5>, argv=0x<address-4>) at git.c:788 git-for-windows#21 0x<address-3> in cmd_main (argc=2, argv=0x<address-2>) at git.c:921 git-for-windows#22 0x<address-1> in main (argc=6, argv=0x<address-0>) at common-main.c:56 ``` Co-Authored-By: Johannes Schindelin <johannes.schindelin@gmx.de> Co-Authored-By: Andrey Zabavnikov <zabavnikov@gmail.com> Signed-off-by: 孙卓识 <sunzhuoshi@gmail.com> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Andrey Zabavnikov <zabavnikov@gmail.com>

In some setups, old-style submodules (i.e. the ones with .git directory within theirs worktrees) with commondir can be of tremendous help. For example, commondir link can be used to avoid duplication of objects and also to keep branches in sync with multiple copies of the repo's worktree, while keeping the .git directory inside the worktree can be (ab?-)used to exploit the sharing of the same submodule worktree across different projects (this at least works on Windows with submodule directory being a directory junction, but having a junction is not relevant for reproducing the bug described below). Unfortunately, at the moment, when `git status` is run in the root repo of such a setup, it gives an output akin to this: ```sh fatal: unable to access '�??\1?/config': Invalid argument fatal: 'git status --porcelain=2' failed in submodule commonlibs ``` where `�??\1?` part of '�??\1?/config' varies from run to run, and `commonlibs` is the name of submodule's directory. Currently, when Git discovers old-style submodule , it spawns subprocess to get its status, like this one: ```sh cd commonlibs; unset GIT_PREFIX; GIT_DIR=.git git status --porcelain=2 ``` Unsurprisingly, the following output is also quite unexpected: ``` fatal: unable to access '`??L&?/config': Invalid argument ``` The core reason for these is that global repository field for commondir is not being cleared to `NULL` after being `free()`'d in `repo_set_commondir()`, which is precisely what this commit fixes. Regarding the further details of the case of investigation, this value of struct pointed by the global `the_repository` pointer is checked for being not-NULL down in the callstack in compatibility layer for MinGW in a function that is called by `repo_set_commondir()` before the `free()`'d value gets assigned in its body (i.e. the body of `repo_set_commondir()`). Backtrace from the check is: #0 mingw_open (filename=0x<address-25> ".git/commondir", oflags=0) at compat/mingw.c:784 git-for-windows#1 0x<address-27> in strbuf_read_file (sb=0x<address-26>, path=0x<address-25> ".git/commondir", hint=0) at strbuf.c:758 git-for-windows#2 0x<address-24> in get_common_dir_noenv (sb=0x<address-23>, gitdir=0x<address-22> ".git") at setup.c:313 git-for-windows#3 0x<address-21> in repo_set_commondir (repo=0x<address-19> <the_repo>, commondir=0x0) at repository.c:57 git-for-windows#4 0x<address-20> in repo_set_gitdir (repo=0x<address-19> <the_repo>, root=0x<address-15> ".git", o=0x<address-18>) at repository.c:76 git-for-windows#5 0x<address-17> in setup_git_env (git_dir=0x<address-15> ".git") at environment.c:179 git-for-windows#6 0x<address-16> in set_git_dir_1 (path=0x<address-15> ".git") at environment.c:334 git-for-windows#7 0x<address-14> in update_relative_gitdir (name=0x0, old_cwd=0x<address-13> "C:/Users/%username%/<root-repo-name>/commonlibs", new_cwd=0x<address-11> "C:/Users/%username%/<root-repo-name>/commonlibs", data=0x0) at environment.c:348 git-for-windows#8 0x<address-12> in chdir_notify ( new_cwd=0x<address-11> "C:/Users/%username%/<root-repo-name>/commonlibs") at chdir-notify.c:72 git-for-windows#9 0x<address-10> in setup_work_tree () at setup.c:428 git-for-windows#10 0x<address-9> in run_builtin (p=0x<address-8> <commands+2856>, argc=2, argv=0x<address-2>) at git.c:458 git-for-windows#11 0x<address-7> in handle_builtin (argc=2, argv=0x<address-2>) at git.c:721 git-for-windows#12 0x<address-6> in run_argv (argcp=0x<address-5>, argv=0x<address-4>) at git.c:788 git-for-windows#13 0x<address-3> in cmd_main (argc=2, argv=0x<address-2>) at git.c:921 git-for-windows#14 0x<address-1> in main (argc=6, argv=0x<address-0>) at common-main.c:56 Backtrace from the death is: #0 die_errno (fmt=0x<address-42> <result_type+2002> "unable to access '%s'") at usage.c:210 git-for-windows#1 0x<address-41> in access_or_die ( path=0x<address-40> "`\001\r��\004/config", mode=4, flag=0) at wrapper.c:667 git-for-windows#2 0x<address-39> in do_git_config_sequence (opts=0x<address-35>, fn=0x<address-37> <git_config_include>, data=0x<address-36>) at config.c:2142 git-for-windows#3 0x<address-38> in config_with_options ( fn=0x<address-37> <git_config_include>, data=0x<address-36>, config_source=0x0, opts=0x<address-35>) at config.c:2198 git-for-windows#4 0x<address-34> in repo_read_config (repo=0x<address-19> <the_repo>) at config.c:2524 git-for-windows#5 0x<address-33> in git_config_check_init ( repo=0x<address-19> <the_repo>) at config.c:2543 git-for-windows#6 0x<address-32> in repo_config_get_bool ( repo=0x<address-19> <the_repo>, key=0x<address-30> <pad+3116> "windows.appendatomically", dest=0x<address-29> <append_atomically>) at config.c:2612 git-for-windows#7 0x<address-31> in git_config_get_bool ( key=0x<address-30> <pad+3116> "windows.appendatomically", dest=0x<address-29> <append_atomically>) at config.c:2714 git-for-windows#8 0x<address-28> in mingw_open ( filename=0x<address-25> ".git/commondir", oflags=0) at compat/mingw.c:785 git-for-windows#9 0x<address-27> in strbuf_read_file (sb=0x<address-26>, path=0x<address-25> ".git/commondir", hint=0) at strbuf.c:758 git-for-windows#10 0x<address-24> in get_common_dir_noenv (sb=0x<address-23>, gitdir=0x<address-22> ".git") at setup.c:313 git-for-windows#11 0x<address-21> in repo_set_commondir (repo=0x<address-19> <the_repo>, commondir=0x0) at repository.c:57 git-for-windows#12 0x<address-20> in repo_set_gitdir (repo=0x<address-19> <the_repo>, root=0x<address-15> ".git", o=0x<address-18>) at repository.c:76 git-for-windows#13 0x<address-17> in setup_git_env (git_dir=0x<address-15> ".git") at environment.c:179 git-for-windows#14 0x<address-16> in set_git_dir_1 (path=0x<address-15> ".git") at environment.c:334 git-for-windows#15 0x<address-14> in update_relative_gitdir (name=0x0, old_cwd=0x<address-13> "C:/Users/%username%/<root-repo-name>/commonlibs", new_cwd=0x<address-11> "C:/Users/%username%/<root-repo-name>/commonlibs", data=0x0) at environment.c:348 git-for-windows#16 0x<address-12> in chdir_notify ( new_cwd=0x<address-11> "C:/Users/%username%/<root-repo-name>/commonlibs") at chdir-notify.c:72 git-for-windows#17 0x<address-10> in setup_work_tree () at setup.c:428 git-for-windows#18 0x<address-9> in run_builtin (p=0x<address-8> <commands+2856>, argc=2, argv=0x<address-2>) at git.c:458 git-for-windows#19 0x<address-7> in handle_builtin (argc=2, argv=0x<address-2>) at git.c:721 git-for-windows#20 0x<address-6> in run_argv (argcp=0x<address-5>, argv=0x<address-4>) at git.c:788 git-for-windows#21 0x<address-3> in cmd_main (argc=2, argv=0x<address-2>) at git.c:921 git-for-windows#22 0x<address-1> in main (argc=6, argv=0x<address-0>) at common-main.c:56 Signed-off-by: Andrey Zabavnikov <zabavnikov@gmail.com>

When "read_strategy_opts()" is called we may have populated the "opts->strategy" before, so we'll need to free() it to avoid leaking memory. We populate it before because we cal get_replay_opts() from within "rebase.c" with an already populated "opts", which we then copy. Then if we're doing a "rebase -i" the sequencer API itself will promptly clobber our alloc'd version of it with its own. If this code is changed to do, instead of the added free() here a: if (opts->strategy) opts->strategy = xstrdup("another leak"); We get a couple of stacktraces from -fsanitize=leak showing how we ended up clobbering the already allocated value, i.e.: Direct leak of 6 byte(s) in 1 object(s) allocated from: #0 0x7f2e8cd45545 in __interceptor_malloc ../../../../src/libsanitizer/lsan/lsan_interceptors.cpp:75 #1 0x7f2e8cb0fcaa in __GI___strdup string/strdup.c:42 #2 0x6c4778 in xstrdup wrapper.c:39 #3 0x66bcb8 in read_strategy_opts sequencer.c:2902 #4 0x66bf7b in read_populate_opts sequencer.c:2969 #5 0x6723f9 in sequencer_continue sequencer.c:5063 #6 0x4a4f74 in run_sequencer_rebase builtin/rebase.c:348 #7 0x4a64c8 in run_specific_rebase builtin/rebase.c:753 #8 0x4a9b8b in cmd_rebase builtin/rebase.c:1824 #9 0x407a32 in run_builtin git.c:466 #10 0x407e0a in handle_builtin git.c:721 #11 0x40803d in run_argv git.c:788 #12 0x40850f in cmd_main git.c:923 #13 0x4eee79 in main common-main.c:57 #14 0x7f2e8ca9f209 in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58 #15 0x7f2e8ca9f2bb in __libc_start_main_impl ../csu/libc-start.c:389 #16 0x405fd0 in _start (git+0x405fd0) Direct leak of 4 byte(s) in 1 object(s) allocated from: #0 0x7f2e8cd45545 in __interceptor_malloc ../../../../src/libsanitizer/lsan/lsan_interceptors.cpp:75 #1 0x7f2e8cb0fcaa in __GI___strdup string/strdup.c:42 #2 0x6c4778 in xstrdup wrapper.c:39 #3 0x4a3c31 in xstrdup_or_null git-compat-util.h:1169 #4 0x4a447a in get_replay_opts builtin/rebase.c:163 #5 0x4a4f5b in run_sequencer_rebase builtin/rebase.c:346 #6 0x4a64c8 in run_specific_rebase builtin/rebase.c:753 #7 0x4a9b8b in cmd_rebase builtin/rebase.c:1824 #8 0x407a32 in run_builtin git.c:466 #9 0x407e0a in handle_builtin git.c:721 #10 0x40803d in run_argv git.c:788 #11 0x40850f in cmd_main git.c:923 #12 0x4eee79 in main common-main.c:57 #13 0x7f2e8ca9f209 in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58 #14 0x7f2e8ca9f2bb in __libc_start_main_impl ../csu/libc-start.c:389 #15 0x405fd0 in _start (git+0x405fd0) This can be seen in e.g. the 4th test of "t3404-rebase-interactive.sh". In the larger picture the ownership of the "struct replay_opts" is quite a mess, e.g. in this case rebase.c's static "get_replay_opts()" function partially creates it, but nothing in rebase.c will free() it. The structure is "mostly owned" by the sequencer API, but it also expects to get these partially populated versions of it. It would be better to have rebase keep track of what it allocated, and free() that, and to pass that as a "const" to the sequencer API, which would copy what it needs to its own version, and to free() that. But doing so is a much larger change, and however messy the ownership boundary is here is consistent with what we're doing already, so let's just free() this to fix the leak. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Taylor Blau <me@ttaylorr.com>

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

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

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

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

An out-of-bounds read can be triggered when parsing an incomplete padding format string passed via `--pretty=format` or in Git archives when files are marked with the `export-subst` gitattribute. This bug exists since we have introduced support for truncating output via the `trunc` keyword a7f01c6 (pretty: support truncating in %>, %< and %><, 2013-04-19). Before this commit, we used to find the end of the formatting string by using strchr(3P). This function returns a `NULL` pointer in case the character in question wasn't found. The subsequent check whether any character was found thus simply checked the returned pointer. After the commit we switched to strcspn(3P) though, which only returns the offset to the first found character or to the trailing NUL byte. As the end pointer is now computed by adding the offset to the start pointer it won't be `NULL` anymore, and as a consequence the check doesn't do anything anymore. The out-of-bounds data that is being read can in fact end up in the formatted string. As a consequence, it is possible to leak memory contents either by calling git-log(1) or via git-archive(1) when any of the archived files is marked with the `export-subst` gitattribute. ==10888==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x602000000398 at pc 0x7f0356047cb2 bp 0x7fff3ffb95d0 sp 0x7fff3ffb8d78 READ of size 1 at 0x602000000398 thread T0 #0 0x7f0356047cb1 in __interceptor_strchrnul /usr/src/debug/gcc/libsanitizer/sanitizer_common/sanitizer_common_interceptors.inc:725 #1 0x563b7cec9a43 in strbuf_expand strbuf.c:417 #2 0x563b7cda7060 in repo_format_commit_message pretty.c:1869 #3 0x563b7cda8d0f in pretty_print_commit pretty.c:2161 #4 0x563b7cca04c8 in show_log log-tree.c:781 #5 0x563b7cca36ba in log_tree_commit log-tree.c:1117 #6 0x563b7c927ed5 in cmd_log_walk_no_free builtin/log.c:508 #7 0x563b7c92835b in cmd_log_walk builtin/log.c:549 #8 0x563b7c92b1a2 in cmd_log builtin/log.c:883 #9 0x563b7c802993 in run_builtin git.c:466 #10 0x563b7c803397 in handle_builtin git.c:721 #11 0x563b7c803b07 in run_argv git.c:788 #12 0x563b7c8048a7 in cmd_main git.c:923 #13 0x563b7ca99682 in main common-main.c:57 #14 0x7f0355e3c28f (/usr/lib/libc.so.6+0x2328f) #15 0x7f0355e3c349 in __libc_start_main (/usr/lib/libc.so.6+0x23349) #16 0x563b7c7fe0e4 in _start ../sysdeps/x86_64/start.S:115 0x602000000398 is located 0 bytes to the right of 8-byte region [0x602000000390,0x602000000398) allocated by thread T0 here: #0 0x7f0356072faa in __interceptor_strdup /usr/src/debug/gcc/libsanitizer/asan/asan_interceptors.cpp:439 #1 0x563b7cf7317c in xstrdup wrapper.c:39 #2 0x563b7cd9a06a in save_user_format pretty.c:40 #3 0x563b7cd9b3e5 in get_commit_format pretty.c:173 #4 0x563b7ce54ea0 in handle_revision_opt revision.c:2456 #5 0x563b7ce597c9 in setup_revisions revision.c:2850 #6 0x563b7c9269e0 in cmd_log_init_finish builtin/log.c:269 #7 0x563b7c927362 in cmd_log_init builtin/log.c:348 #8 0x563b7c92b193 in cmd_log builtin/log.c:882 #9 0x563b7c802993 in run_builtin git.c:466 #10 0x563b7c803397 in handle_builtin git.c:721 #11 0x563b7c803b07 in run_argv git.c:788 #12 0x563b7c8048a7 in cmd_main git.c:923 #13 0x563b7ca99682 in main common-main.c:57 #14 0x7f0355e3c28f (/usr/lib/libc.so.6+0x2328f) #15 0x7f0355e3c349 in __libc_start_main (/usr/lib/libc.so.6+0x23349) #16 0x563b7c7fe0e4 in _start ../sysdeps/x86_64/start.S:115 SUMMARY: AddressSanitizer: heap-buffer-overflow /usr/src/debug/gcc/libsanitizer/sanitizer_common/sanitizer_common_interceptors.inc:725 in __interceptor_strchrnul Shadow bytes around the buggy address: 0x0c047fff8020: fa fa fd fd fa fa 00 06 fa fa 05 fa fa fa fd fd 0x0c047fff8030: fa fa 00 02 fa fa 06 fa fa fa 05 fa fa fa fd fd 0x0c047fff8040: fa fa 00 07 fa fa 03 fa fa fa fd fd fa fa 00 00 0x0c047fff8050: fa fa 00 01 fa fa fd fd fa fa 00 00 fa fa 00 01 0x0c047fff8060: fa fa 00 06 fa fa 00 06 fa fa 05 fa fa fa 05 fa =>0x0c047fff8070: fa fa 00[fa]fa fa fd fa fa fa fd fd fa fa fd fd 0x0c047fff8080: fa fa fd fd fa fa 00 00 fa fa 00 fa fa fa fd fa 0x0c047fff8090: fa fa fd fd fa fa 00 00 fa fa fa fa fa fa fa fa 0x0c047fff80a0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa 0x0c047fff80b0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa 0x0c047fff80c0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa Shadow byte legend (one shadow byte represents 8 application bytes): Addressable: 00 Partially addressable: 01 02 03 04 05 06 07 Heap left redzone: fa Freed heap region: fd Stack left redzone: f1 Stack mid redzone: f2 Stack right redzone: f3 Stack after return: f5 Stack use after scope: f8 Global redzone: f9 Global init order: f6 Poisoned by user: f7 Container overflow: fc Array cookie: ac Intra object redzone: bb ASan internal: fe Left alloca redzone: ca Right alloca redzone: cb ==10888==ABORTING Fix this bug by checking whether `end` points at the trailing NUL byte. Add a test which catches this out-of-bounds read and which demonstrates that we used to write out-of-bounds data into the formatted message. Reported-by: Markus Vervier <markus.vervier@x41-dsec.de> Original-patch-by: Markus Vervier <markus.vervier@x41-dsec.de> Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>

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

Signed-off-by: Teng Long <dyroneteng@gmail.com> Reviewed-by: Jiang Xin <zhiyou.jx@alibaba-inc.com> Reviewed-by: 依云 <lilydjwg@gmail.com> Reviewed-by: pan93412 <pan93412@gmail.com> Reviewed0by: Fangyi Zhou <me@fangyi.io>

* 'tl/zh_CN_2.41.0_rnd1' of github.com:dyrone/git: l10n: zh_CN: Git 2.41.0 round #1

l10n-2.41.0-2 * tag 'l10n-2.41.0-2' of https://github.com/git-l10n/git-po: l10n: zh_TW.po: Git 2.41.0 l10n: sv.po: Update Swedish translation (5515t0f0u) l10n: Update Catalan translation l10n: Update German translation l10n: po-id for 2.41 (round 1) l10n: Update Catalan translation l10n: tr: Update Turkish translations for 2.41.0 l10n: fr.po v2.41.0 rnd2 l10n: fr.po v2.41.0 rnd1 l10n: fr: fix translation of stash save help l10n: zh_CN: Git 2.41.0 round #1 l10n: bg.po: Updated Bulgarian translation (5515t) l10n: update uk localization l10n: uk: remove stale lines l10n: uk: add initial translation l10n: TEAMS: Update pt_PT repo link

When "update-index --unresolve $path" cannot find the resolve-undo record for the path the user requested to unresolve, it stuffs the blobs from HEAD and MERGE_HEAD to stage #2 and stage #3 as a fallback. For this reason, the operation does not even start unless both "HEAD" and "MERGE_HEAD" exist. This is suboptimal in a few ways: * It does not recreate stage #1. Even though it is a correct design decision not to do so (because it is impossible to recreate in general cases, without knowing how we got there, including what merge strategy was used), it is much less useful not to have that information in the index. * It limits the "unresolve" operation only during a conflicted "git merge" and nothing else. Other operations like "rebase", "cherry-pick", and "switch -m" may result in conflicts, and the user may want to unresolve the conflict that they incorrectly resolved in order to redo the resolution, but the fallback would not kick in. * Most importantly, the entire "unresolve" operation is disabled after a conflicted merge is committed and MERGE_HEAD is removed, even though the index has perfectly usable resolve-undo records. By lazily reading the HEAD and MERGE_HEAD only when we need to go to the fallback codepath, we will allow cases where resolve-undo records are available (which is 100% of the time, unless the user is reading from an index file created by Git more than 10 years ago) to proceed even after a conflicted merge was committed, during other mergy operations that do not use MERGE_HEAD, or after the result of such mergy operations has been committed. Signed-off-by: Junio C Hamano <gitster@pobox.com>

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>

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>

It is tempting to think of "files and directories" of the current directory as valid inputs to the add and set subcommands of git sparse-checkout. However, in non-cone mode, they often aren't and using them as potential completions leads to *many* forms of confusion: Issue #1. It provides the *wrong* files and directories. For git sparse-checkout add we always want to add files and directories not currently in our sparse checkout, which means we want file and directories not currently present in the current working tree. Providing the files and directories currently present is thus always wrong. For git sparse-checkout set we have a similar problem except in the subset of cases where we are trying to narrow our checkout to a strict subset of what we already have. That is not a very common scenario, especially since it often does not even happen to be true for the first use of the command; for years we required users to create a sparse-checkout via git sparse-checkout init git sparse-checkout set <args...> (or use a clone option that did the init step for you at clone time). The init command creates a minimal sparse-checkout with just the top-level directory present, meaning the set command has to be used to expand the checkout. Thus, only in a special and perhaps unusual cases would any of the suggestions from normal file and directory completion be appropriate. Issue #2: Suggesting patterns that lead to warnings is unfriendly. If the user specifies any regular file and omits the leading '/', then the sparse-checkout command will warn the user that their command is problematic and suggest they use a leading slash instead. Issue #3: Completion gets confused by leading '/', and provides wrong paths. Users often want to anchor their patterns to the toplevel of the repository, especially when listing individual files. There are a number of reasons for this, but notably even sparse-checkout encourages them to do so (as noted above). However, if users do so (via adding a leading '/' to their pattern), then bash completion will interpret the leading slash not as a request for a path at the toplevel of the repository, but as a request for a path at the root of the filesytem. That means at best that completion cannot help with such paths, and if it does find any completions, they are almost guaranteed to be wrong. Issue #4: Suggesting invalid patterns from subdirectories is unfriendly. There is no per-directory equivalent to .gitignore with sparse-checkouts. There is only a single worktree-global $GIT_DIR/info/sparse-checkout file. As such, paths to files must be specified relative to the toplevel of a repository. Providing suggestions of paths that are relative to the current working directory, as bash completion defaults to, is wrong when the current working directory is not the worktree toplevel directory. Issue #5: Paths with special characters will be interpreted incorrectly The entries in the sparse-checkout file are patterns, not paths. While most paths also qualify as patterns (though even in such cases it would be better for users to not use them directly but prefix them with a leading '/'), there are a variety of special characters that would need special escaping beyond the normal shell escaping: '*', '?', '\', '[', ']', and any leading '#' or '!'. If completion suggests any such paths, users will likely expect them to be treated as an exact path rather than as a pattern that might match some number of files other than 1. Because of the combination of the above issues, turn completion off for the `set` and `add` subcommands of `sparse-checkout` when in non-cone mode, but leave a NEEDSWORK comment specifying what could theoretically be done if someone wanted to provide completion rules that were more helpful than harmful. Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

It is tempting to think of "files and directories" of the current directory as valid inputs to the add and set subcommands of git sparse-checkout. However, in non-cone mode, they often aren't and using them as potential completions leads to *many* forms of confusion: Issue #1. It provides the *wrong* files and directories. For git sparse-checkout add we always want to add files and directories not currently in our sparse checkout, which means we want file and directories not currently present in the current working tree. Providing the files and directories currently present is thus always wrong. For git sparse-checkout set we have a similar problem except in the subset of cases where we are trying to narrow our checkout to a strict subset of what we already have. That is not a very common scenario, especially since it often does not even happen to be true for the first use of the command; for years we required users to create a sparse-checkout via git sparse-checkout init git sparse-checkout set <args...> (or use a clone option that did the init step for you at clone time). The init command creates a minimal sparse-checkout with just the top-level directory present, meaning the set command has to be used to expand the checkout. Thus, only in a special and perhaps unusual cases would any of the suggestions from normal file and directory completion be appropriate. Issue #2: Suggesting patterns that lead to warnings is unfriendly. If the user specifies any regular file and omits the leading '/', then the sparse-checkout command will warn the user that their command is problematic and suggest they use a leading slash instead. Issue #3: Completion gets confused by leading '/', and provides wrong paths. Users often want to anchor their patterns to the toplevel of the repository, especially when listing individual files. There are a number of reasons for this, but notably even sparse-checkout encourages them to do so (as noted above). However, if users do so (via adding a leading '/' to their pattern), then bash completion will interpret the leading slash not as a request for a path at the toplevel of the repository, but as a request for a path at the root of the filesytem. That means at best that completion cannot help with such paths, and if it does find any completions, they are almost guaranteed to be wrong. Issue #4: Suggesting invalid patterns from subdirectories is unfriendly. There is no per-directory equivalent to .gitignore with sparse-checkouts. There is only a single worktree-global $GIT_DIR/info/sparse-checkout file. As such, paths to files must be specified relative to the toplevel of a repository. Providing suggestions of paths that are relative to the current working directory, as bash completion defaults to, is wrong when the current working directory is not the worktree toplevel directory. Issue #5: Paths with special characters will be interpreted incorrectly The entries in the sparse-checkout file are patterns, not paths. While most paths also qualify as patterns (though even in such cases it would be better for users to not use them directly but prefix them with a leading '/'), there are a variety of special characters that would need special escaping beyond the normal shell escaping: '*', '?', '\', '[', ']', and any leading '#' or '!'. If completion suggests any such paths, users will likely expect them to be treated as an exact path rather than as a pattern that might match some number of files other than 1. However, despite the first four issues, we can note that _if_ users are using tab completion, then they are probably trying to specify a path in the index. As such, we transform their argument into a top-level-rooted pattern that matches such a file. For example, if they type: git sparse-checkout add Make<TAB> we could "complete" to git sparse-checkout add /Makefile or, if they ran from the Documentation/technical/ subdirectory: git sparse-checkout add m<TAB> we could "complete" it to: git sparse-checkout add /Documentation/technical/multi-pack-index.txt Note in both cases I use "complete" in quotes, because we actually add characters both before and after the argument in question, so we are kind of abusing "bash completions" to be "bash completions AND beginnings". The fifth issue is a bit stickier, especially when you consider that we not only need to deal with escaping issues because of special meanings of patterns in sparse-checkout & gitignore files, but also that we need to consider escaping issues due to ls-files needing to sometimes quote or escape characters, and because the shell needs to escape some characters. The multiple interacting forms of escaping could get ugly; this patch makes no attempt to do so and simply documents that we decided to not deal with those corner cases for now but at least get the common cases right. Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

When reusing objects from a pack, we keep track of a set of one or more `reused_chunk`s, corresponding to sections of one or more object(s) from a source pack that we are reusing. Each chunk contains two pieces of information: - the offset of the first object in the source pack (relative to the beginning of the source pack) - the difference between that offset, and the corresponding offset in the pack we're generating The purpose of keeping track of these is so that we can patch an OFS_DELTAs that cross over a section of the reuse pack that we didn't take. For instance, consider a hypothetical pack as shown below: (chunk #2) __________... / / +--------+---------+-------------------+---------+ ... | <base> | <other> | (unused) | <delta> | ... +--------+---------+-------------------+---------+ \ / \______________/ (chunk #1) Suppose that we are sending objects "base", "other", and "delta", and that the "delta" object is stored as an OFS_DELTA, and that its base is "base". If we don't send any objects in the "(unused)" range, we can't copy the delta'd object directly, since its delta offset includes a range of the pack that we didn't copy, so we have to account for that difference when patching and reassembling the delta. In order to compute this value correctly, we need to know not only where we are in the packfile we're assembling (with `hashfile_total(f)`) but also the position of the first byte of the packfile that we are currently reusing. Currently, this works just fine, since when reusing only a single pack those two values are always identical (because verbatim reuse is the first thing pack-objects does when enabled after writing the pack header). But when reusing multiple packs which have one or more gaps, we'll need to account for these two values diverging. Together, these two allow us to compute the reused chunk's offset difference relative to the start of the reused pack, as desired. Helped-by: Jeff King <peff@peff.net> Signed-off-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

The t5309 script triggers a racy false positive with SANITIZE=leak on a multi-core system. Running with "--stress --run=6" usually fails within 10 seconds or so for me, complaining with something like: + git index-pack --fix-thin --stdin fatal: REF_DELTA at offset 46 already resolved (duplicate base 01d7713666f4de822776c7622c10f1b07de280dc?) ================================================================= ==3904583==ERROR: LeakSanitizer: detected memory leaks Direct leak of 32 byte(s) in 1 object(s) allocated from: #0 0x7fa790d01986 in __interceptor_realloc ../../../../src/libsanitizer/lsan/lsan_interceptors.cpp:98 #1 0x7fa790add769 in __pthread_getattr_np nptl/pthread_getattr_np.c:180 #2 0x7fa790d117c5 in __sanitizer::GetThreadStackTopAndBottom(bool, unsigned long*, unsigned long*) ../../../../src/libsanitizer/sanitizer_common/sanitizer_linux_libcdep.cpp:150 #3 0x7fa790d11957 in __sanitizer::GetThreadStackAndTls(bool, unsigned long*, unsigned long*, unsigned long*, unsigned long*) ../../../../src/libsanitizer/sanitizer_common/sanitizer_linux_libcdep.cpp:598 #4 0x7fa790d03fe8 in __lsan::ThreadStart(unsigned int, unsigned long long, __sanitizer::ThreadType) ../../../../src/libsanitizer/lsan/lsan_posix.cpp:51 #5 0x7fa790d013fd in __lsan_thread_start_func ../../../../src/libsanitizer/lsan/lsan_interceptors.cpp:440 #6 0x7fa790adc3eb in start_thread nptl/pthread_create.c:444 #7 0x7fa790b5ca5b in clone3 ../sysdeps/unix/sysv/linux/x86_64/clone3.S:81 SUMMARY: LeakSanitizer: 32 byte(s) leaked in 1 allocation(s). Aborted What happens is this: 0. We construct a bogus pack with a duplicate object in it and trigger index-pack. 1. We spawn a bunch of worker threads to resolve deltas (on my system it is 16 threads). 2. One of the threads sees the duplicate object and bails by calling exit(), taking down all of the threads. This is expected and is the point of the test. 3. At the time exit() is called, we may still be spawning threads from the main process via pthread_create(). LSan hooks thread creation to update its book-keeping; it has to know where each thread's stack is (so it can find entry points for reachable memory). So it calls pthread_getattr_np() to get information about the new thread. That may allocate memory that must be freed with a matching call to pthread_attr_destroy(). Probably LSan does that immediately, but if you're unlucky enough, the exit() will happen while it's between those two calls, and the allocated pthread_attr_t appears as a leak. This isn't a real leak. It's not even in our code, but rather in the LSan instrumentation code. So we could just ignore it. But the false positive can cause people to waste time tracking it down. It's possibly something that LSan could protect against (e.g., cover the getattr/destroy pair with a mutex, and then in the final post-exit() check for leaks try to take the same mutex). But I don't know enough about LSan to say if that's a reasonable approach or not (or if my analysis is even completely correct). In the meantime, it's pretty easy to avoid the race by making creation of the worker threads "atomic". That is, we'll spawn all of them before letting any of them start to work. That's easy to do because we already have a work_lock() mutex for handing out that work. If the main process takes it, then all of the threads will immediately block until we've finished spawning and released it. This shouldn't make any practical difference for non-LSan runs. The thread spawning is quick, and could happen before any worker thread gets scheduled anyway. Probably other spots that use threads are subject to the same issues. But since we have to manually insert locking (and since this really is kind of a hack), let's not bother with them unless somebody experiences a similar racy false-positive in practice. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Commit d6a8c58 (midx-write.c: support reading an existing MIDX with `packs_to_include`, 2024-05-29) changed the MIDX generation machinery to support reading from an existing MIDX when writing a new one. Unfortunately, the rest of the MIDX generation machinery is not prepared to deal with such a change. For instance, the function responsible for adding to the object ID fanout table from a MIDX source (midx_fanout_add_midx_fanout()) will gladly add objects from an existing MIDX for some fanout level regardless of whether or not those objects came from packs that are to be included in the subsequent MIDX write. This results in broken pseudo-pack object order (leading to incorrect object traversal results) and segmentation faults, like so (generated by running the added test prior to the changes in midx-write.c): #0 0x000055ee31393f47 in midx_pack_order (ctx=0x7ffdde205c70) at midx-write.c:590 #1 0x000055ee31395a69 in write_midx_internal (object_dir=0x55ee32570440 ".git/objects", packs_to_include=0x7ffdde205e20, packs_to_drop=0x0, preferred_pack_name=0x0, refs_snapshot=0x0, flags=15) at midx-write.c:1171 #2 0x000055ee31395f38 in write_midx_file_only (object_dir=0x55ee32570440 ".git/objects", packs_to_include=0x7ffdde205e20, preferred_pack_name=0x0, refs_snapshot=0x0, flags=15) at midx-write.c:1274 [...] In stack frame #0, the code on midx-write.c:590 is using the new pack ID corresponding to some object which was added from the existing MIDX. Importantly, the pack from which that object was selected in the existing MIDX does not appear in the new MIDX as it was excluded via `--stdin-packs`. In this instance, the pack in question had pack ID "1" in the existing MIDX, but since it was excluded from the new MIDX, we never filled in that entry in the pack_perm table, resulting in: (gdb) p *ctx->pack_perm@2 $1 = {0, 1515870810} Which is what causes the segfault above when we try and read: struct pack_info *pack = &ctx->info[ctx->pack_perm[i]]; if (pack->bitmap_pos == BITMAP_POS_UNKNOWN) pack->bitmap_pos = 0; Fundamentally, we should be able to read information from an existing MIDX when generating a new one. But in practice the midx-write.c code assumes that we won't run into issues like the above with incongruent pack IDs, and often makes those assumptions in extremely subtle and fragile ways. Instead, let's avoid reading from an existing MIDX altogether, and stick with the pre-d6a8c58675 implementation. Harden against any regressions in this area by adding a test which demonstrates these issues. Signed-off-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

When performing multi-pack reuse, reuse_partial_packfile_from_bitmap() is responsible for generating an array of bitmapped_pack structs from which to perform reuse. In the multi-pack case, we loop over the MIDXs packs and copy the result of calling `nth_bitmapped_pack()` to construct the list of reusable paths. But we may also want to do pack-reuse over a single pack, either because we only had one pack to perform reuse over (in the case of single-pack bitmaps), or because we explicitly asked to do single pack reuse even with a MIDX[^1]. When this is the case, the array we generate of reusable packs contains only a single element, which is either (a) the pack attached to the single-pack bitmap, or (b) the MIDX's preferred pack. In 795006f (pack-bitmap: gracefully handle missing BTMP chunks, 2024-04-15), we refactored the reuse_partial_packfile_from_bitmap() function and stopped assigning the pack_int_id field when reusing only the MIDX's preferred pack. This results in an uninitialized read down in try_partial_reuse() like so: ==7474==WARNING: MemorySanitizer: use-of-uninitialized-value #0 0x55c5cd191dde in try_partial_reuse pack-bitmap.c:1887:8 #1 0x55c5cd191dde in reuse_partial_packfile_from_bitmap_1 pack-bitmap.c:2001:8 #2 0x55c5cd191dde in reuse_partial_packfile_from_bitmap pack-bitmap.c:2105:3 #3 0x55c5cce0bd0e in get_object_list_from_bitmap builtin/pack-objects.c:4043:3 #4 0x55c5cce0bd0e in get_object_list builtin/pack-objects.c:4156:27 #5 0x55c5cce0bd0e in cmd_pack_objects builtin/pack-objects.c:4596:3 #6 0x55c5ccc8fac8 in run_builtin git.c:474:11 which happens when try_partial_reuse() tries to call midx_pair_to_pack_pos() when it tries to reject cross-pack deltas. Avoid the uninitialized read by ensuring that the pack_int_id field is set in the single-pack reuse case by setting it to either the MIDX preferred pack's pack_int_id, or '-1', in the case of single-pack bitmaps. In the latter case, we never read the pack_int_id field, so the choice of '-1' is intentional as a "garbage in, garbage out" measure. Guard against further regressions in this area by adding a test which ensures that we do not throw out deltas from the preferred pack as "cross-pack" due to an uninitialized pack_int_id. [^1]: This can happen for a couple of reasons, either because the repository is configured with 'pack.allowPackReuse=(true|single)', or because the MIDX was generated prior to the introduction of the BTMP chunk, which contains information necessary to perform multi-pack reuse. Reported-by: Kyle Lippincott <spectral@google.com> Signed-off-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Memory sanitizer (msan) is detecting a use of an uninitialized variable (`size`) in `read_attr_from_index`: ==2268==WARNING: MemorySanitizer: use-of-uninitialized-value #0 0x5651f3416504 in read_attr_from_index git/attr.c:868:11 #1 0x5651f3415530 in read_attr git/attr.c #2 0x5651f3413d74 in bootstrap_attr_stack git/attr.c:968:6 #3 0x5651f3413d74 in prepare_attr_stack git/attr.c:1004:2 #4 0x5651f3413d74 in collect_some_attrs git/attr.c:1199:2 #5 0x5651f3413144 in git_check_attr git/attr.c:1345:2 #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>

sschuberth and others added 4 commits September 3, 2014 22:43

MinGW: Use MakeMaker to build the Perl libraries

95b7175

This way the libraries get properly installed into the "site_perl" directory and we just have to move them out of the "mingw" directory. Signed-off-by: Sebastian Schuberth <sschuberth@gmail.com>

Makefile: Set htmldir to match the default HTML docs location under MSYS

145186a

Signed-off-by: Sebastian Schuberth <sschuberth@gmail.com>

t7800: Use "test_cmp_text" in all places where "echo" is used

9264b1f

While "echo", being a Bash built-in, produces Unix line-endings, "git" and "cat" produce DOS line-endings on MSYS, so ignore line-ending differences. Signed-off-by: Sebastian Schuberth <sschuberth@gmail.com>

sschuberth added a commit that referenced this pull request Sep 4, 2014

Merge pull request #1 from t-b/new-sdk-fixes

cd79f58

Fixes for the new SDK

sschuberth merged commit cd79f58 into git-for-windows:master Sep 4, 2014

t-b deleted the new-sdk-fixes branch September 4, 2014 15:59

t-b added a commit that referenced this pull request Oct 13, 2014

Merge pull request #1 from t-b/new-sdk-fixes

ccb6a7a

Fixes for the new SDK

dscho pushed a commit that referenced this pull request Mar 6, 2015

t9001: style modernisation phase #1

aca5606

Don't chop test_expect_success line into pieces and concatenate with '\'. That's so 2005. Signed-off-by: Junio C Hamano <gitster@pobox.com>

dscho pushed a commit that referenced this pull request Mar 6, 2015

Post 2.3 cycle (batch #1)

18d0fec

Signed-off-by: Junio C Hamano <gitster@pobox.com>

ghost mentioned this pull request Jul 9, 2015

Unknown SSL protocol error in connection to github.com:443 #234

Closed

jmedemaUD mentioned this pull request Sep 1, 2015

git gui clones non-master default branch as master #345

Closed

galehar mentioned this pull request Sep 10, 2015

Unknown SSL protocol error with HTTPS proxy decryption #373

Closed

DarwinJS mentioned this pull request Dec 14, 2015

Use of "icons\quicklaunch" install option, while executing under system account causes hang #570

Closed

PhilipOakley mentioned this pull request Jan 16, 2016

Git 2.6.2 and higher sending NTLM token instead of Kerberos when Negotiate is used #611

Closed

paladox mentioned this pull request Apr 6, 2016

Fix using windows path in git for windows #720

Closed

1 task

git-for-windows-ci pushed a commit that referenced this pull request Jun 1, 2023

Merge branch 'tl/zh_CN_2.41.0_rnd1' of github.com:dyrone/git

f9bb784

* 'tl/zh_CN_2.41.0_rnd1' of github.com:dyrone/git: l10n: zh_CN: Git 2.41.0 round #1

jonypeixoto mentioned this pull request Aug 11, 2023

warning: failed to restrict file handles #4554

Closed

1 task

venus-Liu mentioned this pull request Dec 25, 2023

git error #4748

Open

vivtiwary mentioned this pull request Jan 5, 2024

Issue in pulling or pushing codes for windows #4757

Closed

TomClabault mentioned this pull request Jun 17, 2024

warning: failed to restrict file handles (5) #5012

Open

1 task

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fixes for the new SDK #1

Fixes for the new SDK #1

t-b commented Sep 3, 2014

dscho commented Sep 4, 2014

sschuberth commented Sep 4, 2014

sschuberth commented Sep 4, 2014

t-b commented Sep 4, 2014

Fixes for the new SDK #1

Fixes for the new SDK #1

Conversation

t-b commented Sep 3, 2014

dscho commented Sep 4, 2014

sschuberth commented Sep 4, 2014

sschuberth commented Sep 4, 2014

t-b commented Sep 4, 2014