commit-graph: verify parent list

The commit-graph file stores parents in a two-column portion of the commit data chunk. If there is only one parent, then the second column stores 0xFFFFFFFF to indicate no second parent. The 'verify' subcommand checks the parent list for the commit loaded from the commit-graph and the one parsed from the object database. Test these checks for corrupt parents, too many parents, and wrong parents. Add a boundary check to insert_parent_or_die() for when the parent position value is out of range. The octopus merge will be tested in a later commit. Signed-off-by: Derrick Stolee <dstolee@microsoft.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Derrick Stolee committed Jun 27, 2018 at 09:24 UTC 53614b13516ffd27bb045b696d0815b9da435cb0
2 files changed +46
commit-graph.c
+28
@@ -244,6 +244,9 @@ static struct commit_list **insert_parent_or_die(struct commit_graph *g,
244 struct commit *c;
245 struct object_id oid;
246
247 + if (pos >= g->num_commits)
248 + die("invalid parent position %"PRIu64, pos);
249 +
250 hashcpy(oid.hash, g->chunk_oid_lookup + g->hash_len * pos);
251 c = lookup_commit(&oid);
252 if (!c)
@@ -907,6 +910,7 @@ int verify_commit_graph(struct repository *r, struct commit_graph *g)
910
911 for (i = 0; i < g->num_commits; i++) {
912 struct commit *graph_commit, *odb_commit;
913 + struct commit_list *graph_parents, *odb_parents;
914
915 hashcpy(cur_oid.hash, g->chunk_oid_lookup + g->hash_len * i);
916
@@ -924,6 +928,30 @@ int verify_commit_graph(struct repository *r, struct commit_graph *g)
928 oid_to_hex(&cur_oid),
929 oid_to_hex(get_commit_tree_oid(graph_commit)),
930 oid_to_hex(get_commit_tree_oid(odb_commit)));
931 +
932 + graph_parents = graph_commit->parents;
933 + odb_parents = odb_commit->parents;
934 +
935 + while (graph_parents) {
936 + if (odb_parents == NULL) {
937 + graph_report("commit-graph parent list for commit %s is too long",
938 + oid_to_hex(&cur_oid));
939 + break;
940 + }
941 +
942 + if (oidcmp(&graph_parents->item->object.oid, &odb_parents->item->object.oid))
943 + graph_report("commit-graph parent for %s is %s != %s",
944 + oid_to_hex(&cur_oid),
945 + oid_to_hex(&graph_parents->item->object.oid),
946 + oid_to_hex(&odb_parents->item->object.oid));
947 +
948 + graph_parents = graph_parents->next;
949 + odb_parents = odb_parents->next;
950 + }
951 +
952 + if (odb_parents != NULL)
953 + graph_report("commit-graph parent list for commit %s terminates early",
954 + oid_to_hex(&cur_oid));
955 }
956
957 return verify_commit_graph_error;
t/t5318-commit-graph.sh
+18
@@ -269,6 +269,9 @@ GRAPH_BYTE_OID_LOOKUP_ORDER=$(($GRAPH_OID_LOOKUP_OFFSET + $HASH_LEN * 8))
269 GRAPH_BYTE_OID_LOOKUP_MISSING=$(($GRAPH_OID_LOOKUP_OFFSET + $HASH_LEN * 4 + 10))
270 GRAPH_COMMIT_DATA_OFFSET=$(($GRAPH_OID_LOOKUP_OFFSET + $HASH_LEN * $NUM_COMMITS))
271 GRAPH_BYTE_COMMIT_TREE=$GRAPH_COMMIT_DATA_OFFSET
272 +GRAPH_BYTE_COMMIT_PARENT=$(($GRAPH_COMMIT_DATA_OFFSET + $HASH_LEN))
273 +GRAPH_BYTE_COMMIT_EXTRA_PARENT=$(($GRAPH_COMMIT_DATA_OFFSET + $HASH_LEN + 4))
274 +GRAPH_BYTE_COMMIT_WRONG_PARENT=$(($GRAPH_COMMIT_DATA_OFFSET + $HASH_LEN + 3))
275
276 # usage: corrupt_graph_and_verify <position> <data> <string>
277 # Manipulates the commit-graph file at the position
@@ -348,4 +351,19 @@ test_expect_success 'detect incorrect tree OID' '
351 "root tree OID for commit"
352 '
353
354 +test_expect_success 'detect incorrect parent int-id' '
355 + corrupt_graph_and_verify $GRAPH_BYTE_COMMIT_PARENT "\01" \
356 + "invalid parent"
357 +'
358 +
359 +test_expect_success 'detect extra parent int-id' '
360 + corrupt_graph_and_verify $GRAPH_BYTE_COMMIT_EXTRA_PARENT "\00" \
361 + "is too long"
362 +'
363 +
364 +test_expect_success 'detect wrong parent' '
365 + corrupt_graph_and_verify $GRAPH_BYTE_COMMIT_WRONG_PARENT "\01" \
366 + "commit-graph parent for"
367 +'
368 +
369 test_done