Forking mgwport #17
Assignees
Comments
|
I'd be in favor of forking into |
|
Okay, so I'll move it to git-for-windows as @sschuberth was also in favour of that. |
|
Done, see https://github.com/git-for-windows/mgwport. |
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 12, 2021
add_pending_object() populates rev.pending, we need to take care of
clearing it once we're done.
This code is run close to the end of a checkout, therefore this leak
seems like it would have very little impact. See also LSAN output
from t0020 below:
Direct leak of 2048 byte(s) in 1 object(s) allocated from:
#0 0x49ab79 in realloc ../projects/compiler-rt/lib/asan/asan_malloc_linux.cpp:164:3
#1 0x9acc46 in xrealloc wrapper.c:126:8
#2 0x83e3a3 in add_object_array_with_path object.c:337:3
#3 0x8f672a in add_pending_object_with_path revision.c:329:2
#4 0x8eaeab in add_pending_object_with_mode revision.c:336:2
#5 0x8eae9d in add_pending_object revision.c:342:2
#6 0x5154a0 in show_local_changes builtin/checkout.c:602:2
#7 0x513b00 in merge_working_tree builtin/checkout.c:979:3
#8 0x512cb3 in switch_branches builtin/checkout.c:1242:9
#9 0x50f8de in checkout_branch builtin/checkout.c:1646:9
#10 0x50ba12 in checkout_main builtin/checkout.c:2003:9
#11 0x5086c0 in cmd_checkout builtin/checkout.c:2055:8
#12 0x4cd91d in run_builtin git.c:467:11
#13 0x4cb5f3 in handle_builtin git.c:719:3
#14 0x4ccf47 in run_argv git.c:808:4
#15 0x4caf49 in cmd_main git.c:939:19
#16 0x69e43e in main common-main.c:52:11
#17 0x7f5dd1d50349 in __libc_start_main (/lib64/libc.so.6+0x24349)
SUMMARY: AddressSanitizer: 2048 byte(s) leaked in 1 allocation(s).
Signed-off-by: Andrzej Hunt <ajrhunt@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
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>
dscho
pushed a commit
that referenced
this issue
Apr 28, 2021
add_pending_object() populates rev.pending, we need to take care of
clearing it once we're done.
This code is run close to the end of a checkout, therefore this leak
seems like it would have very little impact. See also LSAN output
from t0020 below:
Direct leak of 2048 byte(s) in 1 object(s) allocated from:
#0 0x49ab79 in realloc ../projects/compiler-rt/lib/asan/asan_malloc_linux.cpp:164:3
#1 0x9acc46 in xrealloc wrapper.c:126:8
#2 0x83e3a3 in add_object_array_with_path object.c:337:3
#3 0x8f672a in add_pending_object_with_path revision.c:329:2
#4 0x8eaeab in add_pending_object_with_mode revision.c:336:2
#5 0x8eae9d in add_pending_object revision.c:342:2
#6 0x5154a0 in show_local_changes builtin/checkout.c:602:2
#7 0x513b00 in merge_working_tree builtin/checkout.c:979:3
#8 0x512cb3 in switch_branches builtin/checkout.c:1242:9
#9 0x50f8de in checkout_branch builtin/checkout.c:1646:9
#10 0x50ba12 in checkout_main builtin/checkout.c:2003:9
#11 0x5086c0 in cmd_checkout builtin/checkout.c:2055:8
#12 0x4cd91d in run_builtin git.c:467:11
#13 0x4cb5f3 in handle_builtin git.c:719:3
#14 0x4ccf47 in run_argv git.c:808:4
#15 0x4caf49 in cmd_main git.c:939:19
#16 0x69e43e in main common-main.c:52:11
#17 0x7f5dd1d50349 in __libc_start_main (/lib64/libc.so.6+0x24349)
SUMMARY: AddressSanitizer: 2048 byte(s) leaked in 1 allocation(s).
Signed-off-by: Andrzej Hunt <ajrhunt@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
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>
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>
dscho
pushed a commit
that referenced
this issue
Aug 25, 2023
When t5583-push-branches.sh was originally introduced via 425b4d7 (push: introduce '--branches' option, 2023-05-06), it was not leak-free. In fact, the test did not even run correctly until 022fbb6 (t5583: fix shebang line, 2023-05-12), but after applying that patch, we see a failure at t5583.8: ==2529087==ERROR: LeakSanitizer: detected memory leaks Direct leak of 384 byte(s) in 1 object(s) allocated from: #0 0x7fb536330986 in __interceptor_realloc ../../../../src/libsanitizer/lsan/lsan_interceptors.cpp:98 #1 0x55e07606cbf9 in xrealloc wrapper.c:140 #2 0x55e075fb6cb3 in prio_queue_put prio-queue.c:42 #3 0x55e075ec81cb in get_reachable_subset commit-reach.c:917 #4 0x55e075fe9cce in add_missing_tags remote.c:1518 #5 0x55e075fea1e4 in match_push_refs remote.c:1665 #6 0x55e076050a8e in transport_push transport.c:1378 #7 0x55e075e2eb74 in push_with_options builtin/push.c:401 #8 0x55e075e2edb0 in do_push builtin/push.c:458 #9 0x55e075e2ff7a in cmd_push builtin/push.c:702 #10 0x55e075d8aaf0 in run_builtin git.c:452 #11 0x55e075d8af08 in handle_builtin git.c:706 #12 0x55e075d8b12c in run_argv git.c:770 #13 0x55e075d8b6a0 in cmd_main git.c:905 #14 0x55e075e81f07 in main common-main.c:60 #15 0x7fb5360ab6c9 in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58 #16 0x7fb5360ab784 in __libc_start_main_impl ../csu/libc-start.c:360 #17 0x55e075d88f40 in _start (git+0x1ff40) (BuildId: 38ad998b85a535e786129979443630d025ec2453) SUMMARY: LeakSanitizer: 384 byte(s) leaked in 1 allocation(s). This leak was addressed independently via 68b5117 (commit-reach: fix memory leak in get_reachable_subset(), 2023-06-03), which makes t5583 leak-free. But t5583 was not in the tree when 68b5117 was written, and the two only met after the latter was merged back in via 693bde4 (Merge branch 'mh/commit-reach-get-reachable-plug-leak', 2023-06-20). At that point, t5583 was leak-free. Let's mark it as such accordingly. Signed-off-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
git-for-windows-ci
pushed a commit
that referenced
this issue
Aug 29, 2023
When t5583-push-branches.sh was originally introduced via 425b4d7 (push: introduce '--branches' option, 2023-05-06), it was not leak-free. In fact, the test did not even run correctly until 022fbb6 (t5583: fix shebang line, 2023-05-12), but after applying that patch, we see a failure at t5583.8: ==2529087==ERROR: LeakSanitizer: detected memory leaks Direct leak of 384 byte(s) in 1 object(s) allocated from: #0 0x7fb536330986 in __interceptor_realloc ../../../../src/libsanitizer/lsan/lsan_interceptors.cpp:98 #1 0x55e07606cbf9 in xrealloc wrapper.c:140 #2 0x55e075fb6cb3 in prio_queue_put prio-queue.c:42 #3 0x55e075ec81cb in get_reachable_subset commit-reach.c:917 #4 0x55e075fe9cce in add_missing_tags remote.c:1518 #5 0x55e075fea1e4 in match_push_refs remote.c:1665 #6 0x55e076050a8e in transport_push transport.c:1378 #7 0x55e075e2eb74 in push_with_options builtin/push.c:401 #8 0x55e075e2edb0 in do_push builtin/push.c:458 #9 0x55e075e2ff7a in cmd_push builtin/push.c:702 #10 0x55e075d8aaf0 in run_builtin git.c:452 #11 0x55e075d8af08 in handle_builtin git.c:706 #12 0x55e075d8b12c in run_argv git.c:770 #13 0x55e075d8b6a0 in cmd_main git.c:905 #14 0x55e075e81f07 in main common-main.c:60 #15 0x7fb5360ab6c9 in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58 #16 0x7fb5360ab784 in __libc_start_main_impl ../csu/libc-start.c:360 #17 0x55e075d88f40 in _start (git+0x1ff40) (BuildId: 38ad998b85a535e786129979443630d025ec2453) SUMMARY: LeakSanitizer: 384 byte(s) leaked in 1 allocation(s). This leak was addressed independently via 68b5117 (commit-reach: fix memory leak in get_reachable_subset(), 2023-06-03), which makes t5583 leak-free. But t5583 was not in the tree when 68b5117 was written, and the two only met after the latter was merged back in via 693bde4 (Merge branch 'mh/commit-reach-get-reachable-plug-leak', 2023-06-20). At that point, t5583 was leak-free. Let's mark it as such accordingly. Signed-off-by: Taylor Blau <me@ttaylorr.com> Acked-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
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>
git-for-windows-ci
pushed a commit
that referenced
this issue
Aug 19, 2024
It was recently reported that concurrent reads and writes may cause the
reftable backend to segfault. The root cause of this is that we do not
properly keep track of reftable readers across reloads.
Suppose that you have a reftable iterator and then decide to reload the
stack while iterating through the iterator. When the stack has been
rewritten since we have created the iterator, then we would end up
discarding a subset of readers that may still be in use by the iterator.
The consequence is that we now try to reference deallocated memory,
which of course segfaults.
One way to trigger this is in t5616, where some background maintenance
jobs have been leaking from one test into another. This leads to stack
traces like the following one:
+ git -c protocol.version=0 -C pc1 fetch --filter=blob:limit=29999 --refetch origin
AddressSanitizer:DEADLYSIGNAL
=================================================================
==657994==ERROR: AddressSanitizer: SEGV on unknown address 0x7fa0f0ec6089 (pc 0x55f23e52ddf9 bp
0x7ffe7bfa1700 sp 0x7ffe7bfa1700 T0)
==657994==The signal is caused by a READ memory access.
#0 0x55f23e52ddf9 in get_var_int reftable/record.c:29
#1 0x55f23e53295e in reftable_decode_keylen reftable/record.c:170
#2 0x55f23e532cc0 in reftable_decode_key reftable/record.c:194
#3 0x55f23e54e72e in block_iter_next reftable/block.c:398
#4 0x55f23e5573dc in table_iter_next_in_block reftable/reader.c:240
#5 0x55f23e5573dc in table_iter_next reftable/reader.c:355
#6 0x55f23e5573dc in table_iter_next reftable/reader.c:339
#7 0x55f23e551283 in merged_iter_advance_subiter reftable/merged.c:69
#8 0x55f23e55169e in merged_iter_next_entry reftable/merged.c:123
#9 0x55f23e55169e in merged_iter_next_void reftable/merged.c:172
#10 0x55f23e537625 in reftable_iterator_next_ref reftable/generic.c:175
#11 0x55f23e2cf9c6 in reftable_ref_iterator_advance refs/reftable-backend.c:464
#12 0x55f23e2d996e in ref_iterator_advance refs/iterator.c:13
#13 0x55f23e2d996e in do_for_each_ref_iterator refs/iterator.c:452
#14 0x55f23dca6767 in get_ref_map builtin/fetch.c:623
#15 0x55f23dca6767 in do_fetch builtin/fetch.c:1659
#16 0x55f23dca6767 in fetch_one builtin/fetch.c:2133
#17 0x55f23dca6767 in cmd_fetch builtin/fetch.c:2432
#18 0x55f23dba7764 in run_builtin git.c:484
#19 0x55f23dba7764 in handle_builtin git.c:741
#20 0x55f23dbab61e in run_argv git.c:805
#21 0x55f23dbab61e in cmd_main git.c:1000
#22 0x55f23dba4781 in main common-main.c:64
#23 0x7fa0f063fc89 in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58
#24 0x7fa0f063fd44 in __libc_start_main_impl ../csu/libc-start.c:360
#25 0x55f23dba6ad0 in _start (git+0xadfad0) (BuildId: 803b2b7f59beb03d7849fb8294a8e2145dd4aa27)
While it is somewhat awkward that the maintenance processes survive
tests in the first place, it is totally expected that reftables should
work alright with concurrent writers. Seemingly they don't.
The only underlying resource that we need to care about in this context
is the reftable reader, which is responsible for reading a single table
from disk. These readers get discarded immediately (unless reused) when
calling `reftable_stack_reload()`, which is wrong. We can only close
them once we know that there are no iterators using them anymore.
Prepare for a fix by converting the reftable readers to be refcounted.
Reported-by: Jeff King <peff@peff.net>
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
git-for-windows-ci
pushed a commit
that referenced
this issue
Aug 22, 2024
It was recently reported that concurrent reads and writes may cause the
reftable backend to segfault. The root cause of this is that we do not
properly keep track of reftable readers across reloads.
Suppose that you have a reftable iterator and then decide to reload the
stack while iterating through the iterator. When the stack has been
rewritten since we have created the iterator, then we would end up
discarding a subset of readers that may still be in use by the iterator.
The consequence is that we now try to reference deallocated memory,
which of course segfaults.
One way to trigger this is in t5616, where some background maintenance
jobs have been leaking from one test into another. This leads to stack
traces like the following one:
+ git -c protocol.version=0 -C pc1 fetch --filter=blob:limit=29999 --refetch origin
AddressSanitizer:DEADLYSIGNAL
=================================================================
==657994==ERROR: AddressSanitizer: SEGV on unknown address 0x7fa0f0ec6089 (pc 0x55f23e52ddf9 bp
0x7ffe7bfa1700 sp 0x7ffe7bfa1700 T0)
==657994==The signal is caused by a READ memory access.
#0 0x55f23e52ddf9 in get_var_int reftable/record.c:29
#1 0x55f23e53295e in reftable_decode_keylen reftable/record.c:170
#2 0x55f23e532cc0 in reftable_decode_key reftable/record.c:194
#3 0x55f23e54e72e in block_iter_next reftable/block.c:398
#4 0x55f23e5573dc in table_iter_next_in_block reftable/reader.c:240
#5 0x55f23e5573dc in table_iter_next reftable/reader.c:355
#6 0x55f23e5573dc in table_iter_next reftable/reader.c:339
#7 0x55f23e551283 in merged_iter_advance_subiter reftable/merged.c:69
#8 0x55f23e55169e in merged_iter_next_entry reftable/merged.c:123
#9 0x55f23e55169e in merged_iter_next_void reftable/merged.c:172
#10 0x55f23e537625 in reftable_iterator_next_ref reftable/generic.c:175
#11 0x55f23e2cf9c6 in reftable_ref_iterator_advance refs/reftable-backend.c:464
#12 0x55f23e2d996e in ref_iterator_advance refs/iterator.c:13
#13 0x55f23e2d996e in do_for_each_ref_iterator refs/iterator.c:452
#14 0x55f23dca6767 in get_ref_map builtin/fetch.c:623
#15 0x55f23dca6767 in do_fetch builtin/fetch.c:1659
#16 0x55f23dca6767 in fetch_one builtin/fetch.c:2133
#17 0x55f23dca6767 in cmd_fetch builtin/fetch.c:2432
#18 0x55f23dba7764 in run_builtin git.c:484
#19 0x55f23dba7764 in handle_builtin git.c:741
#20 0x55f23dbab61e in run_argv git.c:805
#21 0x55f23dbab61e in cmd_main git.c:1000
#22 0x55f23dba4781 in main common-main.c:64
#23 0x7fa0f063fc89 in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58
#24 0x7fa0f063fd44 in __libc_start_main_impl ../csu/libc-start.c:360
#25 0x55f23dba6ad0 in _start (git+0xadfad0) (BuildId: 803b2b7f59beb03d7849fb8294a8e2145dd4aa27)
While it is somewhat awkward that the maintenance processes survive
tests in the first place, it is totally expected that reftables should
work alright with concurrent writers. Seemingly they don't.
The only underlying resource that we need to care about in this context
is the reftable reader, which is responsible for reading a single table
from disk. These readers get discarded immediately (unless reused) when
calling `reftable_stack_reload()`, which is wrong. We can only close
them once we know that there are no iterators using them anymore.
Prepare for a fix by converting the reftable readers to be refcounted.
Reported-by: Jeff King <peff@peff.net>
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
git-for-windows-ci
pushed a commit
that referenced
this issue
Aug 23, 2024
It was recently reported that concurrent reads and writes may cause the
reftable backend to segfault. The root cause of this is that we do not
properly keep track of reftable readers across reloads.
Suppose that you have a reftable iterator and then decide to reload the
stack while iterating through the iterator. When the stack has been
rewritten since we have created the iterator, then we would end up
discarding a subset of readers that may still be in use by the iterator.
The consequence is that we now try to reference deallocated memory,
which of course segfaults.
One way to trigger this is in t5616, where some background maintenance
jobs have been leaking from one test into another. This leads to stack
traces like the following one:
+ git -c protocol.version=0 -C pc1 fetch --filter=blob:limit=29999 --refetch origin
AddressSanitizer:DEADLYSIGNAL
=================================================================
==657994==ERROR: AddressSanitizer: SEGV on unknown address 0x7fa0f0ec6089 (pc 0x55f23e52ddf9 bp
0x7ffe7bfa1700 sp 0x7ffe7bfa1700 T0)
==657994==The signal is caused by a READ memory access.
#0 0x55f23e52ddf9 in get_var_int reftable/record.c:29
#1 0x55f23e53295e in reftable_decode_keylen reftable/record.c:170
#2 0x55f23e532cc0 in reftable_decode_key reftable/record.c:194
#3 0x55f23e54e72e in block_iter_next reftable/block.c:398
#4 0x55f23e5573dc in table_iter_next_in_block reftable/reader.c:240
#5 0x55f23e5573dc in table_iter_next reftable/reader.c:355
#6 0x55f23e5573dc in table_iter_next reftable/reader.c:339
#7 0x55f23e551283 in merged_iter_advance_subiter reftable/merged.c:69
#8 0x55f23e55169e in merged_iter_next_entry reftable/merged.c:123
#9 0x55f23e55169e in merged_iter_next_void reftable/merged.c:172
#10 0x55f23e537625 in reftable_iterator_next_ref reftable/generic.c:175
#11 0x55f23e2cf9c6 in reftable_ref_iterator_advance refs/reftable-backend.c:464
#12 0x55f23e2d996e in ref_iterator_advance refs/iterator.c:13
#13 0x55f23e2d996e in do_for_each_ref_iterator refs/iterator.c:452
#14 0x55f23dca6767 in get_ref_map builtin/fetch.c:623
#15 0x55f23dca6767 in do_fetch builtin/fetch.c:1659
#16 0x55f23dca6767 in fetch_one builtin/fetch.c:2133
#17 0x55f23dca6767 in cmd_fetch builtin/fetch.c:2432
#18 0x55f23dba7764 in run_builtin git.c:484
#19 0x55f23dba7764 in handle_builtin git.c:741
#20 0x55f23dbab61e in run_argv git.c:805
#21 0x55f23dbab61e in cmd_main git.c:1000
#22 0x55f23dba4781 in main common-main.c:64
#23 0x7fa0f063fc89 in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58
#24 0x7fa0f063fd44 in __libc_start_main_impl ../csu/libc-start.c:360
#25 0x55f23dba6ad0 in _start (git+0xadfad0) (BuildId: 803b2b7f59beb03d7849fb8294a8e2145dd4aa27)
While it is somewhat awkward that the maintenance processes survive
tests in the first place, it is totally expected that reftables should
work alright with concurrent writers. Seemingly they don't.
The only underlying resource that we need to care about in this context
is the reftable reader, which is responsible for reading a single table
from disk. These readers get discarded immediately (unless reused) when
calling `reftable_stack_reload()`, which is wrong. We can only close
them once we know that there are no iterators using them anymore.
Prepare for a fix by converting the reftable readers to be refcounted.
Reported-by: Jeff King <peff@peff.net>
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Upstream mgwport is dead, last update from Friday, November 11 2011, I've tried to email the author no answer.
For the deterministic build changes I need to therefore create a new package mingw32-mgwport.
Do we want to have mgwport in git-for-windows org or should I keep it in t-b? For me both is okay.
The text was updated successfully, but these errors were encountered: