| Message ID | cover.1718392943.git.me@ttaylorr.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | pseudo-merge: various small fixes | expand |
On Fri, Jun 14, 2024 at 7:23 PM Taylor Blau <me@ttaylorr.com> wrote: > > Here is a small reroll of a couple of patches I wrote to fix various > small issues with the tb/pseudo-merge-reachability-bitmaps topic. > > The only change since last time is replacing: > > if (st_mult(index->pseudo_merges.nr, sizeof(uint64_t)) > table_size - 24) > > with: > > if (st_add(st_mult(index->pseudo_merges.nr, sizeof(uint64_t)), 24) > table_size) > > based on helpful review from Junio. For convenience, a range-diff is > below. Thanks in advance for any final review on this topic :-). > > Taylor Blau (2): > Documentation/technical/bitmap-format.txt: add missing position table > pack-bitmap.c: ensure pseudo-merge offset reads are bounded > > Documentation/technical/bitmap-format.txt | 9 +++++++++ > pack-bitmap.c | 5 +++++ > 2 files changed, 14 insertions(+) > > Range-diff against v1: > -: ---------- > 1: a71ec05e5d Documentation/technical/bitmap-format.txt: add missing position table > 1: 0a16399d14 ! 2: 8abd564e7c pack-bitmap.c: ensure pseudo-merge offset reads are bounded > @@ Commit message > end of the mmap'd region. > > Prevent this by ensuring that we have at least `table_size - 24` many > - bytes available to read (subtracting 24 as the length of the metadata > - component). > + bytes available to read (adding 24 to the left-hand side of our > + inequality to account for the length of the metadata component). > > This is sufficient to prevent us from reading off the end of the > pseudo-merge extension, and ensures that all of the get_be64() calls > @@ pack-bitmap.c: static int load_bitmap_header(struct bitmap_index *index) > index->pseudo_merges.commits_nr = get_be32(index_end - 20); > index->pseudo_merges.nr = get_be32(index_end - 24); > > -+ if (st_mult(index->pseudo_merges.nr, sizeof(uint64_t)) > table_size - 24) > ++ if (st_add(st_mult(index->pseudo_merges.nr, > ++ sizeof(uint64_t)), > ++ 24) > table_size) > + return error(_("corrupted bitmap index file, pseudo-merge table too short")); > + > CALLOC_ARRAY(index->pseudo_merges.v, > > base-commit: 0b7500dc66ffcb6b1ccc3332715936a59c6b5ce4 > -- > 2.45.0.33.g0a16399d14.dirty Series looks good to me.
Elijah Newren <newren@gmail.com> writes: > On Fri, Jun 14, 2024 at 7:23 PM Taylor Blau <me@ttaylorr.com> wrote: >> >> Here is a small reroll of a couple of patches I wrote to fix various >> small issues with the tb/pseudo-merge-reachability-bitmaps topic. >> > ... > Series looks good to me. Thanks, it looked good to me, too. Let's roll it into the base topic that has been waiting in 'next'.
Here is a small reroll of a couple of patches I wrote to fix various small issues with the tb/pseudo-merge-reachability-bitmaps topic. The only change since last time is replacing: if (st_mult(index->pseudo_merges.nr, sizeof(uint64_t)) > table_size - 24) with: if (st_add(st_mult(index->pseudo_merges.nr, sizeof(uint64_t)), 24) > table_size) based on helpful review from Junio. For convenience, a range-diff is below. Thanks in advance for any final review on this topic :-). Taylor Blau (2): Documentation/technical/bitmap-format.txt: add missing position table pack-bitmap.c: ensure pseudo-merge offset reads are bounded Documentation/technical/bitmap-format.txt | 9 +++++++++ pack-bitmap.c | 5 +++++ 2 files changed, 14 insertions(+) Range-diff against v1: -: ---------- > 1: a71ec05e5d Documentation/technical/bitmap-format.txt: add missing position table 1: 0a16399d14 ! 2: 8abd564e7c pack-bitmap.c: ensure pseudo-merge offset reads are bounded @@ Commit message end of the mmap'd region. Prevent this by ensuring that we have at least `table_size - 24` many - bytes available to read (subtracting 24 as the length of the metadata - component). + bytes available to read (adding 24 to the left-hand side of our + inequality to account for the length of the metadata component). This is sufficient to prevent us from reading off the end of the pseudo-merge extension, and ensures that all of the get_be64() calls @@ pack-bitmap.c: static int load_bitmap_header(struct bitmap_index *index) index->pseudo_merges.commits_nr = get_be32(index_end - 20); index->pseudo_merges.nr = get_be32(index_end - 24); -+ if (st_mult(index->pseudo_merges.nr, sizeof(uint64_t)) > table_size - 24) ++ if (st_add(st_mult(index->pseudo_merges.nr, ++ sizeof(uint64_t)), ++ 24) > table_size) + return error(_("corrupted bitmap index file, pseudo-merge table too short")); + CALLOC_ARRAY(index->pseudo_merges.v, base-commit: 0b7500dc66ffcb6b1ccc3332715936a59c6b5ce4 -- 2.45.0.33.g0a16399d14.dirty