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 795006fff4 (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@xxxxxxxxxx>
Signed-off-by: Taylor Blau <me@xxxxxxxxxxxx>
---
pack-bitmap.c | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/pack-bitmap.c b/pack-bitmap.c
index fe8e8a51d3..8b9a2c698f 100644
--- a/pack-bitmap.c
+++ b/pack-bitmap.c
@@ -2073,6 +2073,7 @@ void reuse_partial_packfile_from_bitmap(struct bitmap_index *bitmap_git,
QSORT(packs, packs_nr, bitmapped_pack_cmp);
} else {
struct packed_git *pack;
+ uint32_t pack_int_id;
if (bitmap_is_midx(bitmap_git)) {
uint32_t preferred_pack_pos;
@@ -2083,12 +2084,21 @@ void reuse_partial_packfile_from_bitmap(struct bitmap_index *bitmap_git,
}
pack = bitmap_git->midx->packs[preferred_pack_pos];
+ pack_int_id = preferred_pack_pos;
} else {
pack = bitmap_git->pack;
+ /*
+ * Any value for 'pack_int_id' will do here. When we
+ * process the pack via try_partial_reuse(), we won't
+ * use the `pack_int_id` field since we have a non-MIDX
+ * bitmap.
+ */
+ pack_int_id = 0;
}
ALLOC_GROW(packs, packs_nr + 1, packs_alloc);
packs[packs_nr].p = pack;
+ packs[packs_nr].pack_int_id = pack_int_id;
packs[packs_nr].bitmap_nr = pack->num_objects;
packs[packs_nr].bitmap_pos = 0;
--
2.45.2.445.g1b76f06508
