"GOT", but the "O" is a cute, smiling pufferfish. Index | Thread | Search

From:
Stefan Sperling <stsp@stsp.name>
Subject:
fix double-free in gotadmin pack
To:
gameoftrees@openbsd.org
Date:
Tue, 24 Feb 2026 10:43:04 +0100

Download raw body.

Thread
Stopping gotadmin pack with Ctrl-C during the commit coloring phase
can trigger a double free if coloring is offloaded to got-read-pack:

$ gotadmin pack -a -r /git/ports.git/
1844 commits colored^Cgotadmin(86432) in free(): double free 0x6d15cc5cf80

Fix below. ok?

 fix a double-free on error in got_pack_paint_commits()
 
 Before heading out on error clear the qid pointer if we have not yet
 removed qid from the list. Otherwise the error path will free the qid
 while it is still on the list.
 
 Also garbage collect the qid0 pointer which has been unused for some time.
 
M  lib/pack_create_privsep.c  |  20+  16-

1 file changed, 20 insertions(+), 16 deletions(-)

commit - d07cf78033793607c02ab0e8bcb66de1f4a02a6f
commit + 18603fd0426083fc6d8d8398e420d22fdc5a2333
blob - 311c7e65273b2764eabc6160b3675b91e1165e9f
blob + bd9cc726a8d3d9d2ecd8381f3cedb0feb27ef0c9
--- lib/pack_create_privsep.c
+++ lib/pack_create_privsep.c
@@ -211,7 +211,6 @@ struct recv_painted_commit_arg {
 	struct got_ratelimit *rl;
 	got_cancel_cb cancel_cb;
 	void *cancel_arg;
-	struct got_object_qid **qid0;
 };
 
 static const struct got_error *
@@ -276,8 +275,7 @@ recv_painted_commit(void *arg, struct got_object_id *i
 }
 
 static const struct got_error *
-paint_packed_commits(struct got_object_qid **qid0,
-    struct got_pack *pack, intptr_t color,
+paint_packed_commits(struct got_pack *pack, intptr_t color,
     int *ncolored, int *nqueued, int *nskip, struct got_object_id_queue *ids,
     struct got_object_idset *keep, struct got_object_idset *drop,
     struct got_object_idset *skip, struct got_repository *repo,
@@ -312,7 +310,6 @@ paint_packed_commits(struct got_object_qid **qid0,
 	arg.rl = rl;
 	arg.cancel_cb = cancel_cb;
 	arg.cancel_arg = cancel_arg;
-	arg.qid0 = qid0;
 	err = got_privsep_recv_painted_commits(&next_ids,
 	    recv_painted_commit, &arg, pack->privsep_child->ibuf);
 	if (err)
@@ -428,34 +425,41 @@ got_pack_paint_commits(int *ncolored, struct got_objec
 			if (idx != -1) {
 				err = got_privsep_init_commit_painting(
 				    pack->privsep_child->ibuf);
-				if (err)
+				if (err) {
+					qid = NULL;
 					goto done;
+				}
 				err = send_idset(pack->privsep_child->ibuf,
 				    keep);
-				if (err)
+				if (err) {
+					qid = NULL;
 					goto done;
+				}
 				err = send_idset(pack->privsep_child->ibuf,
 				    drop);
-				if (err)
+				if (err) {
+					qid = NULL;
 					goto done;
+				}
 				err = send_idset(pack->privsep_child->ibuf,
 				    skip);
-				if (err)
+				if (err) {
+					qid = NULL;
 					goto done;
-				err = paint_packed_commits(&qid, pack,
+				}
+				err = paint_packed_commits(pack,
 				    color, ncolored, &nqueued, &nskip,
 				    ids, keep, drop, skip, repo,
 				    progress_cb, progress_arg, rl,
 				    cancel_cb, cancel_arg);
-				if (qid) {
-					STAILQ_REMOVE(ids, qid,
-					    got_object_qid, entry);
-					nqueued--;
-					got_object_qid_free(qid);
+				if (err) {
 					qid = NULL;
-				}
-				if (err)
 					goto done;
+				}
+				STAILQ_REMOVE(ids, qid, got_object_qid, entry);
+				nqueued--;
+				got_object_qid_free(qid);
+				qid = NULL;
 				continue;
 			}
 		}