On Fri, Jun 14, 2024 at 7:23 PM Taylor Blau <me@xxxxxxxxxxxx> 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.
