[PATCH] delta-islands: fix segfault when freeing island marks

[Patrick Steinhardt <ps@pks.im>]

[-- Attachment #1: Type: text/plain, Size: 2530 bytes --]

In 647982bb71 (delta-islands: free island_marks and bitmaps, 2023-02-03)
we have introduced logic to free `island_marks` in order to reduce heap
memory usage in git-pack-objects(1). This commit is causing segfaults in
the case where this Git command does not load delta islands at all, e.g.
when reading object IDs from standard input. One such crash can be hit
when using repacking multi-pack-indices with delta islands enabled.

The root cause of this bug is that we unconditionally dereference the
`island_marks` variable even in the case where it is a `NULL` pointer,
which is fixed by making it conditional. Note that we still leave the
logic in place to set the pointer to `-1` to detect use-after-free bugs
even when there are no loaded island marks at all.
---

An easy way to reproduce the segfault is:

    $ git pack-objects .git/objects/pack/pack --delta-islands </dev/null

I didn't add a test for git-pack-objects(1) directly though, mostly
because I didn't find any location to put it. I'm happy to do so though
in case we want that.

 delta-islands.c             | 12 +++++++-----
 t/t5319-multi-pack-index.sh | 16 ++++++++++++++++
 2 files changed, 23 insertions(+), 5 deletions(-)

diff --git a/delta-islands.c b/delta-islands.c
index 8b234cb85b..afdec0a878 100644
--- a/delta-islands.c
+++ b/delta-islands.c
@@ -517,11 +517,13 @@ void free_island_marks(void)
 {
 	struct island_bitmap *bitmap;
 
-	kh_foreach_value(island_marks, bitmap, {
-		if (!--bitmap->refcount)
-			free(bitmap);
-	});
-	kh_destroy_oid_map(island_marks);
+	if (island_marks) {
+		kh_foreach_value(island_marks, bitmap, {
+			if (!--bitmap->refcount)
+				free(bitmap);
+		});
+		kh_destroy_oid_map(island_marks);
+	}
 
 	/* detect use-after-free with a an address which is never valid: */
 	island_marks = (void *)-1;
diff --git a/t/t5319-multi-pack-index.sh b/t/t5319-multi-pack-index.sh
index b5f9b10922..499d5d4c78 100755
--- a/t/t5319-multi-pack-index.sh
+++ b/t/t5319-multi-pack-index.sh
@@ -1015,4 +1015,20 @@ test_expect_success 'complains when run outside of a repository' '
 	grep "not a git repository" err
 '
 
+test_expect_success 'repack with delta islands' '
+	git init repo &&
+	test_when_finished "rm -fr repo" &&
+	(
+		cd repo &&
+
+		test_commit first &&
+		git repack &&
+		test_commit second &&
+		git repack &&
+
+		git multi-pack-index write &&
+		git -c repack.useDeltaIslands=true multi-pack-index repack
+	)
+'
+
 test_done
-- 
2.39.2


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]