[PATCH] fetch-pack: test: demonstrate segmentation fault when run with fsckObjects but without --lock-pack

[Kirill Smelkov <kirr@nexedi.com>]

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(+)

diff --git a/t/t5500-fetch-pack.sh b/t/t5500-fetch-pack.sh
index 1bc15a3f08..e3dbe79613 100755
--- a/t/t5500-fetch-pack.sh
+++ b/t/t5500-fetch-pack.sh
@@ -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

--
2.39.2