From: Stefan Sperling Subject: gotd: remove "sendfd" pledge promise from repo_write To: gameoftrees@openbsd.org Date: Fri, 28 Oct 2022 12:08:19 +0200 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)