[PATCH 4/5] builtin/repack.c: be more conservative with unsigned overflows
- From
Taylor Blau <me@ttaylorr.com>
- Date
- Mar 5, 2021, 15:21 UTC
- Message-ID
- <d55324f7a256fce491a29a1debf142f817eb01d3.1614957681.git.me@ttaylorr.com>
- In-Reply-To
- <cover.1614957681.git.me@ttaylorr.com>
There are a number of places in the geometric repack code where we multiply the number of objects in a pack by another unsigned value. We trust that the number of objects in a pack is always representable by a uint32_t, but we don't necessarily trust that that number can be multiplied without overflow.
Sprinkle some unsigned_add_overflows() and unsigned_mult_overflows() in split_pack_geometry() to check that we never overflow any unsigned types when adding or multiplying them.
Arguably these checks are a little too conservative, and certainly they do not help the readability of this function. But they are serving a useful purpose, so I think they are worthwhile overall.
Suggested-by: Junio C Hamano <gitster@pobox.com> Signed-off-by: Taylor Blau <me@ttaylorr.com> --- builtin/repack.c | 24 ++++++++++++++++++++++-- 1 file changed, 22 insertions(+), 2 deletions(-)
diff --git a/builtin/repack.c b/builtin/repack.c index 21a5778e73..677c6b75c1 100644 --- a/builtin/repack.c +++ b/builtin/repack.c @@ -363,6 +363,12 @@ static void split_pack_geometry(struct pack_geometry *geometry, int factor) for (i = geometry->pack_nr - 1; i > 0; i--) { struct packed_git *ours = geometry->pack[i]; struct packed_git *prev = geometry->pack[i - 1]; + + if (unsigned_mult_overflows(factor, geometry_pack_weight(prev))) + die(_("pack %s too large to consider in geometric " + "progression"), + prev->pack_name); + if (geometry_pack_weight(ours) < factor * geometry_pack_weight(prev)) break; } @@ -388,11 +394,25 @@ static void split_pack_geometry(struct pack_geometry *geometry, int factor) * packs in the heavy half need to be joined into it (if any) to restore * the geometric progression. */ - for (i = 0; i < split; i++) - total_size += geometry_pack_weight(geometry->pack[i]); + for (i = 0; i < split; i++) { + struct packed_git *p = geometry->pack[i]; + + if (unsigned_add_overflows(total_size, geometry_pack_weight(p))) + die(_("pack %s too large to roll up"), p->pack_name); + total_size += geometry_pack_weight(p); + } for (i = split; i < geometry->pack_nr; i++) { struct packed_git *ours = geometry->pack[i]; + + if (unsigned_mult_overflows(factor, total_size)) + die(_("pack %s too large to roll up"), ours->pack_name); + if (geometry_pack_weight(ours) < factor * total_size) { + if (unsigned_add_overflows(total_size, + geometry_pack_weight(ours))) + die(_("pack %s too large to roll up"), + ours->pack_name); + split++; total_size += geometry_pack_weight(ours); } else
-- 2.30.0.667.g81c0cbc6fd