commit 494e2b9b6acdb92382e65d0d9c990aaf8d3cbb22
from: Kyle Ackerman <kackerman0102@gmail.com>
via: Stefan Sperling <stsp@stsp.name>
date: Wed Mar 27 09:07:00 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 - c237d8914d9e7b85e50ad13b46083cdc61a78977
commit + 494e2b9b6acdb92382e65d0d9c990aaf8d3cbb22
blob - d517f36f2ccbda4c48348016a20e6d6e87fbd1a4
blob + 88d154e5954311dd1b59e15fad4c18b614032565
--- 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 - c956fa4a7551d6ffe10a3bd5888209dde0a148b8
blob + a17fad35a4e3a657892518b204167bccb6b24289
--- lib/object_open_privsep.c
+++ lib/object_open_privsep.c
@@ -24,6 +24,7 @@
 
 #include <errno.h>
 #include <imsg.h>
+#include <stddef.h>
 #include <stdio.h>
 #include <stdint.h>
 #include <stdlib.h>
@@ -1214,7 +1215,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);
@@ -1248,7 +1249,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;
 
@@ -1258,13 +1259,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 - 6ce1034017ac7d4f91415a7a95a62f755ea3e17d
blob + af5c95d6c8b067c2c0f4749f234e23c9124bfdff
--- 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;