Re: [PATCH 03/23] pack-bitmap: bounds-check size of cache extension

From: Taylor Blau <me@ttaylorr.com>
Date: Nov 13, 2020, 21:29 UTC
Message-ID: <X6760infcF0hRYTG@nand.local>
In-Reply-To: <20201113045700.GA743619@coredump.intra.peff.net>

On Thu, Nov 12, 2020 at 11:57:00PM -0500, Jeff King wrote:

Show 20 quoted lines

> On Thu, Nov 12, 2020 at 06:47:09PM +0100, Martin Ågren wrote:
>
> > The addition should be ok or mmap has failed on us. Do we know that we
> > have room for the final hash there so that the subtraction is ok? Yes,
> > from the previous commit, we know we have room for the header, which is
> > even larger. But that's cheating a bit -- see below.
>
> Yeah, I agree this ought to be checking the minimum size against the
> header _plus_ the trailer.
>
> I think the previous patch is actually where it goes wrong. The original
> was checking for a minimum of:
>
>   if (index->map_size < sizeof(*header) + the_hash_algo->rawsz)
>
> which is the header plus the trailer. We want to readjust for the
> MAX_RAWSZ part of the header, so it should be:
>
>   size_t header_size = sizeof(*header) - GIT_MAX_RAWSZ + the_hash_algo->rawsz;
>   if (index->map_size < sizeof(*header) + the_hash_algo->rawsz)

I'm not sure that I follow. If you apply this to the second patch in this series, the only thing that changes is that it factors out:

  index->map_pos += ...;

into

  size_t header_size = ...;
  // ...
  index->map_pos += header_size;

What am I missing here?

Thanks, Taylor