commit d68b973725ce0a2c7b1f5094448a444d4f2effdc
from: Omar Polo <op@omarpolo.com>
via: Thomas Adam <thomas@xteddy.org>
date: Mon Sep 05 13:07:23 2022 UTC

plug leak in the commit graph iterator

We fail to release the memory for the nodes.  To fix it however, we some
consumer of the commit graph iterator need to be corrected: the returned
pointer is safe to be used only up until the next iter_next call;
save a copy it if it's needed afterwards too.

ok stsp@

commit - 95326260eb5a3786dbd04e735b6c8c9313523ff2
commit + d68b973725ce0a2c7b1f5094448a444d4f2effdc
blob - df390724e9edfa06b08182337c4be01973733a6f
blob + aa27a2d638ce25060a6d3de6eb3f8dc63a7b40aa
--- got/got.c
+++ got/got.c
@@ -9708,6 +9708,7 @@ collect_commits(struct got_object_id_queue *commits,
 	struct got_object_id *parent_id = NULL;
 	struct got_object_qid *qid;
         struct got_object_id *commit_id = initial_commit_id;
+	struct got_object_id *tmp = NULL;
 
 	err = got_commit_graph_open(&graph, "/", 1);
 	if (err)
@@ -9738,10 +9739,19 @@ collect_commits(struct got_object_id_queue *commits,
 			if (err)
 				goto done;
 			STAILQ_INSERT_HEAD(commits, qid, entry);
-			commit_id = parent_id;
+
+			free(tmp);
+			tmp = got_object_id_dup(parent_id);
+			if (tmp == NULL) {
+				err = got_error_from_errno(
+				    "got_object_id_dup");
+				goto done;
+			}
+			commit_id = tmp;
 		}
 	}
 done:
+	free(tmp);
 	got_commit_graph_close(graph);
 	return err;
 }
blob - 5a86e1b7b7cbe92252f4617311f6639b8edb06c5
blob + 46796cfdc61a895985d270fcd52fa5ff3f3da374
--- lib/blame.c
+++ lib/blame.c
@@ -595,7 +595,12 @@ blame_open(struct got_blame **blamep, const char *path
 			goto done;
 		}
 		if (next_id) {
-			id = next_id;
+			free(id);
+			id = got_object_id_dup(next_id);
+			if (id == NULL) {
+				err = got_error_from_errno("got_object_id_dup");
+				goto done;
+			}
 			err = blame_commit(blame, id, path, repo, cb, arg);
 			if (err) {
 				if (err->code == GOT_ERR_ITER_COMPLETED)
@@ -628,6 +633,7 @@ done:
 	if (graph)
 		got_commit_graph_close(graph);
 	free(obj_id);
+	free(id);
 	if (blob)
 		got_object_blob_close(blob);
 	if (start_commit)
blob - 459c3b19abc1463afcdca9782c4979020bd46091
blob + 69e0e7352395e10354f4f67c96c98956bf34910f
--- lib/commit_graph.c
+++ lib/commit_graph.c
@@ -90,6 +90,12 @@ struct got_commit_graph {
 	 * commit timestmap.
 	 */
 	struct got_commit_graph_iter_list iter_list;
+
+	/*
+	 * Temporary storage for the id returned by
+	 * got_commit_graph_iter_next.
+	 */
+	struct got_object_id id;
 };
 
 static const struct got_error *
@@ -614,8 +620,11 @@ got_commit_graph_iter_next(struct got_object_id **id,
 			return err;
 	}
 
-	*id = &node->id;
+	memcpy(&graph->id, &node->id, sizeof(graph->id));
+	*id = &graph->id;
+
 	TAILQ_REMOVE(&graph->iter_list, node, entry);
+	free(node);
 	return NULL;
 }
 
blob - 347c7a5386f3266e0392dbb930896feeb8278b52
blob + bfc4692ed93bfd9f74dea2238d3700924e4d9a4f
--- tog/tog.c
+++ tog/tog.c
@@ -2097,12 +2097,19 @@ alloc_commit_queue_entry(struct got_commit_object *com
     struct got_object_id *id)
 {
 	struct commit_queue_entry *entry;
+	struct got_object_id *dup;
 
 	entry = calloc(1, sizeof(*entry));
 	if (entry == NULL)
 		return NULL;
 
-	entry->id = id;
+	dup = got_object_id_dup(id);
+	if (dup == NULL) {
+		free(entry);
+		return NULL;
+	}
+
+	entry->id = dup;
 	entry->commit = commit;
 	return entry;
 }
@@ -2116,7 +2123,7 @@ pop_commit(struct commit_queue *commits)
 	TAILQ_REMOVE(&commits->head, entry, entry);
 	got_object_commit_close(entry->commit);
 	commits->ncommits--;
-	/* Don't free entry->id! It is owned by the commit graph. */
+	free(entry->id);
 	free(entry);
 }