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

From:
Stefan Sperling <stsp@stsp.name>
Subject:
Re: gotd: received unexpected privsep message
To:
Martijn van Duren <openbsd+got@list.imperialat.at>
Cc:
gameoftrees@openbsd.org
Date:
Mon, 2 Jun 2025 10:47:04 +0200

Download raw body.

Thread
On Mon, Jun 02, 2025 at 10:46:09AM +0200, Stefan Sperling wrote:
> I will send a follow-up shortly with a patch that can be applied to 0.110.


M  gotd/session_write.c  |  47+  17-
M  lib/serve.c           |  11+   2-

2 files changed, 58 insertions(+), 19 deletions(-)

commit - 6f02d0a77cf43f47eebfc9af8ebcfcd0241e6729
commit + 9da14085861c77601c0807025d6b429e2f9f9762
blob - 1c759a341683fe39614e0c779362809c3d1610e2
blob + 069c62b60913e06646d131680cf603799a2febf6
--- gotd/session_write.c
+++ gotd/session_write.c
@@ -84,6 +84,7 @@ static struct gotd_session_write {
 	struct timeval request_timeout;
 	enum gotd_session_write_state state;
 	struct gotd_imsgev repo_child_iev;
+	int repo_child_packfd;
 } gotd_session;
 
 static struct gotd_session_client {
@@ -1095,9 +1096,6 @@ static const struct got_error *
 recv_packfile(struct gotd_session_client *client)
 {
 	const struct got_error *err = NULL;
-	struct gotd_imsg_recv_packfile ipack;
-	char *basepath = NULL, *pack_path = NULL, *idx_path = NULL;
-	int packfd = -1, idxfd = -1;
 	int pipe[2] = { -1, -1 };
 
 	if (client->packfile_path) {
@@ -1108,23 +1106,51 @@ recv_packfile(struct gotd_session_client *client)
 	if (socketpair(AF_UNIX, SOCK_STREAM, PF_UNSPEC, pipe) == -1)
 		return got_error_from_errno("socketpair");
 
-	/* Send pack pipe end 0 to repo child process. */
-	if (gotd_imsg_compose_event(&gotd_session.repo_child_iev,
-	    GOTD_IMSG_PACKFILE_PIPE, PROC_SESSION_WRITE, pipe[0],
-	        NULL, 0) == -1) {
+	/*
+	 * Send pack data pipe end 0 to gotsh(1) (expects just an fd, no data).
+	 *
+	 * We will forward the other pipe end to the repo_write process only
+	 * once we have confirmation that gotsh(1) has received its end.
+	 */
+	if (gotd_imsg_compose_event(&client->iev,
+	    GOTD_IMSG_PACKFILE_PIPE, PROC_GOTD, pipe[0], NULL, 0) == -1) {
 		err = got_error_from_errno("imsg compose PACKFILE_PIPE");
 		pipe[0] = -1;
 		goto done;
 	}
 	pipe[0] = -1;
 
-	/* Send pack pipe end 1 to gotsh(1) (expects just an fd, no data). */
-	if (gotd_imsg_compose_event(&client->iev,
-	    GOTD_IMSG_PACKFILE_PIPE, PROC_SESSION_WRITE, pipe[1],
-	    NULL, 0) == -1)
-		err = got_error_from_errno("imsg compose PACKFILE_PIPE");
+	gotd_session.repo_child_packfd = pipe[1];
 	pipe[1] = -1;
+done:
+	if (pipe[0] != -1 && close(pipe[0]) == -1 && err == NULL)
+		err = got_error_from_errno("close");
+	if (pipe[1] != -1 && close(pipe[1]) == -1 && err == NULL)
+		err = got_error_from_errno("close");
+	return err;
+}
 
+static const struct got_error *
+send_packfds_to_repo_child(void)
+{
+	const struct got_error *err = NULL;
+	struct gotd_session_client *client = &gotd_session_client;
+	char *basepath = NULL, *pack_path = NULL, *idx_path = NULL;
+	int packfd = -1, idxfd = -1;
+	struct gotd_imsg_recv_packfile ipack;
+
+	/*
+	 * gotsh(1) has received its end of the pack pipe.
+	 * Send pack pipe end 1 to repo child process.
+	 */
+	if (gotd_imsg_compose_event(
+	    &gotd_session.repo_child_iev,
+	    GOTD_IMSG_PACKFILE_PIPE, PROC_SESSION_WRITE,
+	    gotd_session.repo_child_packfd, NULL, 0) == -1)
+		return got_error_from_errno("imsg compose PACKFILE_PIPE");
+
+	gotd_session.repo_child_packfd = -1;
+
 	if (asprintf(&basepath, "%s/%s/receiving-from-uid-%d.pack",
 	    got_repo_get_path(gotd_session.repo), GOT_OBJECTS_PACK_DIR,
 	    client->euid) == -1) {
@@ -1177,13 +1203,8 @@ recv_packfile(struct gotd_session_client *client)
 		goto done;
 	}
 	packfd = -1;
-
 done:
 	free(basepath);
-	if (pipe[0] != -1 && close(pipe[0]) == -1 && err == NULL)
-		err = got_error_from_errno("close");
-	if (pipe[1] != -1 && close(pipe[1]) == -1 && err == NULL)
-		err = got_error_from_errno("close");
 	if (packfd != -1 && close(packfd) == -1 && err == NULL)
 		err = got_error_from_errno("close");
 	if (idxfd != -1 && close(idxfd) == -1 && err == NULL)
@@ -1343,6 +1364,14 @@ session_dispatch_client(int fd, short events, void *ar
 				break;
 			}
 			break;
+		case GOTD_IMSG_PACKFILE_READY:
+			if (gotd_session.state != GOTD_STATE_EXPECT_PACKFILE ||
+			    gotd_session.repo_child_packfd == -1) {
+				err = got_error(GOT_ERR_PRIVSEP_MSG);
+				break;
+			}
+			err = send_packfds_to_repo_child();
+			break;
 		default:
 			log_debug("unexpected imsg %d", imsg.hdr.type);
 			err = got_error(GOT_ERR_PRIVSEP_MSG);
@@ -1692,6 +1721,7 @@ session_write_main(const char *title, const char *repo
 	memcpy(&gotd_session.request_timeout, request_timeout,
 	    sizeof(gotd_session.request_timeout));
 	gotd_session.repo_cfg = repo_cfg;
+	gotd_session.repo_child_packfd = -1;
 
 	if (imsgbuf_init(&gotd_session.notifier_iev.ibuf, -1) == -1) {
 		err = got_error_from_errno("imsgbuf_init");
blob - bf8e3261ba0241ac634ea7756c6793870799a9ed
blob + b2eaac52554ef197cab3ee63518c4a9c0bf19009
--- lib/serve.c
+++ lib/serve.c
@@ -1069,7 +1069,7 @@ done:
 }
 
 static const struct got_error *
-recv_packfile(struct imsg *imsg, int infd)
+recv_packfile(struct imsg *imsg, struct imsgbuf *ibuf, int infd)
 {
 	const struct got_error *err = NULL;
 	size_t datalen;
@@ -1085,6 +1085,15 @@ recv_packfile(struct imsg *imsg, int infd)
 	if (packfd == -1)
 		return got_error(GOT_ERR_PRIVSEP_NO_FD);
 
+	if (imsg_compose(ibuf, GOTD_IMSG_PACKFILE_READY,
+	    0, 0, -1, NULL, 0) == -1) {
+		return got_error_from_errno("imsg_compose PACKFILE_READY");
+
+	}
+	err = gotd_imsg_flush(ibuf);
+	if (err)
+		return err;
+
 	while (!pack_done) {
 		ssize_t r = 0;
 
@@ -1329,7 +1338,7 @@ serve_write(int infd, int outfd, int gotd_sock, const 
 			err = gotd_imsg_recv_error(NULL, &imsg);
 			goto done;
 		case GOTD_IMSG_PACKFILE_PIPE:
-			err = recv_packfile(&imsg, infd);
+			err = recv_packfile(&imsg, &ibuf, infd);
 			if (err) {
 				if (err->code != GOT_ERR_EOF)
 					goto done;