Skip to content

Commit e6aff37

Browse files
ttaylorrgitster
authored andcommitted
pack-bitmap.c: avoid uninitialized pack_int_id during reuse
When performing multi-pack reuse, reuse_partial_packfile_from_bitmap() is responsible for generating an array of bitmapped_pack structs from which to perform reuse. In the multi-pack case, we loop over the MIDXs packs and copy the result of calling `nth_bitmapped_pack()` to construct the list of reusable paths. But we may also want to do pack-reuse over a single pack, either because we only had one pack to perform reuse over (in the case of single-pack bitmaps), or because we explicitly asked to do single pack reuse even with a MIDX[^1]. When this is the case, the array we generate of reusable packs contains only a single element, which is either (a) the pack attached to the single-pack bitmap, or (b) the MIDX's preferred pack. In 795006f (pack-bitmap: gracefully handle missing BTMP chunks, 2024-04-15), we refactored the reuse_partial_packfile_from_bitmap() function and stopped assigning the pack_int_id field when reusing only the MIDX's preferred pack. This results in an uninitialized read down in try_partial_reuse() like so: ==7474==WARNING: MemorySanitizer: use-of-uninitialized-value #0 0x55c5cd191dde in try_partial_reuse pack-bitmap.c:1887:8 #1 0x55c5cd191dde in reuse_partial_packfile_from_bitmap_1 pack-bitmap.c:2001:8 #2 0x55c5cd191dde in reuse_partial_packfile_from_bitmap pack-bitmap.c:2105:3 #3 0x55c5cce0bd0e in get_object_list_from_bitmap builtin/pack-objects.c:4043:3 #4 0x55c5cce0bd0e in get_object_list builtin/pack-objects.c:4156:27 #5 0x55c5cce0bd0e in cmd_pack_objects builtin/pack-objects.c:4596:3 #6 0x55c5ccc8fac8 in run_builtin git.c:474:11 which happens when try_partial_reuse() tries to call midx_pair_to_pack_pos() when it tries to reject cross-pack deltas. Avoid the uninitialized read by ensuring that the pack_int_id field is set in the single-pack reuse case by setting it to either the MIDX preferred pack's pack_int_id, or '0', in the case of single-pack bitmaps. In the latter case, we never read the pack_int_id field, so the choice of '0' is arbitrary. [^1]: This can happen for a couple of reasons, either because the repository is configured with 'pack.allowPackReuse=(true|single)', or because the MIDX was generated prior to the introduction of the BTMP chunk, which contains information necessary to perform multi-pack reuse. Reported-by: Kyle Lippincott <spectral@google.com> Signed-off-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
1 parent 786a3e4 commit e6aff37

File tree

1 file changed

+10
-0
lines changed

1 file changed

+10
-0
lines changed

Diff for: pack-bitmap.c

+10
Original file line numberDiff line numberDiff line change
@@ -2074,6 +2074,7 @@ void reuse_partial_packfile_from_bitmap(struct bitmap_index *bitmap_git,
20742074
QSORT(packs, packs_nr, bitmapped_pack_cmp);
20752075
} else {
20762076
struct packed_git *pack;
2077+
uint32_t pack_int_id;
20772078

20782079
if (bitmap_is_midx(bitmap_git)) {
20792080
uint32_t preferred_pack_pos;
@@ -2084,12 +2085,21 @@ void reuse_partial_packfile_from_bitmap(struct bitmap_index *bitmap_git,
20842085
}
20852086

20862087
pack = bitmap_git->midx->packs[preferred_pack_pos];
2088+
pack_int_id = preferred_pack_pos;
20872089
} else {
20882090
pack = bitmap_git->pack;
2091+
/*
2092+
* Any value for 'pack_int_id' will do here. When we
2093+
* process the pack via try_partial_reuse(), we won't
2094+
* use the `pack_int_id` field since we have a non-MIDX
2095+
* bitmap.
2096+
*/
2097+
pack_int_id = 0;
20892098
}
20902099

20912100
ALLOC_GROW(packs, packs_nr + 1, packs_alloc);
20922101
packs[packs_nr].p = pack;
2102+
packs[packs_nr].pack_int_id = pack_int_id;
20932103
packs[packs_nr].bitmap_nr = pack->num_objects;
20942104
packs[packs_nr].bitmap_pos = 0;
20952105

0 commit comments

Comments
 (0)