"GOT", but the "O" is a cute, smiling sun Index | Thread

From:
Stefan Sperling <stsp@stsp.name>
Subject:
gotd: remove "sendfd" pledge promise from repo_write
To:
gameoftrees@openbsd.org
Date:
Fri, 28 Oct 2022 12:08:19 +0200

Download raw body.

Have the parent process send one end of the pipe directly to gotsh(1),
such that repo_write can run without "sendfd".
Combining "sendfd" and "recvfd" in the same process is frowned upon.

ok?
 
diff 417258479785447bb8b2d6106b1220cd833e4591 e228960be4ba7555768fabb33fb29a89beee42cf
commit - 417258479785447bb8b2d6106b1220cd833e4591
commit + e228960be4ba7555768fabb33fb29a89beee42cf
blob - 4e94e74827b4a318903802c2324b5b653b29e5bd
blob + 0d1b70abc84b7ffd073e85521770b144812ebcfe
--- gotd/gotd.c
+++ gotd/gotd.c
@@ -720,6 +720,7 @@ recv_packfile(struct gotd_client *client)
 	memset(&ipipe, 0, sizeof(ipipe));
 	ipipe.client_id = client->id;
 
+	/* Send pack pipe end 0 to repo_write. */
 	if (gotd_imsg_compose_event(&client->repo_write->iev,
 	    GOTD_IMSG_PACKFILE_PIPE, PROC_GOTD, pipe[0],
 	        &ipipe, sizeof(ipipe)) == -1) {
@@ -729,9 +730,9 @@ recv_packfile(struct gotd_client *client)
 	}
 	pipe[0] = -1;
 
-	if (gotd_imsg_compose_event(&client->repo_write->iev,
-	    GOTD_IMSG_PACKFILE_PIPE, PROC_GOTD, pipe[1],
-	        &ipipe, sizeof(ipipe)) == -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_GOTD, pipe[1], NULL, 0) == -1)
 		err = got_error_from_errno("imsg compose PACKFILE_PIPE");
 	pipe[1] = -1;
 
@@ -2041,7 +2042,7 @@ main(int argc, char **argv)
 		exit(0);
 	case PROC_REPO_WRITE:
 #ifndef PROFILE
-		if (pledge("stdio rpath sendfd recvfd", NULL) == -1)
+		if (pledge("stdio rpath recvfd", NULL) == -1)
 			err(1, "pledge");
 #endif
 		repo_write_main(title, pack_fds, temp_fds);
blob - fa09c1ca57d4bdb4c0dec036592b4d2967d28018
blob + 72ae9a0a2f15bd70bd8d6d112727e39f7c311690
--- gotd/repo_write.c
+++ gotd/repo_write.c
@@ -82,7 +82,7 @@ struct repo_write_client {
 	STAILQ_ENTRY(repo_write_client)	 entry;
 	uint32_t			 id;
 	int				 fd;
-	int				 pack_pipe[2];
+	int				 pack_pipe;
 	struct got_pack			 pack;
 	uint8_t				 pack_sha1[SHA1_DIGEST_LENGTH];
 	int				 packidx_fd;
@@ -107,8 +107,7 @@ add_client(struct repo_write_client *client, uint32_t 
 
 	client->id = client_id;
 	client->fd = fd;
-	client->pack_pipe[0] = -1;
-	client->pack_pipe[1] = -1;
+	client->pack_pipe = -1;
 	client->packidx_fd = -1;
 	STAILQ_INIT(&client->ref_updates);
 	client->nref_updates = 0;
@@ -929,8 +928,7 @@ recv_packfile(struct repo_write_client **client, struc
 	if (*client == NULL || STAILQ_EMPTY(&(*client)->ref_updates))
 		return got_error(GOT_ERR_CLIENT_ID);
 
-	if ((*client)->pack_pipe[0] == -1 ||
-	    (*client)->pack_pipe[1] == -1 ||
+	if ((*client)->pack_pipe == -1 ||
 	    (*client)->packidx_fd == -1)
 		return got_error(GOT_ERR_PRIVSEP_NO_FD);
 
@@ -969,22 +967,13 @@ recv_packfile(struct repo_write_client **client, struc
 		tempfiles[i] = f;
 	}
 
-	/* Send pack file pipe to gotsh(1). */
-	if (imsg_compose(&ibuf, GOTD_IMSG_RECV_PACKFILE, PROC_REPO_WRITE,
-	    repo_write.pid, (*client)->pack_pipe[1], NULL, 0) == -1) {
-		(*client)->pack_pipe[1] = -1;
-		err = got_error_from_errno("imsg_compose ACK");
-		if (err)	
-			goto done;
-	}
-	(*client)->pack_pipe[1] = -1;
 	err = gotd_imsg_flush(&ibuf);
 	if (err)
 		goto done;
 
 	log_debug("receiving pack data");
 	unpack_err = recv_packdata(&pack_filesize, (*client)->pack_sha1,
-	    (*client)->pack_pipe[0], pack->fd);
+	    (*client)->pack_pipe, pack->fd);
 	if (ireq.report_status) {
 		err = report_pack_status(*client, unpack_err);
 		if (err) {
@@ -1023,9 +1012,9 @@ done:
 	if (lseek((*client)->packidx_fd, 0L, SEEK_SET) == -1)
 		err = got_error_from_errno("lseek");
 done:
-	if (close((*client)->pack_pipe[0]) == -1 && err == NULL)
+	if (close((*client)->pack_pipe) == -1 && err == NULL)
 		err = got_error_from_errno("close");
-	(*client)->pack_pipe[0] = -1;
+	(*client)->pack_pipe = -1;
 	for (i = 0; i < nitems(repo_tempfiles); i++) {
 		struct repo_tempfile *t = &repo_tempfiles[i];
 		if (t->idx != -1)
@@ -1197,7 +1186,7 @@ recv_disconnect(struct imsg *imsg)
 	const struct got_error *err = NULL;
 	struct gotd_imsg_disconnect idisconnect;
 	size_t datalen;
-	int client_fd = -1, pipe0 = -1, pipe1 = - 1, idxfd = -1;
+	int client_fd = -1, pack_pipe = -1, idxfd = -1;
 	struct repo_write_client *client = NULL;
 	uint64_t slot;
 
@@ -1224,16 +1213,13 @@ recv_disconnect(struct imsg *imsg)
 	}
 	err = got_pack_close(&client->pack);
 	client_fd = client->fd;
-	pipe0 = client->pack_pipe[0];
-	pipe1 = client->pack_pipe[1];
+	pack_pipe = client->pack_pipe;
 	idxfd = client->packidx_fd;
 	free(client);
 	if (client_fd != -1 && close(client_fd) == -1)
 		err = got_error_from_errno("close");
-	if (pipe0 != -1 && close(pipe0) == -1 && err == NULL)
+	if (pack_pipe != -1 && close(pack_pipe) == -1 && err == NULL)
 		err = got_error_from_errno("close");
-	if (pipe1 != -1 && close(pipe1) == -1 && err == NULL)
-		err = got_error_from_errno("close");
 	if (idxfd != -1 && close(idxfd) == -1 && err == NULL)
 		err = got_error_from_errno("close");
 	return err;
@@ -1259,14 +1245,10 @@ receive_pack_pipe(struct repo_write_client **client, s
 	*client = find_client(ireq.client_id);
 	if (*client == NULL)
 		return got_error(GOT_ERR_CLIENT_ID);
-	if ((*client)->pack_pipe[1] != -1)
+	if ((*client)->pack_pipe != -1)
 		return got_error(GOT_ERR_PRIVSEP_MSG);
 
-	if ((*client)->pack_pipe[0] == -1)
-		(*client)->pack_pipe[0] = imsg->fd;
-	else
-		(*client)->pack_pipe[1] = imsg->fd;
-
+	(*client)->pack_pipe = imsg->fd;
 	return NULL;
 }
 
blob - 87e9f9ab41e1449492b4a1ca2485a8fba48bfbee
blob + 07ef39056b1ae8681d10c81839b68eb97c9a1146
--- lib/serve.c
+++ lib/serve.c
@@ -1381,7 +1381,7 @@ serve_write(int infd, int outfd, int gotd_sock, const 
 		case GOTD_IMSG_ERROR:
 			err = gotd_imsg_recv_error(NULL, &imsg);
 			goto done;
-		case GOTD_IMSG_RECV_PACKFILE:
+		case GOTD_IMSG_PACKFILE_PIPE:
 			err = recv_packfile(&imsg, infd);
 			if (err) {
 				if (err->code != GOT_ERR_EOF)