Derrick Stolee <stolee@xxxxxxxxx> writes:
> On 5/30/2018 6:24 PM, Jakub Narebski wrote:
[...]
>> NOTE: we will be checking Commit Data chunk; I think it would be good
>> idea to verify that size of Commit Data chunk matches (N * (H + 16) bytes)
>> that format gives us, so that we don't accidentally red outside of
>> memory if something got screwed up (like number of commits being wrong,
>> or file truncated).
>
> This is actually how we calculate 'num_commits' during
> load_commit_graph_one():
>
> if (last_chunk_id == GRAPH_CHUNKID_OIDLOOKUP)
> {
> graph->num_commits = (chunk_offset - last_chunk_offset)
> / graph->hash_len;
> }
>
> So, if the chunk doesn't match N*(H+16), we detect this because
> FANOUT[255] != N.
>
> (There is one caveat here: (chunk_offset - last_chunk_offset) may not
> be a multiple of hash_len, and "accidentally" truncate to N in the
> division. I'll add more careful checks for this.)
I have thought for some reason that number of commits N was stored
somewhere directly in the commit-graph header.
Anyway we have three places that we can calculate (or simply read in
case of OID Fanour chunk) the number of commits:
- FANOUT[255] == N
- OID Lookup size = (N * H bytes)
- N = (OID Lookup size) / hash_len
- (OID Lookup size) % hash_len == 0
- Commit Data size = (N * (H + 16) bytes)
- N = (Commir Data size) / (hash_len + 16)
- (Commir Data size) % (hash_len + 16) == 0
>
> We also check out-of-bounds offsets in that method.
Good.
Best,
--
Jakub Narębski
