commit f8c03d0287aa3a745bc50244731a8a8a31ca7a5a
from: Kyle Ackerman <kackerman0102@gmail.com>
via: Thomas Adam <thomas@xteddy.org>
date: Sat Mar 30 17:21:23 2024 UTC

plug a memory leak in 'got blame'

The leak is present in got_privsep_recv_traversed_commits. There is
an edge case where it receives consecutive imsgs. The first behaves as
normal and we got_object_id_dup the last commit id for
changed_commit_id. The following imsg(s) then still allocates the last
commit id, leaking the one(s) prior allocated.

Patch by Kyle Ackerman

commit - 22af6a95d11edbe6ded4e23d85858238a9be64e2
commit + f8c03d0287aa3a745bc50244731a8a8a31ca7a5a
blob - c1ab1a494a04643b73a4f64ba8ad9f03e68f6285
blob + e585687a74d6708a955fac1e86ce92bed195007c
--- lib/got_lib_privsep.h
+++ lib/got_lib_privsep.h
@@ -787,8 +787,8 @@ const struct got_error *got_privsep_recv_gotconfig_rem
 const struct got_error *got_privsep_send_commit_traversal_request(
     struct imsgbuf *, struct got_object_id *, int, const char *);
 const struct got_error *got_privsep_recv_traversed_commits(
-    struct got_commit_object **, struct got_object_id **,
-    struct got_object_id_queue *, struct imsgbuf *);
+    struct got_commit_object **, struct got_object_id_queue *,
+    struct imsgbuf *);
 const struct got_error *got_privsep_send_enumerated_tree(size_t *,
     struct imsgbuf *, struct got_object_id *, const char *,
     struct got_parsed_tree_entry *, int);
blob - 6c527f7bbd1fc97f1c593f5347e467fa88958c52
blob + dc2f8cceed6ff8d8404b533fe7804d2c13fbca58
--- lib/object_open_privsep.c
+++ lib/object_open_privsep.c
@@ -25,6 +25,7 @@
 
 #include <errno.h>
 #include <imsg.h>
+#include <stddef.h>
 #include <stdio.h>
 #include <stdint.h>
 #include <stdlib.h>
@@ -1213,7 +1214,7 @@ got_traverse_packed_commits(struct got_object_id_queue
 	struct got_packidx *packidx = NULL;
 	char *path_packfile = NULL;
 	struct got_commit_object *changed_commit = NULL;
-	struct got_object_id *changed_commit_id = NULL;
+	struct got_object_qid *changed_commit_qid = NULL;
 	int idx;
 
 	err = got_repo_search_packidx(&packidx, &idx, repo, commit_id);
@@ -1247,7 +1248,7 @@ got_traverse_packed_commits(struct got_object_id_queue
 		goto done;
 
 	err = got_privsep_recv_traversed_commits(&changed_commit,
-	    &changed_commit_id, traversed_commits, pack->privsep_child->ibuf);
+	    traversed_commits, pack->privsep_child->ibuf);
 	if (err)
 		goto done;
 
@@ -1257,13 +1258,13 @@ got_traverse_packed_commits(struct got_object_id_queue
 		 * This commit might be opened again soon.
 		 */
 		changed_commit->refcnt++;
-		err = got_repo_cache_commit(repo, changed_commit_id,
+		changed_commit_qid = STAILQ_LAST(traversed_commits, got_object_qid, entry);
+		err = got_repo_cache_commit(repo, &changed_commit_qid->id,
 		    changed_commit);
 		got_object_commit_close(changed_commit);
 	}
 done:
 	free(path_packfile);
-	free(changed_commit_id);
 	return err;
 }
 
blob - 59d1b5dc5a40234f1de473efcb3dfe515584fea3
blob + cfe6f82306dd349e977c779c4a3e58d9b231924c
--- lib/privsep.c
+++ lib/privsep.c
@@ -2692,7 +2692,6 @@ got_privsep_send_commit_traversal_request(struct imsgb
 
 const struct got_error *
 got_privsep_recv_traversed_commits(struct got_commit_object **changed_commit,
-    struct got_object_id **changed_commit_id,
     struct got_object_id_queue *commit_ids, struct imsgbuf *ibuf)
 {
 	const struct got_error *err = NULL;
@@ -2703,7 +2702,6 @@ got_privsep_recv_traversed_commits(struct got_commit_o
 	int i, done = 0;
 
 	*changed_commit = NULL;
-	*changed_commit_id = NULL;
 
 	while (!done) {
 		err = got_privsep_recv_imsg(&imsg, ibuf, 0);
@@ -2729,23 +2727,9 @@ got_privsep_recv_traversed_commits(struct got_commit_o
 				memcpy(&qid->id, &ids[i], sizeof(ids[i]));
 				STAILQ_INSERT_TAIL(commit_ids, qid, entry);
 
-				/* The last commit may contain a change. */
-				if (i == icommits->ncommits - 1) {
-					*changed_commit_id =
-					    got_object_id_dup(&qid->id);
-					if (*changed_commit_id == NULL) {
-						err = got_error_from_errno(
-						    "got_object_id_dup");
-						break;
-					}
-				}
 			}
 			break;
 		case GOT_IMSG_COMMIT:
-			if (*changed_commit_id == NULL) {
-				err = got_error(GOT_ERR_PRIVSEP_MSG);
-				break;
-			}
 			err = get_commit_from_imsg(changed_commit, &imsg,
 			    datalen, ibuf);
 			break;