commit-graph: simplify parse_commit_graph() #1

While we iterate over all entries of the Chunk Lookup table we make sure that we don't attempt to read past the end of the mmap-ed commit-graph file, and check in each iteration that the chunk ID and offset we are about to read is still within the mmap-ed memory region. However, these checks in each iteration are not really necessary, because the number of chunks in the commit-graph file is already known before this loop from the just parsed commit-graph header. So let's check that the commit-graph file is large enough for all entries in the Chunk Lookup table before we start iterating over those entries, and drop those per-iteration checks. While at it, take into account the size of everything that is necessary to have a valid commit-graph file, i.e. the size of the header, the size of the mandatory OID Fanout chunk, and the size of the signature in the trailer as well. Note that this necessitates the change of the error message as well, and, consequently, have to update the 'detect incorrect chunk count' test in 't5318-commit-graph.sh' as well. Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com> Signed-off-by: Derrick Stolee <dstolee@microsoft.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

SZEDER Gábor committed Jun 5, 2020 at 13:00 UTC 2ad4f1a7c43cac4ba52cff56f9090d3322c67224

2 files changed +11 -8

commit-graph.c

+9 -7

@@ -272,6 +272,15 @@ struct commit_graph *parse_commit_graph(void *graph_map, int fd,

272 graph->data = graph_map;

273 graph->data_len = graph_size;

274

275 + if (graph_size < GRAPH_HEADER_SIZE +

276 + (graph->num_chunks + 1) * GRAPH_CHUNKLOOKUP_WIDTH +

277 + GRAPH_FANOUT_SIZE + the_hash_algo->rawsz) {

278 + error(_("commit-graph file is too small to hold %u chunks"),

279 + graph->num_chunks);

280 + free(graph);

281 + return NULL;

282 + }

283 +

284 last_chunk_id = 0;

285 last_chunk_offset = 8;

286 chunk_lookup = data + 8;

@@ -280,13 +289,6 @@ struct commit_graph *parse_commit_graph(void *graph_map, int fd,

289 uint64_t chunk_offset;

290 int chunk_repeated = 0;

291

283 - if (data + graph_size - chunk_lookup <

284 - GRAPH_CHUNKLOOKUP_WIDTH) {

285 - error(_("commit-graph chunk lookup table entry missing; file may be incomplete"));

286 - free(graph);

287 - return NULL;

288 - }

289 -

292 chunk_id = get_be32(chunk_lookup + 0);

293 chunk_offset = get_be64(chunk_lookup + 4);

294

t/t5318-commit-graph.sh

+2 -1

@@ -574,7 +574,8 @@ test_expect_success 'detect invalid checksum hash' '

574

575 test_expect_success 'detect incorrect chunk count' '

576 corrupt_graph_and_verify $GRAPH_BYTE_CHUNK_COUNT "\377" \

577 - "chunk lookup table entry missing" $GRAPH_CHUNK_LOOKUP_OFFSET

577 + "commit-graph file is too small to hold [0-9]* chunks" \

578 + $GRAPH_CHUNK_LOOKUP_OFFSET

579 '

580

581 test_expect_success 'git fsck (checks commit-graph)' '