@matheusbsbs / matheus-bagliano / commits / 6eb095d787

delta-islands: fix segfault when freeing island marks

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. Signed-off-by: Patrick Steinhardt <ps@pks.im> Acked-by: Eric Wong <e@80x24.org> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Patrick Steinhardt committed Feb 16, 2023 at 11:29 UTC 6eb095d78790d0d6cbad2186685dad4e7ba2e3de
2 files changed +23 -5
delta-islands.c
+7 -5
@@ -517,11 +517,13 @@ void free_island_marks(void)
517 {
518 struct island_bitmap *bitmap;
519
520 - kh_foreach_value(island_marks, bitmap, {
521 - if (!--bitmap->refcount)
522 - free(bitmap);
523 - });
524 - kh_destroy_oid_map(island_marks);
520 + if (island_marks) {
521 + kh_foreach_value(island_marks, bitmap, {
522 + if (!--bitmap->refcount)
523 + free(bitmap);
524 + });
525 + kh_destroy_oid_map(island_marks);
526 + }
527
528 /* detect use-after-free with a an address which is never valid: */
529 island_marks = (void *)-1;
t/t5319-multi-pack-index.sh
+16
@@ -1015,4 +1015,20 @@ test_expect_success 'complains when run outside of a repository' '
1015 grep "not a git repository" err
1016 '
1017
1018 +test_expect_success 'repack with delta islands' '
1019 + git init repo &&
1020 + test_when_finished "rm -fr repo" &&
1021 + (
1022 + cd repo &&
1023 +
1024 + test_commit first &&
1025 + git repack &&
1026 + test_commit second &&
1027 + git repack &&
1028 +
1029 + git multi-pack-index write &&
1030 + git -c repack.useDeltaIslands=true multi-pack-index repack
1031 + )
1032 +'
1033 +
1034 test_done