From: Tracey Emery Subject: Re: gotd: remove "sendfd" pledge promise from repo_read To: gameoftrees@openbsd.org Date: Fri, 28 Oct 2022 07:42:21 -0600 On Fri, Oct 28, 2022 at 03:36:57PM +0200, Stefan Sperling wrote: > This patch requires the corresponding patch for repo_write to be > applied first. > ok. same extra tab below > diff 1b564763ebbf2659ed1af8474ba471640384301f c5119b16ae24d36f28af86bcbd2221a56af86a61 > commit - 1b564763ebbf2659ed1af8474ba471640384301f > commit + c5119b16ae24d36f28af86bcbd2221a56af86a61 > 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,25 +664,11 @@ 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) same here > - 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) { > err = got_error_from_errno("fdopen"); > @@ -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; > } > > -- Tracey Emery