@matheusbsbs / matheus-bagliano / commits / 41df0e307f

commit-graph: verify contents match checksum

The commit-graph file ends with a SHA1 hash of the previous contents. If a commit-graph file has errors but the checksum hash is correct, then we know that the problem is a bug in Git and not simply file corruption after-the-fact. Compute the checksum right away so it is the first error that appears, and make the message translatable since this error can be "corrected" by a user by simply deleting the file and recomputing. The rest of the errors are useful only to developers. Be sure to continue checking the rest of the file data if the checksum is wrong. This is important for our tests, as we break the checksum as we modify bytes of the commit-graph file. 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 41df0e307fede8ad01799322af41d8b59d2f6edf
2 files changed +20 -2
commit-graph.c
+14 -2
@@ -833,6 +833,7 @@ void write_commit_graph(const char *obj_dir,
833 oids.nr = 0;
834 }
835
836 +#define VERIFY_COMMIT_GRAPH_ERROR_HASH 2
837 static int verify_commit_graph_error;
838
839 static void graph_report(const char *fmt, ...)
@@ -852,8 +853,10 @@ static void graph_report(const char *fmt, ...)
853 int verify_commit_graph(struct repository *r, struct commit_graph *g)
854 {
855 uint32_t i, cur_fanout_pos = 0;
855 - struct object_id prev_oid, cur_oid;
856 + struct object_id prev_oid, cur_oid, checksum;
857 int generation_zero = 0;
858 + struct hashfile *f;
859 + int devnull;
860
861 if (!g) {
862 graph_report("no commit-graph file loaded");
@@ -872,6 +875,15 @@ int verify_commit_graph(struct repository *r, struct commit_graph *g)
875 if (verify_commit_graph_error)
876 return verify_commit_graph_error;
877
878 + devnull = open("/dev/null", O_WRONLY);
879 + f = hashfd(devnull, NULL);
880 + hashwrite(f, g->data, g->data_len - g->hash_len);
881 + finalize_hashfile(f, checksum.hash, CSUM_CLOSE);
882 + if (hashcmp(checksum.hash, g->data + g->data_len - g->hash_len)) {
883 + graph_report(_("the commit-graph file has incorrect checksum and is likely corrupt"));
884 + verify_commit_graph_error = VERIFY_COMMIT_GRAPH_ERROR_HASH;
885 + }
886 +
887 for (i = 0; i < g->num_commits; i++) {
888 struct commit *graph_commit;
889
@@ -909,7 +921,7 @@ int verify_commit_graph(struct repository *r, struct commit_graph *g)
921 cur_fanout_pos++;
922 }
923
912 - if (verify_commit_graph_error)
924 + if (verify_commit_graph_error & ~VERIFY_COMMIT_GRAPH_ERROR_HASH)
925 return verify_commit_graph_error;
926
927 for (i = 0; i < g->num_commits; i++) {
t/t5318-commit-graph.sh
+6
@@ -279,6 +279,7 @@ GRAPH_COMMIT_DATA_WIDTH=$(($HASH_LEN + 16))
279 GRAPH_OCTOPUS_DATA_OFFSET=$(($GRAPH_COMMIT_DATA_OFFSET + \
280 $GRAPH_COMMIT_DATA_WIDTH * $NUM_COMMITS))
281 GRAPH_BYTE_OCTOPUS=$(($GRAPH_OCTOPUS_DATA_OFFSET + 4))
282 +GRAPH_BYTE_FOOTER=$(($GRAPH_OCTOPUS_DATA_OFFSET + 4 * $NUM_OCTOPUS_EDGES))
283
284 # usage: corrupt_graph_and_verify <position> <data> <string>
285 # Manipulates the commit-graph file at the position
@@ -393,4 +394,9 @@ test_expect_success 'detect incorrect parent for octopus merge' '
394 "invalid parent"
395 '
396
397 +test_expect_success 'detect invalid checksum hash' '
398 + corrupt_graph_and_verify $GRAPH_BYTE_FOOTER "\00" \
399 + "incorrect checksum"
400 +'
401 +
402 test_done