commit 86769de8751a920ee4288ec91157066d6f098bfc from: Stefan Sperling date: Fri Oct 28 20:39:21 2022 UTC remove sendfd pledge promise from gotd repo_read process 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 tracey commit - 7fec5f4ad5baecf5bab9d872a925055270b2cf53 commit + 86769de8751a920ee4288ec91157066d6f098bfc blob - 0d1b70abc84b7ffd073e85521770b144812ebcfe blob + e3a20027002c07e25791ee436b0838e10613c439 --- gotd/gotd.c +++ gotd/gotd.c @@ -682,6 +682,7 @@ send_packfile(struct gotd_client *client) ipipe.client_id = client->id; + /* Send pack pipe end 0 to repo_read. */ if (gotd_imsg_compose_event(&client->repo_read->iev, GOTD_IMSG_PACKFILE_PIPE, PROC_GOTD, pipe[0], &ipipe, sizeof(ipipe)) == -1) { @@ -690,9 +691,9 @@ send_packfile(struct gotd_client *client) return err; } - if (gotd_imsg_compose_event(&client->repo_read->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"); return err; @@ -2034,7 +2035,7 @@ main(int argc, char **argv) break; case PROC_REPO_READ: #ifndef PROFILE - if (pledge("stdio rpath sendfd recvfd", NULL) == -1) + if (pledge("stdio rpath recvfd", NULL) == -1) err(1, "pledge"); #endif repo_read_main(title, pack_fds, temp_fds); blob - b1796ad22b340fe20d6d27d7e25500c103a0e37f blob + 3acfa77b04f537a7820959d7704e095c2998cde3 --- gotd/repo_read.c +++ gotd/repo_read.c @@ -68,7 +68,7 @@ struct repo_read_client { int fd; int delta_cache_fd; int report_progress; - int pack_pipe[2]; + int pack_pipe; struct gotd_object_id_array want_ids; struct gotd_object_id_array have_ids; }; @@ -91,8 +91,7 @@ add_client(struct repo_read_client *client, uint32_t c client->id = client_id; client->fd = fd; client->delta_cache_fd = -1; - client->pack_pipe[0] = -1; - client->pack_pipe[1] = -1; + client->pack_pipe = -1; slot = client_hash(client->id) % nitems(repo_read_clients); STAILQ_INSERT_HEAD(&repo_read_clients[slot], client, entry); } @@ -641,14 +640,10 @@ receive_pack_pipe(struct repo_read_client **client, st *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; } @@ -669,24 +664,10 @@ send_packfile(struct repo_read_client *client, struct got_ratelimit_init(&rl, 2, 0); - if (client->delta_cache_fd == -1 || - client->pack_pipe[0] == -1 || - client->pack_pipe[1] == -1) + if (client->delta_cache_fd == -1 || client->pack_pipe == -1) return got_error(GOT_ERR_PRIVSEP_NO_FD); imsg_init(&ibuf, client->fd); - - /* Send pack file pipe to gotsh(1). */ - if (imsg_compose(&ibuf, GOTD_IMSG_PACKFILE_PIPE, PROC_REPO_READ, - repo_read.pid, client->pack_pipe[1], NULL, 0) == -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; delta_cache = fdopen(client->delta_cache_fd, "w+"); if (delta_cache == NULL) { @@ -699,7 +680,7 @@ send_packfile(struct repo_read_client *client, struct pa.ibuf = &ibuf; pa.report_progress = client->report_progress; - err = got_pack_create(packsha1, client->pack_pipe[0], delta_cache, + err = got_pack_create(packsha1, client->pack_pipe, delta_cache, client->have_ids.ids, client->have_ids.nids, client->want_ids.ids, client->want_ids.nids, repo_read.repo, 0, 1, pack_progress, &pa, &rl, @@ -729,7 +710,7 @@ recv_disconnect(struct imsg *imsg) const struct got_error *err = NULL; struct gotd_imsg_disconnect idisconnect; size_t datalen; - int client_fd, delta_cache_fd, pipe[2]; + int client_fd, delta_cache_fd, pack_pipe; struct repo_read_client *client = NULL; uint64_t slot; @@ -751,17 +732,14 @@ recv_disconnect(struct imsg *imsg) free_object_ids(&client->want_ids); client_fd = client->fd; delta_cache_fd = client->delta_cache_fd; - pipe[0] = client->pack_pipe[0]; - pipe[1] = client->pack_pipe[1]; + pack_pipe = client->pack_pipe; free(client); if (close(client_fd) == -1) err = got_error_from_errno("close"); if (delta_cache_fd != -1 && close(delta_cache_fd) == -1 && err == NULL) return got_error_from_errno("close"); - if (pipe[0] != -1 && close(pipe[0]) == -1 && err == NULL) + if (pack_pipe != -1 && close(pack_pipe) == -1 && err == NULL) return got_error_from_errno("close"); - if (pipe[1] != -1 && close(pipe[1]) == -1 && err == NULL) - return got_error_from_errno("close"); return err; } @@ -830,7 +808,7 @@ repo_read_dispatch(int fd, short event, void *arg) repo_read.title, err->msg); break; } - if (client->pack_pipe[1] == -1) + if (client->pack_pipe == -1) break; err = send_packfile(client, &imsg, iev); if (err) blob - 07ef39056b1ae8681d10c81839b68eb97c9a1146 blob + 4f35feba02a39ad0317cd252dc1020e7194747e1 --- lib/serve.c +++ lib/serve.c @@ -777,20 +777,29 @@ recv_done(int *packfd, int outfd, struct imsgbuf *ibuf if (err) return err; - err = gotd_imsg_poll_recv(&imsg, ibuf, 0); - if (err) - return err; + while (*packfd == -1 && err == NULL) { + err = gotd_imsg_poll_recv(&imsg, ibuf, 0); + if (err) + break; - if (imsg.hdr.type == GOTD_IMSG_ERROR) - err = gotd_imsg_recv_error(NULL, &imsg); - else if (imsg.hdr.type != GOTD_IMSG_PACKFILE_PIPE) - err = got_error(GOT_ERR_PRIVSEP_MSG); - else if (imsg.fd == -1) - err = got_error(GOT_ERR_PRIVSEP_NO_FD); - if (err == NULL) - *packfd = imsg.fd; + switch (imsg.hdr.type) { + case GOTD_IMSG_ERROR: + err = gotd_imsg_recv_error(NULL, &imsg); + break; + case GOTD_IMSG_PACKFILE_PIPE: + if (imsg.fd != -1) + *packfd = imsg.fd; + else + err = got_error(GOT_ERR_PRIVSEP_NO_FD); + break; + default: + err = got_error(GOT_ERR_PRIVSEP_MSG); + break; + } - imsg_free(&imsg); + imsg_free(&imsg); + } + return err; }