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@xxxxxxxxxx>
Cc: Alain Takoudjou <alain.takoudjou@xxxxxxxxxx>
Cc: Jérome Perrin <jerome@xxxxxxxxxx>
Signed-off-by: Kirill Smelkov <kirr@xxxxxxxxxx>
---
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
