@matheusbsbs / matheus-bagliano / commits / fbda77c6c0
Fork 0

commit-graph: avoid memory leaks

A fuzzer running on the entry point provided by fuzz-commit-graph.c revealed a memory leak when parse_commit_graph() creates a struct bloom_filter_settings and then returns early due to error. Fix that error by always freeing that struct first (if it exists) before returning early due to error. While making that change, I also noticed another possible memory leak - when the BLOOMDATA chunk is provided but not BLOOMINDEXES. Also fix that error. Signed-off-by: Jonathan Tan <jonathantanmy@google.com> Reviewed-by: Derrick Stolee <dstolee@microsoft.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Jonathan Tan committed May 4, 2020 at 12:13 UTC fbda77c6c00a7cec65c4f266b09252df73a9deff
1 file changed +11 -11
commit-graph.c
+11 -11
@@ -271,8 +271,7 @@ struct commit_graph *parse_commit_graph(void *graph_map, int fd,
271 if (data + graph_size - chunk_lookup <
272 GRAPH_CHUNKLOOKUP_WIDTH) {
273 error(_("commit-graph chunk lookup table entry missing; file may be incomplete"));
274 - free(graph);
275 - return NULL;
274 + goto free_and_return;
275 }
276
277 chunk_id = get_be32(chunk_lookup + 0);
@@ -283,8 +282,7 @@ struct commit_graph *parse_commit_graph(void *graph_map, int fd,
282 if (chunk_offset > graph_size - the_hash_algo->rawsz) {
283 error(_("commit-graph improper chunk offset %08x%08x"), (uint32_t)(chunk_offset >> 32),
284 (uint32_t)chunk_offset);
286 - free(graph);
287 - return NULL;
285 + goto free_and_return;
286 }
287
288 switch (chunk_id) {
@@ -351,8 +349,7 @@ struct commit_graph *parse_commit_graph(void *graph_map, int fd,
349
350 if (chunk_repeated) {
351 error(_("commit-graph chunk id %08x appears multiple times"), chunk_id);
354 - free(graph);
355 - return NULL;
352 + goto free_and_return;
353 }
354
355 if (last_chunk_id == GRAPH_CHUNKID_OIDLOOKUP)
@@ -371,17 +368,20 @@ struct commit_graph *parse_commit_graph(void *graph_map, int fd,
368 /* We need both the bloom chunks to exist together. Else ignore the data */
369 graph->chunk_bloom_indexes = NULL;
370 graph->chunk_bloom_data = NULL;
374 - graph->bloom_filter_settings = NULL;
371 + FREE_AND_NULL(graph->bloom_filter_settings);
372 }
373
374 hashcpy(graph->oid.hash, graph->data + graph->data_len - graph->hash_len);
375
379 - if (verify_commit_graph_lite(graph)) {
380 - free(graph);
381 - return NULL;
382 - }
376 + if (verify_commit_graph_lite(graph))
377 + goto free_and_return;
378
379 return graph;
380 +
381 +free_and_return:
382 + free(graph->bloom_filter_settings);
383 + free(graph);
384 + return NULL;
385 }
386
387 static struct commit_graph *load_commit_graph_one(const char *graph_file,