commit e15c42decfa8a80fb91cc1e19b467efc34a8c05d
from: Omar Polo <op@omarpolo.com>
date: Mon Sep 05 12:55:41 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 - 6227cf0ee49b322cc297ef95bdad09ea8eae2ec4
commit + e15c42decfa8a80fb91cc1e19b467efc34a8c05d
blob - 1877932f7650a337705d19c325c27b9f594e54bf
blob + 5826d03babb194bb636de184730aa9fb5c2f3690
--- 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 - c11280e036c43527546ff7dea1fdf989ad7cf971
blob + a884480d0412cfdbd1365bccb5edfed89963a94a
--- 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 - 793795838b82267e63c1ce81996fa76ad63f3860
blob + 92bc6630d663a353d140360680f6e62605ba1842
--- 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 - d3fe18a13136b64b9e9ba153ae5a2deda0482b9c
blob + 9d57af707725df8ed20c18c242b8359ad3a3270c
--- tog/tog.c
+++ tog/tog.c
@@ -2092,12 +2092,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;
 }
@@ -2111,7 +2118,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);
 }