$PATH not quite right #18
Comments
|
If you change your windows path to not contain a space before each path component, i.e. It looks strange that the first part of $PATH starts with |
Please open a separate Pull Request to allow discuss these changes. |
|
t-b: actually that fixed it! duh. thanks. |
|
@cluelesscoder I will keep in mind that if I ever am in need for any suggestion how to fill my abundant free time, I will ask you :-P |
dscho
pushed a commit
that referenced
this issue
Mar 10, 2021
The primary goal of this change is to stop leaking init_db_template_dir.
This leak can happen because:
1. git_init_db_config() allocates new memory into init_db_template_dir
without first freeing the existing value.
2. init_db_template_dir might already contain data, either because:
2.1 git_config() can be invoked twice with this callback in a single
process - at least 2 allocations are likely.
2.2 A single git_config() allocation can invoke the callback multiple
times for a given key (see further explanation in the function
docs) - each of those calls will trigger another leak.
The simplest fix for the leak would be to free(init_db_template_dir)
before overwriting it. Instead we choose to convert to fetching
init.templatedir via git_config_get_value() as that is more explicit,
more efficient, and avoids allocations (the returned result is owned by
the config cache, so we aren't responsible for freeing it).
If we remove init_db_template_dir, git_init_db_config() ends up being
responsible only for forwarding core.* config values to
platform_core_config(). However platform_core_config() already ignores
non-core.* config values, so we can safely remove git_init_db_config()
and invoke git_config() directly with platform_core_config() as the
callback.
The platform_core_config forwarding was originally added in:
2878533 (mingw: respect core.hidedotfiles = false in git-init again, 2019-03-11
And I suspect the potential for a leak existed since the original
implementation of git_init_db_config in:
90b4518 (Add `init.templatedir` configuration variable., 2010-02-17)
LSAN output from t0001:
Direct leak of 73 byte(s) in 1 object(s) allocated from:
#0 0x49a859 in realloc /home/abuild/rpmbuild/BUILD/llvm-11.0.0.src/build/../projects/compiler-rt/lib/asan/asan_malloc_linux.cpp:164:3
#1 0x9a7276 in xrealloc /home/ahunt/oss-fuzz/git/wrapper.c:126:8
#2 0x9362ad in strbuf_grow /home/ahunt/oss-fuzz/git/strbuf.c:98:2
#3 0x936eaa in strbuf_add /home/ahunt/oss-fuzz/git/strbuf.c:295:2
#4 0x868112 in strbuf_addstr /home/ahunt/oss-fuzz/git/./strbuf.h:304:2
#5 0x86a8ad in expand_user_path /home/ahunt/oss-fuzz/git/path.c:758:2
#6 0x720bb1 in git_config_pathname /home/ahunt/oss-fuzz/git/config.c:1287:10
#7 0x5960e2 in git_init_db_config /home/ahunt/oss-fuzz/git/builtin/init-db.c:161:11
#8 0x7255b8 in configset_iter /home/ahunt/oss-fuzz/git/config.c:1982:7
#9 0x7253fc in repo_config /home/ahunt/oss-fuzz/git/config.c:2311:2
#10 0x725ca7 in git_config /home/ahunt/oss-fuzz/git/config.c:2399:2
#11 0x593e8d in create_default_files /home/ahunt/oss-fuzz/git/builtin/init-db.c:225:2
#12 0x5935c6 in init_db /home/ahunt/oss-fuzz/git/builtin/init-db.c:449:11
#13 0x59588e in cmd_init_db /home/ahunt/oss-fuzz/git/builtin/init-db.c:714:9
#14 0x4cd60d in run_builtin /home/ahunt/oss-fuzz/git/git.c:453:11
#15 0x4cb2da in handle_builtin /home/ahunt/oss-fuzz/git/git.c:704:3
#16 0x4ccc37 in run_argv /home/ahunt/oss-fuzz/git/git.c:771:4
#17 0x4cac29 in cmd_main /home/ahunt/oss-fuzz/git/git.c:902:19
#18 0x69c4de in main /home/ahunt/oss-fuzz/git/common-main.c:52:11
#19 0x7f23552d6349 in __libc_start_main (/lib64/libc.so.6+0x24349)
Signed-off-by: Andrzej Hunt <ajrhunt@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
dscho
pushed a commit
that referenced
this issue
Mar 15, 2021
The primary goal of this change is to stop leaking init_db_template_dir.
This leak can happen because:
1. git_init_db_config() allocates new memory into init_db_template_dir
without first freeing the existing value.
2. init_db_template_dir might already contain data, either because:
2.1 git_config() can be invoked twice with this callback in a single
process - at least 2 allocations are likely.
2.2 A single git_config() allocation can invoke the callback multiple
times for a given key (see further explanation in the function
docs) - each of those calls will trigger another leak.
The simplest fix for the leak would be to free(init_db_template_dir)
before overwriting it. Instead we choose to convert to fetching
init.templatedir via git_config_get_value() as that is more explicit,
more efficient, and avoids allocations (the returned result is owned by
the config cache, so we aren't responsible for freeing it).
If we remove init_db_template_dir, git_init_db_config() ends up being
responsible only for forwarding core.* config values to
platform_core_config(). However platform_core_config() already ignores
non-core.* config values, so we can safely remove git_init_db_config()
and invoke git_config() directly with platform_core_config() as the
callback.
The platform_core_config forwarding was originally added in:
2878533 (mingw: respect core.hidedotfiles = false in git-init again, 2019-03-11
And I suspect the potential for a leak existed since the original
implementation of git_init_db_config in:
90b4518 (Add `init.templatedir` configuration variable., 2010-02-17)
LSAN output from t0001:
Direct leak of 73 byte(s) in 1 object(s) allocated from:
#0 0x49a859 in realloc /home/abuild/rpmbuild/BUILD/llvm-11.0.0.src/build/../projects/compiler-rt/lib/asan/asan_malloc_linux.cpp:164:3
#1 0x9a7276 in xrealloc /home/ahunt/oss-fuzz/git/wrapper.c:126:8
#2 0x9362ad in strbuf_grow /home/ahunt/oss-fuzz/git/strbuf.c:98:2
#3 0x936eaa in strbuf_add /home/ahunt/oss-fuzz/git/strbuf.c:295:2
#4 0x868112 in strbuf_addstr /home/ahunt/oss-fuzz/git/./strbuf.h:304:2
#5 0x86a8ad in expand_user_path /home/ahunt/oss-fuzz/git/path.c:758:2
#6 0x720bb1 in git_config_pathname /home/ahunt/oss-fuzz/git/config.c:1287:10
#7 0x5960e2 in git_init_db_config /home/ahunt/oss-fuzz/git/builtin/init-db.c:161:11
#8 0x7255b8 in configset_iter /home/ahunt/oss-fuzz/git/config.c:1982:7
#9 0x7253fc in repo_config /home/ahunt/oss-fuzz/git/config.c:2311:2
#10 0x725ca7 in git_config /home/ahunt/oss-fuzz/git/config.c:2399:2
#11 0x593e8d in create_default_files /home/ahunt/oss-fuzz/git/builtin/init-db.c:225:2
#12 0x5935c6 in init_db /home/ahunt/oss-fuzz/git/builtin/init-db.c:449:11
#13 0x59588e in cmd_init_db /home/ahunt/oss-fuzz/git/builtin/init-db.c:714:9
#14 0x4cd60d in run_builtin /home/ahunt/oss-fuzz/git/git.c:453:11
#15 0x4cb2da in handle_builtin /home/ahunt/oss-fuzz/git/git.c:704:3
#16 0x4ccc37 in run_argv /home/ahunt/oss-fuzz/git/git.c:771:4
#17 0x4cac29 in cmd_main /home/ahunt/oss-fuzz/git/git.c:902:19
#18 0x69c4de in main /home/ahunt/oss-fuzz/git/common-main.c:52:11
#19 0x7f23552d6349 in __libc_start_main (/lib64/libc.so.6+0x24349)
Signed-off-by: Andrzej Hunt <ajrhunt@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
dscho
pushed a commit
that referenced
this issue
Apr 28, 2021
limit_list() iterates over the original revs->commits list, and consumes
many of its entries via pop_commit. However we might stop iterating over
the list early (e.g. if we realise that the rest of the list is
uninteresting). If we do stop iterating early, list will be pointing to
the unconsumed portion of revs->commits - and we need to free this list
to avoid a leak. (revs->commits itself will be an invalid pointer: it
will have been free'd during the first pop_commit.)
However the list pointer is later reused to iterate over our new list,
but only for the limiting_can_increase_treesame() branch. We therefore
need to introduce a new variable for that branch - and while we're here
we can rename the original list to original_list as that makes its
purpose more obvious.
This leak was found while running t0090. It's not likely to be very
impactful, but it can happen quite early during some checkout
invocations, and hence seems to be worth fixing:
Direct leak of 16 byte(s) in 1 object(s) allocated from:
#0 0x49a85d in malloc ../projects/compiler-rt/lib/asan/asan_malloc_linux.cpp:145:3
#1 0x9ac084 in do_xmalloc wrapper.c:41:8
#2 0x9ac05a in xmalloc wrapper.c:62:9
#3 0x7175d6 in commit_list_insert commit.c:540:33
#4 0x71800f in commit_list_insert_by_date commit.c:604:9
#5 0x8f8d2e in process_parents revision.c:1128:5
#6 0x8f2f2c in limit_list revision.c:1418:7
#7 0x8f210e in prepare_revision_walk revision.c:3577:7
#8 0x514170 in orphaned_commit_warning builtin/checkout.c:1185:6
#9 0x512f05 in switch_branches builtin/checkout.c:1250:3
#10 0x50f8de in checkout_branch builtin/checkout.c:1646:9
#11 0x50ba12 in checkout_main builtin/checkout.c:2003:9
#12 0x5086c0 in cmd_checkout builtin/checkout.c:2055:8
#13 0x4cd91d in run_builtin git.c:467:11
#14 0x4cb5f3 in handle_builtin git.c:719:3
#15 0x4ccf47 in run_argv git.c:808:4
#16 0x4caf49 in cmd_main git.c:939:19
#17 0x69dc0e in main common-main.c:52:11
#18 0x7faaabd0e349 in __libc_start_main (/lib64/libc.so.6+0x24349)
Indirect leak of 48 byte(s) in 3 object(s) allocated from:
#0 0x49a85d in malloc ../projects/compiler-rt/lib/asan/asan_malloc_linux.cpp:145:3
#1 0x9ac084 in do_xmalloc wrapper.c:41:8
#2 0x9ac05a in xmalloc wrapper.c:62:9
#3 0x717de6 in commit_list_append commit.c:1609:35
#4 0x8f1f9b in prepare_revision_walk revision.c:3554:12
#5 0x514170 in orphaned_commit_warning builtin/checkout.c:1185:6
#6 0x512f05 in switch_branches builtin/checkout.c:1250:3
#7 0x50f8de in checkout_branch builtin/checkout.c:1646:9
#8 0x50ba12 in checkout_main builtin/checkout.c:2003:9
#9 0x5086c0 in cmd_checkout builtin/checkout.c:2055:8
#10 0x4cd91d in run_builtin git.c:467:11
#11 0x4cb5f3 in handle_builtin git.c:719:3
#12 0x4ccf47 in run_argv git.c:808:4
#13 0x4caf49 in cmd_main git.c:939:19
#14 0x69dc0e in main common-main.c:52:11
#15 0x7faaabd0e349 in __libc_start_main (/lib64/libc.so.6+0x24349)
Signed-off-by: Andrzej Hunt <ajrhunt@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
git-for-windows-ci
pushed a commit
that referenced
this issue
Jun 11, 2021
ibuf can be reused for multiple iterations of the loop. Specifically:
deflate() overwrites s.avail_in to show how much of the input buffer
has not been processed yet - and sometimes leaves 'avail_in > 0', in
which case ibuf will be processed again during the loop's subsequent
iteration.
But if we declare ibuf within the loop, then (in theory) we get a new
(and uninitialised) buffer for every iteration. In practice, my compiler
seems to resue the same buffer - meaning that this code does work - but
it doesn't seem safe to rely on this behaviour. MSAN correctly catches
this issue - as soon as we hit the 's.avail_in > 0' condition, we end up
reading from what seems to be uninitialised memory.
Therefore, we move ibuf out of the loop, making this reuse safe.
See MSAN output from t1050-large below - the interesting part is the
ibuf creation at the end, although there's a lot of indirection before
we reach the read from unitialised memory:
==11294==WARNING: MemorySanitizer: use-of-uninitialized-value
#0 0x7f75db58fb1c in crc32_little crc32.c:283:9
#1 0x7f75db58d5b3 in crc32_z crc32.c:220:20
#2 0x7f75db59668c in crc32 crc32.c:242:12
#3 0x8c94f8 in hashwrite csum-file.c:101:15
#4 0x825faf in stream_to_pack bulk-checkin.c:154:5
#5 0x82467b in deflate_to_pack bulk-checkin.c:225:8
#6 0x823ff1 in index_bulk_checkin bulk-checkin.c:264:15
#7 0xa7cff2 in index_stream object-file.c:2234:9
#8 0xa7bff7 in index_fd object-file.c:2256:9
#9 0xa7d22d in index_path object-file.c:2274:7
#10 0xb3c8c9 in add_to_index read-cache.c:802:7
#11 0xb3e039 in add_file_to_index read-cache.c:835:9
#12 0x4a99c3 in add_files add.c:458:7
#13 0x4a7276 in cmd_add add.c:670:18
#14 0x4a1e76 in run_builtin git.c:461:11
#15 0x49e1e7 in handle_builtin git.c:714:3
#16 0x4a0c08 in run_argv git.c:781:4
#17 0x49d5a8 in cmd_main git.c:912:19
#18 0x7974da in main common-main.c:52:11
#19 0x7f75da66f349 in __libc_start_main (/lib64/libc.so.6+0x24349)
#20 0x421bd9 in _start start.S:120
Uninitialized value was stored to memory at
#0 0x7f75db58fa6b in crc32_little crc32.c:283:9
#1 0x7f75db58d5b3 in crc32_z crc32.c:220:20
#2 0x7f75db59668c in crc32 crc32.c:242:12
#3 0x8c94f8 in hashwrite csum-file.c:101:15
#4 0x825faf in stream_to_pack bulk-checkin.c:154:5
#5 0x82467b in deflate_to_pack bulk-checkin.c:225:8
#6 0x823ff1 in index_bulk_checkin bulk-checkin.c:264:15
#7 0xa7cff2 in index_stream object-file.c:2234:9
#8 0xa7bff7 in index_fd object-file.c:2256:9
#9 0xa7d22d in index_path object-file.c:2274:7
#10 0xb3c8c9 in add_to_index read-cache.c:802:7
#11 0xb3e039 in add_file_to_index read-cache.c:835:9
#12 0x4a99c3 in add_files add.c:458:7
#13 0x4a7276 in cmd_add add.c:670:18
#14 0x4a1e76 in run_builtin git.c:461:11
#15 0x49e1e7 in handle_builtin git.c:714:3
#16 0x4a0c08 in run_argv git.c:781:4
#17 0x49d5a8 in cmd_main git.c:912:19
#18 0x7974da in main common-main.c:52:11
#19 0x7f75da66f349 in __libc_start_main (/lib64/libc.so.6+0x24349)
Uninitialized value was stored to memory at
#0 0x447eb9 in __msan_memcpy msan_interceptors.cpp:1558:3
#1 0x7f75db5c2011 in flush_pending deflate.c:746:5
#2 0x7f75db5cafa0 in deflate_stored deflate.c:1815:9
#3 0x7f75db5bb7d2 in deflate deflate.c:1005:34
#4 0xd80b7f in git_deflate zlib.c:244:12
#5 0x825dff in stream_to_pack bulk-checkin.c:140:12
#6 0x82467b in deflate_to_pack bulk-checkin.c:225:8
#7 0x823ff1 in index_bulk_checkin bulk-checkin.c:264:15
#8 0xa7cff2 in index_stream object-file.c:2234:9
#9 0xa7bff7 in index_fd object-file.c:2256:9
#10 0xa7d22d in index_path object-file.c:2274:7
#11 0xb3c8c9 in add_to_index read-cache.c:802:7
#12 0xb3e039 in add_file_to_index read-cache.c:835:9
#13 0x4a99c3 in add_files add.c:458:7
#14 0x4a7276 in cmd_add add.c:670:18
#15 0x4a1e76 in run_builtin git.c:461:11
#16 0x49e1e7 in handle_builtin git.c:714:3
#17 0x4a0c08 in run_argv git.c:781:4
#18 0x49d5a8 in cmd_main git.c:912:19
#19 0x7974da in main common-main.c:52:11
Uninitialized value was stored to memory at
#0 0x447eb9 in __msan_memcpy msan_interceptors.cpp:1558:3
#1 0x7f75db644241 in _tr_stored_block trees.c:873:5
#2 0x7f75db5cad7c in deflate_stored deflate.c:1813:9
#3 0x7f75db5bb7d2 in deflate deflate.c:1005:34
#4 0xd80b7f in git_deflate zlib.c:244:12
#5 0x825dff in stream_to_pack bulk-checkin.c:140:12
#6 0x82467b in deflate_to_pack bulk-checkin.c:225:8
#7 0x823ff1 in index_bulk_checkin bulk-checkin.c:264:15
#8 0xa7cff2 in index_stream object-file.c:2234:9
#9 0xa7bff7 in index_fd object-file.c:2256:9
#10 0xa7d22d in index_path object-file.c:2274:7
#11 0xb3c8c9 in add_to_index read-cache.c:802:7
#12 0xb3e039 in add_file_to_index read-cache.c:835:9
#13 0x4a99c3 in add_files add.c:458:7
#14 0x4a7276 in cmd_add add.c:670:18
#15 0x4a1e76 in run_builtin git.c:461:11
#16 0x49e1e7 in handle_builtin git.c:714:3
#17 0x4a0c08 in run_argv git.c:781:4
#18 0x49d5a8 in cmd_main git.c:912:19
#19 0x7974da in main common-main.c:52:11
Uninitialized value was stored to memory at
#0 0x447eb9 in __msan_memcpy msan_interceptors.cpp:1558:3
#1 0x7f75db5c8fcf in deflate_stored deflate.c:1783:9
#2 0x7f75db5bb7d2 in deflate deflate.c:1005:34
#3 0xd80b7f in git_deflate zlib.c:244:12
#4 0x825dff in stream_to_pack bulk-checkin.c:140:12
#5 0x82467b in deflate_to_pack bulk-checkin.c:225:8
#6 0x823ff1 in index_bulk_checkin bulk-checkin.c:264:15
#7 0xa7cff2 in index_stream object-file.c:2234:9
#8 0xa7bff7 in index_fd object-file.c:2256:9
#9 0xa7d22d in index_path object-file.c:2274:7
#10 0xb3c8c9 in add_to_index read-cache.c:802:7
#11 0xb3e039 in add_file_to_index read-cache.c:835:9
#12 0x4a99c3 in add_files add.c:458:7
#13 0x4a7276 in cmd_add add.c:670:18
#14 0x4a1e76 in run_builtin git.c:461:11
#15 0x49e1e7 in handle_builtin git.c:714:3
#16 0x4a0c08 in run_argv git.c:781:4
#17 0x49d5a8 in cmd_main git.c:912:19
#18 0x7974da in main common-main.c:52:11
#19 0x7f75da66f349 in __libc_start_main (/lib64/libc.so.6+0x24349)
Uninitialized value was stored to memory at
#0 0x447eb9 in __msan_memcpy msan_interceptors.cpp:1558:3
#1 0x7f75db5ea545 in read_buf deflate.c:1181:5
#2 0x7f75db5c97f7 in deflate_stored deflate.c:1791:9
#3 0x7f75db5bb7d2 in deflate deflate.c:1005:34
#4 0xd80b7f in git_deflate zlib.c:244:12
#5 0x825dff in stream_to_pack bulk-checkin.c:140:12
#6 0x82467b in deflate_to_pack bulk-checkin.c:225:8
#7 0x823ff1 in index_bulk_checkin bulk-checkin.c:264:15
#8 0xa7cff2 in index_stream object-file.c:2234:9
#9 0xa7bff7 in index_fd object-file.c:2256:9
#10 0xa7d22d in index_path object-file.c:2274:7
#11 0xb3c8c9 in add_to_index read-cache.c:802:7
#12 0xb3e039 in add_file_to_index read-cache.c:835:9
#13 0x4a99c3 in add_files add.c:458:7
#14 0x4a7276 in cmd_add add.c:670:18
#15 0x4a1e76 in run_builtin git.c:461:11
#16 0x49e1e7 in handle_builtin git.c:714:3
#17 0x4a0c08 in run_argv git.c:781:4
#18 0x49d5a8 in cmd_main git.c:912:19
#19 0x7974da in main common-main.c:52:11
Uninitialized value was created by an allocation of 'ibuf' in the stack frame of function 'stream_to_pack'
#0 0x825710 in stream_to_pack bulk-checkin.c:101
SUMMARY: MemorySanitizer: use-of-uninitialized-value crc32.c:283:9 in crc32_little
Exiting
Signed-off-by: Andrzej Hunt <andrzej@ahunt.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
git-for-windows-ci
pushed a commit
that referenced
this issue
Jun 11, 2021
cache_entry contains an object_id, and compare_ce_content() would
include that field when calling memcmp on a subset of the cache_entry.
Depending on which hashing algorithm is being used, only part of
object_id.hash is actually being used, therefore including it in a
memcmp() is technically incorrect. Instead we choose to exclude the
object_id when calling memcmp(), and call oideq() separately.
This issue was found when running t1700-split-index with MSAN, see MSAN
output below (on my machine, offset 76 corresponds to 4 bytes after the
start of object_id.hash).
Uninitialized bytes in MemcmpInterceptorCommon at offset 76 inside [0x7f60e7c00118, 92)
==27914==WARNING: MemorySanitizer: use-of-uninitialized-value
#0 0x4524ee in memcmp /home/abuild/rpmbuild/BUILD/llvm-11.0.0.src/build/../projects/compiler-rt/lib/msan/../sanitizer_common/sanitizer_common_interceptors.inc:873:10
#1 0xc867ae in compare_ce_content /home/ahunt/git/git/split-index.c:208:8
#2 0xc859fb in prepare_to_write_split_index /home/ahunt/git/git/split-index.c:336:9
#3 0xb4bbca in write_split_index /home/ahunt/git/git/read-cache.c:3107:2
#4 0xb42b4d in write_locked_index /home/ahunt/git/git/read-cache.c:3295:8
#5 0x638058 in try_merge_strategy /home/ahunt/git/git/builtin/merge.c:758:7
#6 0x63057f in cmd_merge /home/ahunt/git/git/builtin/merge.c:1663:9
#7 0x4a1e76 in run_builtin /home/ahunt/git/git/git.c:461:11
#8 0x49e1e7 in handle_builtin /home/ahunt/git/git/git.c:714:3
#9 0x4a0c08 in run_argv /home/ahunt/git/git/git.c:781:4
#10 0x49d5a8 in cmd_main /home/ahunt/git/git/git.c:912:19
#11 0x7974da in main /home/ahunt/git/git/common-main.c:52:11
#12 0x7f60e928e349 in __libc_start_main (/lib64/libc.so.6+0x24349)
#13 0x421bd9 in _start /home/abuild/rpmbuild/BUILD/glibc-2.26/csu/../sysdeps/x86_64/start.S:120
Uninitialized value was stored to memory at
#0 0x447eb9 in __msan_memcpy /home/abuild/rpmbuild/BUILD/llvm-11.0.0.src/build/../projects/compiler-rt/lib/msan/msan_interceptors.cpp:1558:3
#1 0xb4d1e6 in dup_cache_entry /home/ahunt/git/git/read-cache.c:3457:2
#2 0xd214fa in add_entry /home/ahunt/git/git/unpack-trees.c:215:18
#3 0xd1fae0 in keep_entry /home/ahunt/git/git/unpack-trees.c:2276:2
#4 0xd1ff9e in twoway_merge /home/ahunt/git/git/unpack-trees.c:2504:11
#5 0xd27028 in call_unpack_fn /home/ahunt/git/git/unpack-trees.c:593:12
#6 0xd2443d in unpack_nondirectories /home/ahunt/git/git/unpack-trees.c:1106:12
#7 0xd19435 in unpack_callback /home/ahunt/git/git/unpack-trees.c:1306:6
#8 0xd0d7ff in traverse_trees /home/ahunt/git/git/tree-walk.c:532:17
#9 0xd1773a in unpack_trees /home/ahunt/git/git/unpack-trees.c:1683:9
#10 0xdc6370 in checkout /home/ahunt/git/git/merge-ort.c:3590:8
#11 0xdc51c3 in merge_switch_to_result /home/ahunt/git/git/merge-ort.c:3728:7
#12 0xa195a9 in merge_ort_recursive /home/ahunt/git/git/merge-ort-wrappers.c:58:2
#13 0x637fff in try_merge_strategy /home/ahunt/git/git/builtin/merge.c:751:12
#14 0x63057f in cmd_merge /home/ahunt/git/git/builtin/merge.c:1663:9
#15 0x4a1e76 in run_builtin /home/ahunt/git/git/git.c:461:11
#16 0x49e1e7 in handle_builtin /home/ahunt/git/git/git.c:714:3
#17 0x4a0c08 in run_argv /home/ahunt/git/git/git.c:781:4
#18 0x49d5a8 in cmd_main /home/ahunt/git/git/git.c:912:19
#19 0x7974da in main /home/ahunt/git/git/common-main.c:52:11
Uninitialized value was created by a heap allocation
#0 0x44e73d in malloc /home/abuild/rpmbuild/BUILD/llvm-11.0.0.src/build/../projects/compiler-rt/lib/msan/msan_interceptors.cpp:901:3
#1 0xd592f6 in do_xmalloc /home/ahunt/git/git/wrapper.c:41:8
#2 0xd59248 in xmalloc /home/ahunt/git/git/wrapper.c:62:9
#3 0xa17088 in mem_pool_alloc_block /home/ahunt/git/git/mem-pool.c:22:6
#4 0xa16f78 in mem_pool_init /home/ahunt/git/git/mem-pool.c:44:3
#5 0xb481b8 in load_all_cache_entries /home/ahunt/git/git/read-cache.c
#6 0xb44d40 in do_read_index /home/ahunt/git/git/read-cache.c:2298:17
#7 0xb48a1b in read_index_from /home/ahunt/git/git/read-cache.c:2389:8
#8 0xbd5a0b in repo_read_index /home/ahunt/git/git/repository.c:276:8
#9 0xb4bcaf in repo_read_index_unmerged /home/ahunt/git/git/read-cache.c:3326:2
#10 0x62ed26 in cmd_merge /home/ahunt/git/git/builtin/merge.c:1362:6
#11 0x4a1e76 in run_builtin /home/ahunt/git/git/git.c:461:11
#12 0x49e1e7 in handle_builtin /home/ahunt/git/git/git.c:714:3
#13 0x4a0c08 in run_argv /home/ahunt/git/git/git.c:781:4
#14 0x49d5a8 in cmd_main /home/ahunt/git/git/git.c:912:19
#15 0x7974da in main /home/ahunt/git/git/common-main.c:52:11
#16 0x7f60e928e349 in __libc_start_main (/lib64/libc.so.6+0x24349)
SUMMARY: MemorySanitizer: use-of-uninitialized-value /home/abuild/rpmbuild/BUILD/llvm-11.0.0.src/build/../projects/compiler-rt/lib/msan/../sanitizer_common/sanitizer_common_interceptors.inc:873:10 in memcmp
Exiting
Signed-off-by: Andrzej Hunt <andrzej@ahunt.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
git-for-windows-ci
pushed a commit
that referenced
this issue
Jun 15, 2021
cache_entry contains an object_id, and compare_ce_content() would
include that field when calling memcmp on a subset of the cache_entry.
Depending on which hashing algorithm is being used, only part of
object_id.hash is actually being used, therefore including it in a
memcmp() is incorrect. Instead we choose to exclude the object_id when
calling memcmp(), and call oideq() separately.
This issue was found when running t1700-split-index with MSAN, see MSAN
output below (on my machine, offset 76 corresponds to 4 bytes after the
start of object_id.hash).
Uninitialized bytes in MemcmpInterceptorCommon at offset 76 inside [0x7f60e7c00118, 92)
==27914==WARNING: MemorySanitizer: use-of-uninitialized-value
#0 0x4524ee in memcmp /home/abuild/rpmbuild/BUILD/llvm-11.0.0.src/build/../projects/compiler-rt/lib/msan/../sanitizer_common/sanitizer_common_interceptors.inc:873:10
#1 0xc867ae in compare_ce_content /home/ahunt/git/git/split-index.c:208:8
#2 0xc859fb in prepare_to_write_split_index /home/ahunt/git/git/split-index.c:336:9
#3 0xb4bbca in write_split_index /home/ahunt/git/git/read-cache.c:3107:2
#4 0xb42b4d in write_locked_index /home/ahunt/git/git/read-cache.c:3295:8
#5 0x638058 in try_merge_strategy /home/ahunt/git/git/builtin/merge.c:758:7
#6 0x63057f in cmd_merge /home/ahunt/git/git/builtin/merge.c:1663:9
#7 0x4a1e76 in run_builtin /home/ahunt/git/git/git.c:461:11
#8 0x49e1e7 in handle_builtin /home/ahunt/git/git/git.c:714:3
#9 0x4a0c08 in run_argv /home/ahunt/git/git/git.c:781:4
#10 0x49d5a8 in cmd_main /home/ahunt/git/git/git.c:912:19
#11 0x7974da in main /home/ahunt/git/git/common-main.c:52:11
#12 0x7f60e928e349 in __libc_start_main (/lib64/libc.so.6+0x24349)
#13 0x421bd9 in _start /home/abuild/rpmbuild/BUILD/glibc-2.26/csu/../sysdeps/x86_64/start.S:120
Uninitialized value was stored to memory at
#0 0x447eb9 in __msan_memcpy /home/abuild/rpmbuild/BUILD/llvm-11.0.0.src/build/../projects/compiler-rt/lib/msan/msan_interceptors.cpp:1558:3
#1 0xb4d1e6 in dup_cache_entry /home/ahunt/git/git/read-cache.c:3457:2
#2 0xd214fa in add_entry /home/ahunt/git/git/unpack-trees.c:215:18
#3 0xd1fae0 in keep_entry /home/ahunt/git/git/unpack-trees.c:2276:2
#4 0xd1ff9e in twoway_merge /home/ahunt/git/git/unpack-trees.c:2504:11
#5 0xd27028 in call_unpack_fn /home/ahunt/git/git/unpack-trees.c:593:12
#6 0xd2443d in unpack_nondirectories /home/ahunt/git/git/unpack-trees.c:1106:12
#7 0xd19435 in unpack_callback /home/ahunt/git/git/unpack-trees.c:1306:6
#8 0xd0d7ff in traverse_trees /home/ahunt/git/git/tree-walk.c:532:17
#9 0xd1773a in unpack_trees /home/ahunt/git/git/unpack-trees.c:1683:9
#10 0xdc6370 in checkout /home/ahunt/git/git/merge-ort.c:3590:8
#11 0xdc51c3 in merge_switch_to_result /home/ahunt/git/git/merge-ort.c:3728:7
#12 0xa195a9 in merge_ort_recursive /home/ahunt/git/git/merge-ort-wrappers.c:58:2
#13 0x637fff in try_merge_strategy /home/ahunt/git/git/builtin/merge.c:751:12
#14 0x63057f in cmd_merge /home/ahunt/git/git/builtin/merge.c:1663:9
#15 0x4a1e76 in run_builtin /home/ahunt/git/git/git.c:461:11
#16 0x49e1e7 in handle_builtin /home/ahunt/git/git/git.c:714:3
#17 0x4a0c08 in run_argv /home/ahunt/git/git/git.c:781:4
#18 0x49d5a8 in cmd_main /home/ahunt/git/git/git.c:912:19
#19 0x7974da in main /home/ahunt/git/git/common-main.c:52:11
Uninitialized value was created by a heap allocation
#0 0x44e73d in malloc /home/abuild/rpmbuild/BUILD/llvm-11.0.0.src/build/../projects/compiler-rt/lib/msan/msan_interceptors.cpp:901:3
#1 0xd592f6 in do_xmalloc /home/ahunt/git/git/wrapper.c:41:8
#2 0xd59248 in xmalloc /home/ahunt/git/git/wrapper.c:62:9
#3 0xa17088 in mem_pool_alloc_block /home/ahunt/git/git/mem-pool.c:22:6
#4 0xa16f78 in mem_pool_init /home/ahunt/git/git/mem-pool.c:44:3
#5 0xb481b8 in load_all_cache_entries /home/ahunt/git/git/read-cache.c
#6 0xb44d40 in do_read_index /home/ahunt/git/git/read-cache.c:2298:17
#7 0xb48a1b in read_index_from /home/ahunt/git/git/read-cache.c:2389:8
#8 0xbd5a0b in repo_read_index /home/ahunt/git/git/repository.c:276:8
#9 0xb4bcaf in repo_read_index_unmerged /home/ahunt/git/git/read-cache.c:3326:2
#10 0x62ed26 in cmd_merge /home/ahunt/git/git/builtin/merge.c:1362:6
#11 0x4a1e76 in run_builtin /home/ahunt/git/git/git.c:461:11
#12 0x49e1e7 in handle_builtin /home/ahunt/git/git/git.c:714:3
#13 0x4a0c08 in run_argv /home/ahunt/git/git/git.c:781:4
#14 0x49d5a8 in cmd_main /home/ahunt/git/git/git.c:912:19
#15 0x7974da in main /home/ahunt/git/git/common-main.c:52:11
#16 0x7f60e928e349 in __libc_start_main (/lib64/libc.so.6+0x24349)
SUMMARY: MemorySanitizer: use-of-uninitialized-value /home/abuild/rpmbuild/BUILD/llvm-11.0.0.src/build/../projects/compiler-rt/lib/msan/../sanitizer_common/sanitizer_common_interceptors.inc:873:10 in memcmp
Exiting
Signed-off-by: Andrzej Hunt <andrzej@ahunt.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
git-for-windows-ci
pushed a commit
that referenced
this issue
Jul 28, 2021
apply_multi_file_filter and async_query_available_blobs both query
subprocess output using subprocess_read_status, which writes data into
the identically named filter_status strbuf. We add a strbuf_release to
avoid leaking their contents.
Leak output seen when running t0021 with LSAN:
Direct leak of 24 byte(s) in 1 object(s) allocated from:
#0 0x49ab49 in realloc ../projects/compiler-rt/lib/asan/asan_malloc_linux.cpp:164:3
#1 0xa8c2b5 in xrealloc wrapper.c:126:8
#2 0x9ff99d in strbuf_grow strbuf.c:98:2
#3 0x9ff99d in strbuf_addbuf strbuf.c:304:2
#4 0xa101d6 in subprocess_read_status sub-process.c:45:5
#5 0x77793c in apply_multi_file_filter convert.c:886:8
#6 0x77793c in apply_filter convert.c:1042:10
#7 0x77a0b5 in convert_to_git_filter_fd convert.c:1492:7
#8 0x8b48cd in index_stream_convert_blob object-file.c:2156:2
#9 0x8b48cd in index_fd object-file.c:2248:9
#10 0x597411 in hash_fd builtin/hash-object.c:43:9
#11 0x596be1 in hash_object builtin/hash-object.c:59:2
#12 0x596be1 in cmd_hash_object builtin/hash-object.c:153:3
#13 0x4ce83e in run_builtin git.c:475:11
#14 0x4ccafe in handle_builtin git.c:729:3
#15 0x4cb01c in run_argv git.c:818:4
#16 0x4cb01c in cmd_main git.c:949:19
#17 0x6bdc2d in main common-main.c:52:11
#18 0x7f42acf79349 in __libc_start_main (/lib64/libc.so.6+0x24349)
SUMMARY: AddressSanitizer: 24 byte(s) leaked in 1 allocation(s).
Direct leak of 120 byte(s) in 5 object(s) allocated from:
#0 0x49ab49 in realloc ../projects/compiler-rt/lib/asan/asan_malloc_linux.cpp:164:3
#1 0xa8c295 in xrealloc wrapper.c:126:8
#2 0x9ff97d in strbuf_grow strbuf.c:98:2
#3 0x9ff97d in strbuf_addbuf strbuf.c:304:2
#4 0xa101b6 in subprocess_read_status sub-process.c:45:5
#5 0x775c73 in async_query_available_blobs convert.c:960:8
#6 0x80029d in finish_delayed_checkout entry.c:183:9
#7 0xa65d1e in check_updates unpack-trees.c:493:10
#8 0xa5f469 in unpack_trees unpack-trees.c:1747:8
#9 0x525971 in checkout builtin/clone.c:815:6
#10 0x525971 in cmd_clone builtin/clone.c:1409:8
#11 0x4ce83e in run_builtin git.c:475:11
#12 0x4ccafe in handle_builtin git.c:729:3
#13 0x4cb01c in run_argv git.c:818:4
#14 0x4cb01c in cmd_main git.c:949:19
#15 0x6bdc2d in main common-main.c:52:11
#16 0x7fa253fce349 in __libc_start_main (/lib64/libc.so.6+0x24349)
SUMMARY: AddressSanitizer: 120 byte(s) leaked in 5 allocation(s).
Signed-off-by: Andrzej Hunt <andrzej@ahunt.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
git-for-windows-ci
pushed a commit
that referenced
this issue
Oct 6, 2021
In a sparse index it is possible for the tree that is being verified
to be freed while it is being verified. This happens when
index_name_pos() looks up a entry that is missing from the index and
that would be a descendant of a sparse entry. That triggers a call to
ensure_full_index() which frees the cache tree that is being verified.
Carrying on trying to verify the tree after this results in a
use-after-free bug. Instead restart the verification if a sparse index
is converted to a full index. This bug is triggered by a call to
reset_head() in "git rebase --apply". Thanks to René Scharfe for his
help analyzing the problem.
==74345==ERROR: AddressSanitizer: heap-use-after-free on address 0x606000001b20 at pc 0x557cbe82d3a2 bp 0x7ffdfee08090 sp 0x7ffdfee08080
READ of size 4 at 0x606000001b20 thread T0
#0 0x557cbe82d3a1 in verify_one /home/phil/src/git/cache-tree.c:863
#1 0x557cbe82ca9d in verify_one /home/phil/src/git/cache-tree.c:840
#2 0x557cbe82ca9d in verify_one /home/phil/src/git/cache-tree.c:840
#3 0x557cbe82ca9d in verify_one /home/phil/src/git/cache-tree.c:840
#4 0x557cbe830a2b in cache_tree_verify /home/phil/src/git/cache-tree.c:910
#5 0x557cbea53741 in write_locked_index /home/phil/src/git/read-cache.c:3250
#6 0x557cbeab7fdd in reset_head /home/phil/src/git/reset.c:87
#7 0x557cbe72147f in cmd_rebase builtin/rebase.c:2074
#8 0x557cbe5bd151 in run_builtin /home/phil/src/git/git.c:461
#9 0x557cbe5bd151 in handle_builtin /home/phil/src/git/git.c:714
#10 0x557cbe5c0503 in run_argv /home/phil/src/git/git.c:781
#11 0x557cbe5c0503 in cmd_main /home/phil/src/git/git.c:912
#12 0x557cbe5bad28 in main /home/phil/src/git/common-main.c:52
#13 0x7fdd4b82eb24 in __libc_start_main (/usr/lib/libc.so.6+0x27b24)
#14 0x557cbe5bcb8d in _start (/home/phil/src/git/git+0x1b9b8d)
0x606000001b20 is located 0 bytes inside of 56-byte region [0x606000001b20,0x606000001b58)
freed by thread T0 here:
#0 0x7fdd4bacff19 in __interceptor_free /build/gcc/src/gcc/libsanitizer/asan/asan_malloc_linux.cpp:127
#1 0x557cbe82af60 in cache_tree_free /home/phil/src/git/cache-tree.c:35
#2 0x557cbe82aee5 in cache_tree_free /home/phil/src/git/cache-tree.c:31
#3 0x557cbe82aee5 in cache_tree_free /home/phil/src/git/cache-tree.c:31
#4 0x557cbe82aee5 in cache_tree_free /home/phil/src/git/cache-tree.c:31
#5 0x557cbeb2557a in ensure_full_index /home/phil/src/git/sparse-index.c:310
#6 0x557cbea45c4a in index_name_stage_pos /home/phil/src/git/read-cache.c:588
#7 0x557cbe82ce37 in verify_one /home/phil/src/git/cache-tree.c:850
#8 0x557cbe82ca9d in verify_one /home/phil/src/git/cache-tree.c:840
#9 0x557cbe82ca9d in verify_one /home/phil/src/git/cache-tree.c:840
#10 0x557cbe82ca9d in verify_one /home/phil/src/git/cache-tree.c:840
#11 0x557cbe830a2b in cache_tree_verify /home/phil/src/git/cache-tree.c:910
#12 0x557cbea53741 in write_locked_index /home/phil/src/git/read-cache.c:3250
#13 0x557cbeab7fdd in reset_head /home/phil/src/git/reset.c:87
#14 0x557cbe72147f in cmd_rebase builtin/rebase.c:2074
#15 0x557cbe5bd151 in run_builtin /home/phil/src/git/git.c:461
#16 0x557cbe5bd151 in handle_builtin /home/phil/src/git/git.c:714
#17 0x557cbe5c0503 in run_argv /home/phil/src/git/git.c:781
#18 0x557cbe5c0503 in cmd_main /home/phil/src/git/git.c:912
#19 0x557cbe5bad28 in main /home/phil/src/git/common-main.c:52
#20 0x7fdd4b82eb24 in __libc_start_main (/usr/lib/libc.so.6+0x27b24)
previously allocated by thread T0 here:
#0 0x7fdd4bad0459 in __interceptor_calloc /build/gcc/src/gcc/libsanitizer/asan/asan_malloc_linux.cpp:154
#1 0x557cbebc1807 in xcalloc /home/phil/src/git/wrapper.c:140
#2 0x557cbe82b7d8 in cache_tree /home/phil/src/git/cache-tree.c:17
#3 0x557cbe82b7d8 in prime_cache_tree_rec /home/phil/src/git/cache-tree.c:763
#4 0x557cbe82b837 in prime_cache_tree_rec /home/phil/src/git/cache-tree.c:764
#5 0x557cbe82b837 in prime_cache_tree_rec /home/phil/src/git/cache-tree.c:764
#6 0x557cbe8304e1 in prime_cache_tree /home/phil/src/git/cache-tree.c:779
#7 0x557cbeab7fa7 in reset_head /home/phil/src/git/reset.c:85
#8 0x557cbe72147f in cmd_rebase builtin/rebase.c:2074
#9 0x557cbe5bd151 in run_builtin /home/phil/src/git/git.c:461
#10 0x557cbe5bd151 in handle_builtin /home/phil/src/git/git.c:714
#11 0x557cbe5c0503 in run_argv /home/phil/src/git/git.c:781
#12 0x557cbe5c0503 in cmd_main /home/phil/src/git/git.c:912
#13 0x557cbe5bad28 in main /home/phil/src/git/common-main.c:52
#14 0x7fdd4b82eb24 in __libc_start_main (/usr/lib/libc.so.6+0x27b24)
Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
git-for-windows-ci
pushed a commit
that referenced
this issue
Oct 7, 2021
In a sparse index it is possible for the tree that is being verified
to be freed while it is being verified. This happens when the index is
sparse but the cache tree is not and index_name_pos() looks up a path
from the cache tree that is a descendant of a sparse index entry. That
triggers a call to ensure_full_index() which frees the cache tree that
is being verified. Carrying on trying to verify the tree after this
results in a use-after-free bug. Instead restart the verification if a
sparse index is converted to a full index. This bug is triggered by a
call to reset_head() in "git rebase --apply". Thanks to René Scharfe
and Derrick Stolee for their help analyzing the problem.
==74345==ERROR: AddressSanitizer: heap-use-after-free on address 0x606000001b20 at pc 0x557cbe82d3a2 bp 0x7ffdfee08090 sp 0x7ffdfee08080
READ of size 4 at 0x606000001b20 thread T0
#0 0x557cbe82d3a1 in verify_one /home/phil/src/git/cache-tree.c:863
#1 0x557cbe82ca9d in verify_one /home/phil/src/git/cache-tree.c:840
#2 0x557cbe82ca9d in verify_one /home/phil/src/git/cache-tree.c:840
#3 0x557cbe82ca9d in verify_one /home/phil/src/git/cache-tree.c:840
#4 0x557cbe830a2b in cache_tree_verify /home/phil/src/git/cache-tree.c:910
#5 0x557cbea53741 in write_locked_index /home/phil/src/git/read-cache.c:3250
#6 0x557cbeab7fdd in reset_head /home/phil/src/git/reset.c:87
#7 0x557cbe72147f in cmd_rebase builtin/rebase.c:2074
#8 0x557cbe5bd151 in run_builtin /home/phil/src/git/git.c:461
#9 0x557cbe5bd151 in handle_builtin /home/phil/src/git/git.c:714
#10 0x557cbe5c0503 in run_argv /home/phil/src/git/git.c:781
#11 0x557cbe5c0503 in cmd_main /home/phil/src/git/git.c:912
#12 0x557cbe5bad28 in main /home/phil/src/git/common-main.c:52
#13 0x7fdd4b82eb24 in __libc_start_main (/usr/lib/libc.so.6+0x27b24)
#14 0x557cbe5bcb8d in _start (/home/phil/src/git/git+0x1b9b8d)
0x606000001b20 is located 0 bytes inside of 56-byte region [0x606000001b20,0x606000001b58)
freed by thread T0 here:
#0 0x7fdd4bacff19 in __interceptor_free /build/gcc/src/gcc/libsanitizer/asan/asan_malloc_linux.cpp:127
#1 0x557cbe82af60 in cache_tree_free /home/phil/src/git/cache-tree.c:35
#2 0x557cbe82aee5 in cache_tree_free /home/phil/src/git/cache-tree.c:31
#3 0x557cbe82aee5 in cache_tree_free /home/phil/src/git/cache-tree.c:31
#4 0x557cbe82aee5 in cache_tree_free /home/phil/src/git/cache-tree.c:31
#5 0x557cbeb2557a in ensure_full_index /home/phil/src/git/sparse-index.c:310
#6 0x557cbea45c4a in index_name_stage_pos /home/phil/src/git/read-cache.c:588
#7 0x557cbe82ce37 in verify_one /home/phil/src/git/cache-tree.c:850
#8 0x557cbe82ca9d in verify_one /home/phil/src/git/cache-tree.c:840
#9 0x557cbe82ca9d in verify_one /home/phil/src/git/cache-tree.c:840
#10 0x557cbe82ca9d in verify_one /home/phil/src/git/cache-tree.c:840
#11 0x557cbe830a2b in cache_tree_verify /home/phil/src/git/cache-tree.c:910
#12 0x557cbea53741 in write_locked_index /home/phil/src/git/read-cache.c:3250
#13 0x557cbeab7fdd in reset_head /home/phil/src/git/reset.c:87
#14 0x557cbe72147f in cmd_rebase builtin/rebase.c:2074
#15 0x557cbe5bd151 in run_builtin /home/phil/src/git/git.c:461
#16 0x557cbe5bd151 in handle_builtin /home/phil/src/git/git.c:714
#17 0x557cbe5c0503 in run_argv /home/phil/src/git/git.c:781
#18 0x557cbe5c0503 in cmd_main /home/phil/src/git/git.c:912
#19 0x557cbe5bad28 in main /home/phil/src/git/common-main.c:52
#20 0x7fdd4b82eb24 in __libc_start_main (/usr/lib/libc.so.6+0x27b24)
previously allocated by thread T0 here:
#0 0x7fdd4bad0459 in __interceptor_calloc /build/gcc/src/gcc/libsanitizer/asan/asan_malloc_linux.cpp:154
#1 0x557cbebc1807 in xcalloc /home/phil/src/git/wrapper.c:140
#2 0x557cbe82b7d8 in cache_tree /home/phil/src/git/cache-tree.c:17
#3 0x557cbe82b7d8 in prime_cache_tree_rec /home/phil/src/git/cache-tree.c:763
#4 0x557cbe82b837 in prime_cache_tree_rec /home/phil/src/git/cache-tree.c:764
#5 0x557cbe82b837 in prime_cache_tree_rec /home/phil/src/git/cache-tree.c:764
#6 0x557cbe8304e1 in prime_cache_tree /home/phil/src/git/cache-tree.c:779
#7 0x557cbeab7fa7 in reset_head /home/phil/src/git/reset.c:85
#8 0x557cbe72147f in cmd_rebase builtin/rebase.c:2074
#9 0x557cbe5bd151 in run_builtin /home/phil/src/git/git.c:461
#10 0x557cbe5bd151 in handle_builtin /home/phil/src/git/git.c:714
#11 0x557cbe5c0503 in run_argv /home/phil/src/git/git.c:781
#12 0x557cbe5c0503 in cmd_main /home/phil/src/git/git.c:912
#13 0x557cbe5bad28 in main /home/phil/src/git/common-main.c:52
#14 0x7fdd4b82eb24 in __libc_start_main (/usr/lib/libc.so.6+0x27b24)
Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
jeffhostetler
pushed a commit
to jeffhostetler/git
that referenced
this issue
Oct 13, 2021
In a sparse index it is possible for the tree that is being verified
to be freed while it is being verified. This happens when the index is
sparse but the cache tree is not and index_name_pos() looks up a path
from the cache tree that is a descendant of a sparse index entry. That
triggers a call to ensure_full_index() which frees the cache tree that
is being verified. Carrying on trying to verify the tree after this
results in a use-after-free bug. Instead restart the verification if a
sparse index is converted to a full index. This bug is triggered by a
call to reset_head() in "git rebase --apply". Thanks to René Scharfe
and Derrick Stolee for their help analyzing the problem.
==74345==ERROR: AddressSanitizer: heap-use-after-free on address 0x606000001b20 at pc 0x557cbe82d3a2 bp 0x7ffdfee08090 sp 0x7ffdfee08080
READ of size 4 at 0x606000001b20 thread T0
#0 0x557cbe82d3a1 in verify_one /home/phil/src/git/cache-tree.c:863
#1 0x557cbe82ca9d in verify_one /home/phil/src/git/cache-tree.c:840
#2 0x557cbe82ca9d in verify_one /home/phil/src/git/cache-tree.c:840
#3 0x557cbe82ca9d in verify_one /home/phil/src/git/cache-tree.c:840
#4 0x557cbe830a2b in cache_tree_verify /home/phil/src/git/cache-tree.c:910
#5 0x557cbea53741 in write_locked_index /home/phil/src/git/read-cache.c:3250
#6 0x557cbeab7fdd in reset_head /home/phil/src/git/reset.c:87
#7 0x557cbe72147f in cmd_rebase builtin/rebase.c:2074
#8 0x557cbe5bd151 in run_builtin /home/phil/src/git/git.c:461
#9 0x557cbe5bd151 in handle_builtin /home/phil/src/git/git.c:714
#10 0x557cbe5c0503 in run_argv /home/phil/src/git/git.c:781
#11 0x557cbe5c0503 in cmd_main /home/phil/src/git/git.c:912
#12 0x557cbe5bad28 in main /home/phil/src/git/common-main.c:52
#13 0x7fdd4b82eb24 in __libc_start_main (/usr/lib/libc.so.6+0x27b24)
git-for-windows#14 0x557cbe5bcb8d in _start (/home/phil/src/git/git+0x1b9b8d)
0x606000001b20 is located 0 bytes inside of 56-byte region [0x606000001b20,0x606000001b58)
freed by thread T0 here:
#0 0x7fdd4bacff19 in __interceptor_free /build/gcc/src/gcc/libsanitizer/asan/asan_malloc_linux.cpp:127
#1 0x557cbe82af60 in cache_tree_free /home/phil/src/git/cache-tree.c:35
#2 0x557cbe82aee5 in cache_tree_free /home/phil/src/git/cache-tree.c:31
#3 0x557cbe82aee5 in cache_tree_free /home/phil/src/git/cache-tree.c:31
#4 0x557cbe82aee5 in cache_tree_free /home/phil/src/git/cache-tree.c:31
#5 0x557cbeb2557a in ensure_full_index /home/phil/src/git/sparse-index.c:310
#6 0x557cbea45c4a in index_name_stage_pos /home/phil/src/git/read-cache.c:588
#7 0x557cbe82ce37 in verify_one /home/phil/src/git/cache-tree.c:850
#8 0x557cbe82ca9d in verify_one /home/phil/src/git/cache-tree.c:840
#9 0x557cbe82ca9d in verify_one /home/phil/src/git/cache-tree.c:840
#10 0x557cbe82ca9d in verify_one /home/phil/src/git/cache-tree.c:840
#11 0x557cbe830a2b in cache_tree_verify /home/phil/src/git/cache-tree.c:910
#12 0x557cbea53741 in write_locked_index /home/phil/src/git/read-cache.c:3250
#13 0x557cbeab7fdd in reset_head /home/phil/src/git/reset.c:87
git-for-windows#14 0x557cbe72147f in cmd_rebase builtin/rebase.c:2074
git-for-windows#15 0x557cbe5bd151 in run_builtin /home/phil/src/git/git.c:461
git-for-windows#16 0x557cbe5bd151 in handle_builtin /home/phil/src/git/git.c:714
git-for-windows#17 0x557cbe5c0503 in run_argv /home/phil/src/git/git.c:781
git-for-windows#18 0x557cbe5c0503 in cmd_main /home/phil/src/git/git.c:912
git-for-windows#19 0x557cbe5bad28 in main /home/phil/src/git/common-main.c:52
git-for-windows#20 0x7fdd4b82eb24 in __libc_start_main (/usr/lib/libc.so.6+0x27b24)
previously allocated by thread T0 here:
#0 0x7fdd4bad0459 in __interceptor_calloc /build/gcc/src/gcc/libsanitizer/asan/asan_malloc_linux.cpp:154
#1 0x557cbebc1807 in xcalloc /home/phil/src/git/wrapper.c:140
#2 0x557cbe82b7d8 in cache_tree /home/phil/src/git/cache-tree.c:17
#3 0x557cbe82b7d8 in prime_cache_tree_rec /home/phil/src/git/cache-tree.c:763
#4 0x557cbe82b837 in prime_cache_tree_rec /home/phil/src/git/cache-tree.c:764
#5 0x557cbe82b837 in prime_cache_tree_rec /home/phil/src/git/cache-tree.c:764
#6 0x557cbe8304e1 in prime_cache_tree /home/phil/src/git/cache-tree.c:779
#7 0x557cbeab7fa7 in reset_head /home/phil/src/git/reset.c:85
#8 0x557cbe72147f in cmd_rebase builtin/rebase.c:2074
#9 0x557cbe5bd151 in run_builtin /home/phil/src/git/git.c:461
#10 0x557cbe5bd151 in handle_builtin /home/phil/src/git/git.c:714
#11 0x557cbe5c0503 in run_argv /home/phil/src/git/git.c:781
#12 0x557cbe5c0503 in cmd_main /home/phil/src/git/git.c:912
#13 0x557cbe5bad28 in main /home/phil/src/git/common-main.c:52
git-for-windows#14 0x7fdd4b82eb24 in __libc_start_main (/usr/lib/libc.so.6+0x27b24)
Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
git-for-windows-ci
pushed a commit
that referenced
this issue
Jan 10, 2022
When fetching packfiles, we write a bunch of lockfiles for the packfiles
we're writing into the repository. In order to not leave behind any
cruft in case we exit or receive a signal, we register both an exit
handler as well as signal handlers for common signals like SIGINT. These
handlers will then unlink the locks and free the data structure tracking
them. We have observed a deadlock in this logic though:
(gdb) bt
#0 __lll_lock_wait_private () at ../sysdeps/unix/sysv/linux/x86_64/lowlevellock.S:95
#1 0x00007f4932bea2cd in _int_free (av=0x7f4932f2eb20 <main_arena>, p=0x3e3e4200, have_lock=0) at malloc.c:3969
#2 0x00007f4932bee58c in __GI___libc_free (mem=<optimized out>) at malloc.c:2975
#3 0x0000000000662ab1 in string_list_clear ()
#4 0x000000000044f5bc in unlock_pack_on_signal ()
#5 <signal handler called>
#6 _int_free (av=0x7f4932f2eb20 <main_arena>, p=<optimized out>, have_lock=0) at malloc.c:4024
#7 0x00007f4932bee58c in __GI___libc_free (mem=<optimized out>) at malloc.c:2975
#8 0x000000000065afd5 in strbuf_release ()
#9 0x000000000066ddb9 in delete_tempfile ()
#10 0x0000000000610d0b in files_transaction_cleanup.isra ()
#11 0x0000000000611718 in files_transaction_abort ()
#12 0x000000000060d2ef in ref_transaction_abort ()
#13 0x000000000060d441 in ref_transaction_prepare ()
#14 0x000000000060e0b5 in ref_transaction_commit ()
#15 0x00000000004511c2 in fetch_and_consume_refs ()
#16 0x000000000045279a in cmd_fetch ()
#17 0x0000000000407c48 in handle_builtin ()
#18 0x0000000000408df2 in cmd_main ()
#19 0x00000000004078b5 in main ()
The process was killed with a signal, which caused the signal handler to
kick in and try free the data structures after we have unlinked the
locks. It then deadlocks while calling free(3P).
The root cause of this is that it is not allowed to call certain
functions in async-signal handlers, as specified by signal-safety(7).
Next to most I/O functions, this list of disallowed functions also
includes memory-handling functions like malloc(3P) and free(3P) because
they may not be reentrant. As a result, if we execute such functions in
the signal handler, then they may operate on inconistent state and fail
in unexpected ways.
Fix this bug by not calling non-async-signal-safe functions when running
in the signal handler. We're about to re-raise the signal anyway and
will thus exit, so it's not much of a problem to keep the string list of
lockfiles untouched. Note that it's fine though to call unlink(2), so
we'll still clean up the lockfiles correctly.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Reviewed-by: brian m. carlson <sandals@crustytoothpaste.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
git-for-windows-ci
pushed a commit
that referenced
this issue
Jan 14, 2022
The fix is short (~30 lines), but the description is not. Sorry.
There is a set of problems caused by files in what I'll refer to as the
"present-despite-SKIP_WORKTREE" state. This commit aims to not just fix
these problems, but remove the entire class as a possibility -- for
those using sparse checkouts. But first, we need to understand the
problems this class presents. A quick outline:
* Problems
* User facing issues
* Problem space complexity
* Maintenance and code correctness challenges
* SKIP_WORKTREE expectations in Git
* Suggested solution
* Pros/Cons of suggested solution
* Notes on testcase modifications
=== User facing issues ===
There are various ways for users to get files to be present in the
working copy despite having the SKIP_WORKTREE bit set for that file in
the index. This may come from:
* various git commands not really supporting the SKIP_WORKTREE bit[1,2]
* users grabbing files from elsewhere and writing them to the worktree
(perhaps even cached in their editor)
* users attempting to "abort" a sparse-checkout operation with a
not-so-early Ctrl+C (updating $GIT_DIR/info/sparse-checkout and the
working tree is not atomic)[3].
Once users have present-despite-SKIP_WORKTREE files, any modifications
users make to these files will be ignored, possibly to users' confusion.
Further:
* these files will degrade performance for the sparse-index case due
to requiring the index to be expanded (see commit 55dfcf9
("sparse-checkout: clear tracked sparse dirs", 2021-09-08) for why
we try to delete entire directories outside the sparse cone).
* these files will not be updated by by standard commands
(switch/checkout/pull/merge/rebase will leave them alone unless
conflicts happen -- and even then, the conflicted file may be
written somewhere else to avoid overwriting the SKIP_WORKTREE file
that is present and in the way)
* there is nothing in Git that users can use to discover such
files (status, diff, grep, etc. all ignore it)
* there is no reasonable mechanism to "recover" from such a condition
(neither `git sparse-checkout reapply` nor `git reset --hard` will
correct it).
So, not only are users modifications ignored, but the files get
progressively more stale over time. At some point in the future, they
may change their sparseness specification or disable sparse-checkouts.
At that time, all present-despite-SKIP_WORKTREE files will show up as
having lots of modifications because they represent a version from a
different branch or commit. These might include user-made local changes
from days before, but the only way to tell is to have users look through
them all closely.
If these users come to others for help, there will be no logs that
explain the issue; it's just a mysterious list of changes. Users might
adamantly claim (correctly, as it turns out) that they didn't modify
these files, while others presume they did.
[1] https://lore.kernel.org/git/xmqqbmb1a7ga.fsf@gitster-ct.c.googlers.com/
[2] https://lore.kernel.org/git/CABPp-BH9tju7WVm=QZDOvaMDdZbpNXrVWQdN-jmfN8wC6YVhmw@mail.gmail.com/
[3] https://lore.kernel.org/git/CABPp-BFnFpzwGC11TLoLs8YK5yiisA5D5-fFjXnJsbESVDwZsA@mail.gmail.com/
=== Problem space complexity ===
SKIP_WORKTREE has been part of Git for over a decade. Duy did lots of
work on it initially, and several others have since come along and put
lots of work into it. Stolee spent most of 2021 on the sparse-index,
with lots of bugfixes along the way including to non-sparse-index cases
as we are still trying to get sparse checkouts to behave reasonably.
Basically every codepath throughout the treat needs to be aware of an
additional type of file: tracked-but-not-present. The extra type
results in lots of extra testcases and lots of extra code everywhere.
But, the sad thing is that we actually have more than one extra type.
We have tracked, tracked-but-not-present (SKIP_WORKTREE), and
tracked-but-promised-to-not-be-present-but-is-present-anyway
(present-despite-SKIP_WORKTREE). Two types is a monumental amount of
effort to support, and adding a third feels a bit like insanity[4].
[4] Some examples of which can be seen at
https://lore.kernel.org/git/CABPp-BGJ_Nvi5TmgriD9Bh6eNXE2EDq2f8e8QKXAeYG3BxZafA@mail.gmail.com/
=== Maintenance and code correctness challenges ===
Matheus' patches to grep stalled for nearly a year, in part because of
complications of how to handle sparse-checkouts appropriately in all
cases[5][6] (with trying to sanely figure out how to sanely handle
present-despite-SKIP_WORKTREE files being one of the complications).
His rm/add follow-ups also took months because of those kinds of
issues[7]. The corner cases with things like submodules and
SKIP_WORKTREE with the addition of present-despite-SKIP_WORKTREE start
becoming really complex[8].
We've had to add ugly logic to merge-ort to attempt to handle
present-despite-SKIP_WORKTREE files[9], and basically just been forced
to give up in merge-recursive knowing full well that we'll sometimes
silently discard user modifications. Despite stash essentially being a
merge, it needed extra code (beyond what was in merge-ort and
merge-recursive) to manually tweak SKIP_WORKTREE bits in order to avoid
a few different bugs that'd result in an early abort with a partial
stash application[10].
[5] See https://lore.kernel.org/git/5f3f7ac77039d41d1692ceae4b0c5df3bb45b74a.1612901326.git.matheus.bernardino@usp.br/#t
and the dates on the thread; also Matheus and I had several
conversations off-list trying to resolve the issues over that time
[6] ...it finally kind of got unstuck after
https://lore.kernel.org/git/CABPp-BGJ_Nvi5TmgriD9Bh6eNXE2EDq2f8e8QKXAeYG3BxZafA@mail.gmail.com/
[7] See for example
https://lore.kernel.org/git/CABPp-BHwNoVnooqDFPAsZxBT9aR5Dwk5D9sDRCvYSb8akxAJgA@mail.gmail.com/#t
and quotes like "The core functionality of sparse-checkout has always
been only partially implemented", a statement I still believe is true
today.
[8] https://lore.kernel.org/git/pull.809.git.git.1592356884310.gitgitgadget@gmail.com/
[9] See commit 66b209b ("merge-ort: implement CE_SKIP_WORKTREE
handling with conflicted entries", 2021-03-20)
[10] See commit ba359fd ("stash: fix stash application in
sparse-checkouts", 2020-12-01)
=== SKIP_WORKTREE expectations in Git ===
A couple quotes:
From [11] (before the "sparse-checkout" command existed):
If it needs too many special cases, hacks, and conditionals, then it
is not worth the complexity---if it is easier to write a correct code
by allowing Git to populate working tree files, it is perfectly fine
to do so.
In a sense, the sparse checkout "feature" itself is a hack by itself,
and that is why I think this part should be "best effort" as well.
From the git-sparse-checkout manual (still present today):
THIS COMMAND IS EXPERIMENTAL. ITS BEHAVIOR, AND THE BEHAVIOR OF OTHER
COMMANDS IN THE PRESENCE OF SPARSE-CHECKOUTS, WILL LIKELY CHANGE IN
THE FUTURE.
[11] https://lore.kernel.org/git/xmqqbmb1a7ga.fsf@gitster-ct.c.googlers.com/
=== Suggested solution ===
SKIP_WORKTREE was written to allow sparse-checkouts, in particular, as
the name of the option implies, to allow the file to NOT be in the
worktree but consider it to be unchanged rather than deleted.
The suggests a simple solution: present-despite-SKIP_WORKTREE files
should not exist, for those using sparse-checkouts.
Enforce this at index loading time by checking if core.sparseCheckout is
true; if so, check files in the index with the SKIP_WORKTREE bit set to
verify that they are absent from the working tree. If they are present,
unset the bit (in memory, though any commands that write to the index
will record the update).
Users can, of course, can get the SKIP_WORKTREE bit back such as by
running `git sparse-checkout reapply` (if they have ensured the file is
unmodified and doesn't match the specified sparsity patterns).
=== Pros/Cons of suggested solution ===
Pros:
* Solves the user visible problems reported above, which I've been
complaining about for nearly a year but couldn't find a solution to.
* Helps prevent slow performance degradation with a sparse-index.
* Much easier behavior in sparse-checkouts for users to reason about
* Very simple, ~30 lines of code.
* Significantly simplifies some ugly testcases, and obviates the need
to test an entire class of potential issues.
* Reduces code complexity, reasoning, and maintenance. Avoids
disagreements about weird corner cases[12].
* It has been reported that some users might be (ab)using
SKIP_WORKTREE as a let-me-modify-but-keep-the-file-in-the-worktree
mechanism[13, and a few other similar references]. These users know
of multiple caveats and shortcomings in doing so; perhaps not
surprising given the "SKIP_WORKTREE expecations" section above.
However, these users use `git update-index --skip-worktree`, and not
`git sparse-checkout` or core.sparseCheckout=true. As such, these
users would be unaffected by this change and can continue abusing
the system as before.
[12] https://lore.kernel.org/git/CABPp-BH9tju7WVm=QZDOvaMDdZbpNXrVWQdN-jmfN8wC6YVhmw@mail.gmail.com/
[13] https://stackoverflow.com/questions/13630849/git-difference-between-assume-unchanged-and-skip-worktree
Cons:
* When core.sparseCheckout is enabled, this adds a performance cost to
reading the index. I'll defer discussion of this cost to a subsequent
patch, since I have some optimizations to add.
=== Notes on testcase modifications ===
The good:
* t1011: Compare to two cases above it ('read-tree will not throw away
dirty changes, non-sparse'); since the file is present, it should
match the non-sparse case now
* t1092: sparse-index & sparse-checkout now match full-worktree
behavior in more cases! Yaay for consistency!
* t6428, t7012: look at how much simpler the tests become! Merge and
stash can just fail early telling the user there's a file in the
way, instead of not noticing until it's about to write a file and
then have to implement sudden crash avoidance. Hurray for sanity!
* t7817: sparse behavior better matches full tree behavior. Hurray
for sanity!
The confusing:
* t3705: These changes were ONLY needed on Windows, but they don't
hurt other platforms. Let's discuss each individually:
* core.sparseCheckout should be false by default. Nothing in this
testcase toggles that until many, many tests later. However,
early tests (#5 in particular) were testing `update-index
--skip-worktree` behavior in a non-sparse-checkout, but the
Windows tests in CI were behaving as if core.sparseCheckout=true
had been specified somewhere. I do not have access to a Windows
machine. But I just manually did what should have been a no-op
and turned the config off. And it fixed the test.
* I have no idea why the leftover .gitattributes file from this
test was causing failures for test #18 on Windows, but only with
these changes of mine. Test #18 was checking for empty stderr,
and specifically wanted to know that some error completely
unrelated to file endings did not appear. The leftover
.gitattributes file thus caused some spurious stderr unrelated to
the thing being checked. Since other tests did not intend to
test normalization, just proactively remove the .gitattributes
file. I'm certain this is cleaner and better, I'm just unsure
why/how this didn't trigger problems before.
Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
git-for-windows-ci
pushed a commit
that referenced
this issue
Jan 14, 2022
The fix is short (~30 lines), but the description is not. Sorry.
There is a set of problems caused by files in what I'll refer to as the
"present-despite-SKIP_WORKTREE" state. This commit aims to not just fix
these problems, but remove the entire class as a possibility -- for
those using sparse checkouts. But first, we need to understand the
problems this class presents. A quick outline:
* Problems
* User facing issues
* Problem space complexity
* Maintenance and code correctness challenges
* SKIP_WORKTREE expectations in Git
* Suggested solution
* Pros/Cons of suggested solution
* Notes on testcase modifications
=== User facing issues ===
There are various ways for users to get files to be present in the
working copy despite having the SKIP_WORKTREE bit set for that file in
the index. This may come from:
* various git commands not really supporting the SKIP_WORKTREE bit[1,2]
* users grabbing files from elsewhere and writing them to the worktree
(perhaps even cached in their editor)
* users attempting to "abort" a sparse-checkout operation with a
not-so-early Ctrl+C (updating $GIT_DIR/info/sparse-checkout and the
working tree is not atomic)[3].
Once users have present-despite-SKIP_WORKTREE files, any modifications
users make to these files will be ignored, possibly to users' confusion.
Further:
* these files will degrade performance for the sparse-index case due
to requiring the index to be expanded (see commit 55dfcf9
("sparse-checkout: clear tracked sparse dirs", 2021-09-08) for why
we try to delete entire directories outside the sparse cone).
* these files will not be updated by by standard commands
(switch/checkout/pull/merge/rebase will leave them alone unless
conflicts happen -- and even then, the conflicted file may be
written somewhere else to avoid overwriting the SKIP_WORKTREE file
that is present and in the way)
* there is nothing in Git that users can use to discover such
files (status, diff, grep, etc. all ignore it)
* there is no reasonable mechanism to "recover" from such a condition
(neither `git sparse-checkout reapply` nor `git reset --hard` will
correct it).
So, not only are users modifications ignored, but the files get
progressively more stale over time. At some point in the future, they
may change their sparseness specification or disable sparse-checkouts.
At that time, all present-despite-SKIP_WORKTREE files will show up as
having lots of modifications because they represent a version from a
different branch or commit. These might include user-made local changes
from days before, but the only way to tell is to have users look through
them all closely.
If these users come to others for help, there will be no logs that
explain the issue; it's just a mysterious list of changes. Users might
adamantly claim (correctly, as it turns out) that they didn't modify
these files, while others presume they did.
[1] https://lore.kernel.org/git/xmqqbmb1a7ga.fsf@gitster-ct.c.googlers.com/
[2] https://lore.kernel.org/git/CABPp-BH9tju7WVm=QZDOvaMDdZbpNXrVWQdN-jmfN8wC6YVhmw@mail.gmail.com/
[3] https://lore.kernel.org/git/CABPp-BFnFpzwGC11TLoLs8YK5yiisA5D5-fFjXnJsbESVDwZsA@mail.gmail.com/
=== Problem space complexity ===
SKIP_WORKTREE has been part of Git for over a decade. Duy did lots of
work on it initially, and several others have since come along and put
lots of work into it. Stolee spent most of 2021 on the sparse-index,
with lots of bugfixes along the way including to non-sparse-index cases
as we are still trying to get sparse checkouts to behave reasonably.
Basically every codepath throughout the treat needs to be aware of an
additional type of file: tracked-but-not-present. The extra type
results in lots of extra testcases and lots of extra code everywhere.
But, the sad thing is that we actually have more than one extra type.
We have tracked, tracked-but-not-present (SKIP_WORKTREE), and
tracked-but-promised-to-not-be-present-but-is-present-anyway
(present-despite-SKIP_WORKTREE). Two types is a monumental amount of
effort to support, and adding a third feels a bit like insanity[4].
[4] Some examples of which can be seen at
https://lore.kernel.org/git/CABPp-BGJ_Nvi5TmgriD9Bh6eNXE2EDq2f8e8QKXAeYG3BxZafA@mail.gmail.com/
=== Maintenance and code correctness challenges ===
Matheus' patches to grep stalled for nearly a year, in part because of
complications of how to handle sparse-checkouts appropriately in all
cases[5][6] (with trying to sanely figure out how to sanely handle
present-despite-SKIP_WORKTREE files being one of the complications).
His rm/add follow-ups also took months because of those kinds of
issues[7]. The corner cases with things like submodules and
SKIP_WORKTREE with the addition of present-despite-SKIP_WORKTREE start
becoming really complex[8].
We've had to add ugly logic to merge-ort to attempt to handle
present-despite-SKIP_WORKTREE files[9], and basically just been forced
to give up in merge-recursive knowing full well that we'll sometimes
silently discard user modifications. Despite stash essentially being a
merge, it needed extra code (beyond what was in merge-ort and
merge-recursive) to manually tweak SKIP_WORKTREE bits in order to avoid
a few different bugs that'd result in an early abort with a partial
stash application[10].
[5] See https://lore.kernel.org/git/5f3f7ac77039d41d1692ceae4b0c5df3bb45b74a.1612901326.git.matheus.bernardino@usp.br/#t
and the dates on the thread; also Matheus and I had several
conversations off-list trying to resolve the issues over that time
[6] ...it finally kind of got unstuck after
https://lore.kernel.org/git/CABPp-BGJ_Nvi5TmgriD9Bh6eNXE2EDq2f8e8QKXAeYG3BxZafA@mail.gmail.com/
[7] See for example
https://lore.kernel.org/git/CABPp-BHwNoVnooqDFPAsZxBT9aR5Dwk5D9sDRCvYSb8akxAJgA@mail.gmail.com/#t
and quotes like "The core functionality of sparse-checkout has always
been only partially implemented", a statement I still believe is true
today.
[8] https://lore.kernel.org/git/pull.809.git.git.1592356884310.gitgitgadget@gmail.com/
[9] See commit 66b209b ("merge-ort: implement CE_SKIP_WORKTREE
handling with conflicted entries", 2021-03-20)
[10] See commit ba359fd ("stash: fix stash application in
sparse-checkouts", 2020-12-01)
=== SKIP_WORKTREE expectations in Git ===
A couple quotes:
* From [11] (before the "sparse-checkout" command existed):
If it needs too many special cases, hacks, and conditionals, then it
is not worth the complexity---if it is easier to write a correct code
by allowing Git to populate working tree files, it is perfectly fine
to do so.
In a sense, the sparse checkout "feature" itself is a hack by itself,
and that is why I think this part should be "best effort" as well.
* From the git-sparse-checkout manual (still present today):
THIS COMMAND IS EXPERIMENTAL. ITS BEHAVIOR, AND THE BEHAVIOR OF OTHER
COMMANDS IN THE PRESENCE OF SPARSE-CHECKOUTS, WILL LIKELY CHANGE IN
THE FUTURE.
[11] https://lore.kernel.org/git/xmqqbmb1a7ga.fsf@gitster-ct.c.googlers.com/
=== Suggested solution ===
SKIP_WORKTREE was written to allow sparse-checkouts, in particular, as
the name of the option implies, to allow the file to NOT be in the
worktree but consider it to be unchanged rather than deleted.
The suggests a simple solution: present-despite-SKIP_WORKTREE files
should not exist, for those using sparse-checkouts.
Enforce this at index loading time by checking if core.sparseCheckout is
true; if so, check files in the index with the SKIP_WORKTREE bit set to
verify that they are absent from the working tree. If they are present,
unset the bit (in memory, though any commands that write to the index
will record the update).
Users can, of course, can get the SKIP_WORKTREE bit back such as by
running `git sparse-checkout reapply` (if they have ensured the file is
unmodified and doesn't match the specified sparsity patterns).
=== Pros/Cons of suggested solution ===
Pros:
* Solves the user visible problems reported above, which I've been
complaining about for nearly a year but couldn't find a solution to.
* Helps prevent slow performance degradation with a sparse-index.
* Much easier behavior in sparse-checkouts for users to reason about
* Very simple, ~30 lines of code.
* Significantly simplifies some ugly testcases, and obviates the need
to test an entire class of potential issues.
* Reduces code complexity, reasoning, and maintenance. Avoids
disagreements about weird corner cases[12].
* It has been reported that some users might be (ab)using
SKIP_WORKTREE as a let-me-modify-but-keep-the-file-in-the-worktree
mechanism[13, and a few other similar references]. These users know
of multiple caveats and shortcomings in doing so; perhaps not
surprising given the "SKIP_WORKTREE expecations" section above.
However, these users use `git update-index --skip-worktree`, and not
`git sparse-checkout` or core.sparseCheckout=true. As such, these
users would be unaffected by this change and can continue abusing
the system as before.
[12] https://lore.kernel.org/git/CABPp-BH9tju7WVm=QZDOvaMDdZbpNXrVWQdN-jmfN8wC6YVhmw@mail.gmail.com/
[13] https://stackoverflow.com/questions/13630849/git-difference-between-assume-unchanged-and-skip-worktree
Cons:
* When core.sparseCheckout is enabled, this adds a performance cost to
reading the index. I'll defer discussion of this cost to a subsequent
patch, since I have some optimizations to add.
=== Notes on testcase modifications ===
The good:
* t1011: Compare to two cases above it ('read-tree will not throw away
dirty changes, non-sparse'); since the file is present, it should
match the non-sparse case now
* t1092: sparse-index & sparse-checkout now match full-worktree
behavior in more cases! Yaay for consistency!
* t6428, t7012: look at how much simpler the tests become! Merge and
stash can just fail early telling the user there's a file in the
way, instead of not noticing until it's about to write a file and
then have to implement sudden crash avoidance. Hurray for sanity!
* t7817: sparse behavior better matches full tree behavior. Hurray
for sanity!
The confusing:
* t3705: These changes were ONLY needed on Windows, but they don't
hurt other platforms. Let's discuss each individually:
* core.sparseCheckout should be false by default. Nothing in this
testcase toggles that until many, many tests later. However,
early tests (#5 in particular) were testing `update-index
--skip-worktree` behavior in a non-sparse-checkout, but the
Windows tests in CI were behaving as if core.sparseCheckout=true
had been specified somewhere. I do not have access to a Windows
machine. But I just manually did what should have been a no-op
and turned the config off. And it fixed the test.
* I have no idea why the leftover .gitattributes file from this
test was causing failures for test #18 on Windows, but only with
these changes of mine. Test #18 was checking for empty stderr,
and specifically wanted to know that some error completely
unrelated to file endings did not appear. The leftover
.gitattributes file thus caused some spurious stderr unrelated to
the thing being checked. Since other tests did not intend to
test normalization, just proactively remove the .gitattributes
file. I'm certain this is cleaner and better, I'm just unsure
why/how this didn't trigger problems before.
Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
sceptical-coder
added a commit
to sceptical-coder/git
that referenced
this issue
Oct 30, 2022
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>
sceptical-coder
added a commit
to sceptical-coder/git
that referenced
this issue
Oct 31, 2022
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>
sceptical-coder
added a commit
to sceptical-coder/git
that referenced
this issue
Nov 3, 2022
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>
sceptical-coder
added a commit
to sceptical-coder/git
that referenced
this issue
Nov 3, 2022
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>
sceptical-coder
added a commit
to sceptical-coder/git
that referenced
this issue
Nov 3, 2022
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>
sceptical-coder
added a commit
to sceptical-coder/git
that referenced
this issue
Nov 3, 2022
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>
sceptical-coder
added a commit
to sceptical-coder/git
that referenced
this issue
Nov 3, 2022
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>
sceptical-coder
added a commit
to sceptical-coder/git
that referenced
this issue
Nov 3, 2022
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>
sceptical-coder
added a commit
to sceptical-coder/git
that referenced
this issue
Nov 3, 2022
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>
dscho
pushed a commit
to sceptical-coder/git
that referenced
this issue
Nov 4, 2022
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>
derrickstolee
pushed a commit
that referenced
this issue
Jan 17, 2023
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>
derrickstolee
pushed a commit
that referenced
this issue
Jan 17, 2023
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>
derrickstolee
pushed a commit
that referenced
this issue
Jan 17, 2023
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>
derrickstolee
pushed a commit
that referenced
this issue
Jan 17, 2023
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>
derrickstolee
pushed a commit
that referenced
this issue
Jan 17, 2023
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>
git-for-windows-ci
pushed a commit
that referenced
this issue
Jun 17, 2024
Memory sanitizer (msan) is detecting a use of an uninitialized variable
(`size`) in `read_attr_from_index`:
==2268==WARNING: MemorySanitizer: use-of-uninitialized-value
#0 0x5651f3416504 in read_attr_from_index git/attr.c:868:11
#1 0x5651f3415530 in read_attr git/attr.c
#2 0x5651f3413d74 in bootstrap_attr_stack git/attr.c:968:6
#3 0x5651f3413d74 in prepare_attr_stack git/attr.c:1004:2
#4 0x5651f3413d74 in collect_some_attrs git/attr.c:1199:2
#5 0x5651f3413144 in git_check_attr git/attr.c:1345:2
#6 0x5651f34728da in convert_attrs git/convert.c:1320:2
#7 0x5651f3473425 in would_convert_to_git_filter_fd git/convert.c:1373:2
#8 0x5651f357a35e in index_fd git/object-file.c:2630:34
#9 0x5651f357aa15 in index_path git/object-file.c:2657:7
#10 0x5651f35db9d9 in add_to_index git/read-cache.c:766:7
#11 0x5651f35dc170 in add_file_to_index git/read-cache.c:799:9
#12 0x5651f321f9b2 in add_files git/builtin/add.c:346:7
#13 0x5651f321f9b2 in cmd_add git/builtin/add.c:565:18
#14 0x5651f321d327 in run_builtin git/git.c:474:11
#15 0x5651f321bc9e in handle_builtin git/git.c:729:3
#16 0x5651f321a792 in run_argv git/git.c:793:4
#17 0x5651f321a792 in cmd_main git/git.c:928:19
#18 0x5651f33dde1f in main git/common-main.c:62:11
The issue exists because `size` is an output parameter from
`read_blob_data_from_index`, but it's only modified if
`read_blob_data_from_index` returns non-NULL. The read of `size` when
calling `read_attr_from_buf` unconditionally may read from an
uninitialized value. `read_attr_from_buf` checks that `buf` is non-NULL
before reading from `size`, but by then it's already too late: the
uninitialized read will have happened already. Furthermore, there's no
guarantee that the compiler won't reorder things so that it checks
`size` before checking `!buf`.
Make the call to `read_attr_from_buf` conditional on `buf` being
non-NULL, ensuring that `size` is not read if it's never set.
Signed-off-by: Kyle Lippincott <spectral@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
After opening up up PortableGit (using a git-bash.bat file to keep the settings portable per http://stackoverflow.com/a/3455231/647393 - which by the way should be standard for PortableGit IMO), I found that my $PATH looks like this:
//bin:.:/usr/local/bin:/mingw/bin:/bin:/c/Misc/ConEmu/ConEmu:/c/Windows/system32:/c/Windows:/c/Windows/System32/Wbem:/c/Windows/System32/WindowsPowerShell/v1.0/:/c/Program Files (x86)/ATI Technologies/ATI.ACE/Core-Static:/c/Program Files (x86)/Heroku/bin:/c/Program Files (x86)/git/cmd:/c/Program Files (x86)/Java/jre7/bin: C:/Misc/PortablePython-2.7.5.1/App/Scripts
The last path looks different than the rest and therefore doesn't work, and I don't know why. The last two are both part of the user PATH environment variable in Windows 7 but the Java one doesn't get converted into C:/ rather than /c/.
I was able to fix this by editing .bash_profile to append :/c/misc/portablepython-2.7.5.1/app/scripts which works. Incidentally .bash_profile works but .bashrc does not work, which seems opposite of what I'd expect per .bash_profile only applying to "login shells" per http://stackoverflow.com/questions/415403/whats-the-difference-between-bashrc-bash-profile-and-environment.
The text was updated successfully, but these errors were encountered: