@matheusbsbs / matheus-bagliano / commits / 2a15964128

builtin/repack.c: be more conservative with unsigned overflows

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> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Taylor Blau committed Mar 5, 2021 at 10:21 UTC 2a15964128edd08278efeacc75e75c77cfdde77e
1 file changed +22 -2
builtin/repack.c
+22 -2
@@ -363,6 +363,12 @@ static void split_pack_geometry(struct pack_geometry *geometry, int factor)
363 for (i = geometry->pack_nr - 1; i > 0; i--) {
364 struct packed_git *ours = geometry->pack[i];
365 struct packed_git *prev = geometry->pack[i - 1];
366 +
367 + if (unsigned_mult_overflows(factor, geometry_pack_weight(prev)))
368 + die(_("pack %s too large to consider in geometric "
369 + "progression"),
370 + prev->pack_name);
371 +
372 if (geometry_pack_weight(ours) < factor * geometry_pack_weight(prev))
373 break;
374 }
@@ -388,11 +394,25 @@ static void split_pack_geometry(struct pack_geometry *geometry, int factor)
394 * packs in the heavy half need to be joined into it (if any) to restore
395 * the geometric progression.
396 */
391 - for (i = 0; i < split; i++)
392 - total_size += geometry_pack_weight(geometry->pack[i]);
397 + for (i = 0; i < split; i++) {
398 + struct packed_git *p = geometry->pack[i];
399 +
400 + if (unsigned_add_overflows(total_size, geometry_pack_weight(p)))
401 + die(_("pack %s too large to roll up"), p->pack_name);
402 + total_size += geometry_pack_weight(p);
403 + }
404 for (i = split; i < geometry->pack_nr; i++) {
405 struct packed_git *ours = geometry->pack[i];
406 +
407 + if (unsigned_mult_overflows(factor, total_size))
408 + die(_("pack %s too large to roll up"), ours->pack_name);
409 +
410 if (geometry_pack_weight(ours) < factor * total_size) {
411 + if (unsigned_add_overflows(total_size,
412 + geometry_pack_weight(ours)))
413 + die(_("pack %s too large to roll up"),
414 + ours->pack_name);
415 +
416 split++;
417 total_size += geometry_pack_weight(ours);
418 } else