@@ -1056,6 +1056,20 @@ test_expect_success 'fetch with --filter=blob:limit=0' '
fetch_filter_blob_limit_zero server server
'
+test_expect_failure 'fetch with fsckObjects but without --lock-pack does not segfault' '
+ rm -rf server client &&
+ git init server &&
+ test_commit -C server 1 &&
+
+ git init client &&
+ # unpackLimit=1 forces to keep received pack as pack instead of
+ # unpacking it to loose objects. The code is currently segfaulting when
+ # trying to index that kept pack.
+ git -c fetch.fsckObjects=true -c fetch.unpackLimit=1 \
+ -C client fetch-pack ../server \
+ $(git -C server rev-parse refs/heads/main)
+'
+
. "$TEST_DIRECTORY"/lib-httpd.sh
start_httpd
When running git-backup[1] over lab.nexedi.com archive we noticed that Git started to crash on fetch-pack operation[2] after Git upgrade. We tracked this down to be a regression introduced by Git 2.31, more specifically by commit 5476e1efded5 (fetch-pack: print and use dangling .gitmodules), which started to index and lock packfiles on do_keep=y && fsck_objects=y even if pack_lockfiles=NULL, which leads to NULL-dereference when trying to append an entry to that pack_lockfiles stringlist. Please find attached a test that demonstrates this problem. When run with test_expect_failure changed to test_expect_success the test crashes with ./test-lib.sh: line 1063: 599675 Segmentation fault (core dumped) git -c fetch.fsckObjects=true -c fetch.unpackLimit=1 -C client fetch-pack ../server $(git -C server rev-parse refs/heads/main) and the backtrace is Program terminated with signal SIGSEGV, Segmentation fault. #0 0x0000558032c7ef3b in string_list_append_nodup (list=0x0, string=0x558033b695f0 ".git/objects/pack/pack-c1b2455a361bb50a0db087e7202db73d62938fa1.keep") at string-list.c:218 218 ALLOC_GROW(list->items, list->nr + 1, list->alloc); (gdb) bt #0 0x0000558032c7ef3b in string_list_append_nodup (list=0x0, string=0x558033b695f0 ".git/objects/pack/pack-c1b2455a361bb50a0db087e7202db73d62938fa1.keep") at string-list.c:218 #1 0x0000558032b5b83f in get_pack (args=0x7ffe680f3fa0, xd=0x7ffe680f4058, pack_lockfiles=0x0, index_pack_args=0x0, sought=0x558033b65e90, nr_sought=1, gitmodules_oids=0x558032e17b88 <fsck_options+72>) at fetch-pack.c:1042 #2 0x0000558032b5e0b3 in do_fetch_pack_v2 (args=0x7ffe680f3fa0, fd=0x7ffe680f4058, orig_ref=0x558033b67b90, sought=0x558033b65e90, nr_sought=1, shallows=0x7ffe680f3df0, si=0x7ffe680f3e10, pack_lockfiles=0x0) at fetch-pack.c:1781 #3 0x0000558032b5ef3c in fetch_pack (args=0x7ffe680f3fa0, fd=0x7ffe680f4058, ref=0x558033b67b90, sought=0x558033b65e90, nr_sought=1, shallow=0x7ffe680f3f80, pack_lockfiles=0x0, version=protocol_v2) at fetch-pack.c:2073 #4 0x0000558032a08851 in cmd_fetch_pack (argc=3, argv=0x7ffe680f4500, prefix=0x0) at builtin/fetch-pack.c:242 #5 0x00005580329b2be3 in run_builtin (p=0x558032e0bb30 <commands+1008>, argc=3, argv=0x7ffe680f4500) at git.c:474 #6 0x00005580329b2ffe in handle_builtin (argc=3, argv=0x7ffe680f4500) at git.c:729 #7 0x00005580329b3222 in run_argv (argcp=0x7ffe680f433c, argv=0x7ffe680f4330) at git.c:793 #8 0x00005580329b3796 in cmd_main (argc=3, argv=0x7ffe680f4500) at git.c:928 #9 0x0000558032ab9002 in main (argc=10, argv=0x7ffe680f44c8) at common-main.c:62 A simple debug patch below --- a/fetch-pack.c +++ b/fetch-pack.c @@ -1032,7 +1032,7 @@ static int get_pack(struct fetch_pack_args *args, cmd.git_cmd = 1; if (start_command(&cmd)) die(_("fetch-pack: unable to fork off %s"), cmd_name); - if (do_keep && (pack_lockfiles || fsck_objects)) { + if (do_keep && (pack_lockfiles /*|| fsck_objects*/)) { int is_well_formed; char *pack_lockfile = index_pack_lockfile(cmd.out, &is_well_formed); makes the crash go away, but I did not investigated what should be the proper logic. Thanks beforehand for fixing this NULL-pointer dereference. Kirill [1] https://lab.nexedi.com/kirr/git-backup [2] https://lab.nexedi.com/kirr/git-backup/-/blob/3230197c/git-backup.go#L717-739 Cc: Jonathan Tan <jonathantanmy@google.com> Cc: Alain Takoudjou <alain.takoudjou@nexedi.com> Cc: Jérome Perrin <jerome@nexedi.com> Signed-off-by: Kirill Smelkov <kirr@nexedi.com> --- t/t5500-fetch-pack.sh | 14 ++++++++++++++ 1 file changed, 14 insertions(+)