Download raw body.
gotd: remove "sendfd" pledge promise from repo_write
On Fri, Oct 28, 2022 at 12:08:19PM +0200, Stefan Sperling wrote:
> 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?
ok. one extra tab below
>
> 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)
here
> - 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)
>
--
Tracey Emery
gotd: remove "sendfd" pledge promise from repo_write