From: Martijn van Duren Subject: Re: gotd: received unexpected privsep message To: gameoftrees@openbsd.org Cc: Stefan Sperling Date: Mon, 02 Jun 2025 17:24:50 +0200 Can confirm that this one fixes the issue for me. Thanks. On Mon, 2025-06-02 at 10:47 +0200, Stefan Sperling wrote: > On Mon, Jun 02, 2025 at 10:46:09AM +0200, Stefan Sperling wrote: > > I will send a follow-up shortly with a patch that can be applied to 0.110. > > > M gotd/session_write.c | 47+ 17- > M lib/serve.c | 11+ 2- > > 2 files changed, 58 insertions(+), 19 deletions(-) > > commit - 6f02d0a77cf43f47eebfc9af8ebcfcd0241e6729 > commit + 9da14085861c77601c0807025d6b429e2f9f9762 > blob - 1c759a341683fe39614e0c779362809c3d1610e2 > blob + 069c62b60913e06646d131680cf603799a2febf6 > --- gotd/session_write.c > +++ gotd/session_write.c > @@ -84,6 +84,7 @@ static struct gotd_session_write { > struct timeval request_timeout; > enum gotd_session_write_state state; > struct gotd_imsgev repo_child_iev; > + int repo_child_packfd; > } gotd_session; > > static struct gotd_session_client { > @@ -1095,9 +1096,6 @@ static const struct got_error * > recv_packfile(struct gotd_session_client *client) > { > const struct got_error *err = NULL; > - struct gotd_imsg_recv_packfile ipack; > - char *basepath = NULL, *pack_path = NULL, *idx_path = NULL; > - int packfd = -1, idxfd = -1; > int pipe[2] = { -1, -1 }; > > if (client->packfile_path) { > @@ -1108,23 +1106,51 @@ recv_packfile(struct gotd_session_client *client) > if (socketpair(AF_UNIX, SOCK_STREAM, PF_UNSPEC, pipe) == -1) > return got_error_from_errno("socketpair"); > > - /* Send pack pipe end 0 to repo child process. */ > - if (gotd_imsg_compose_event(&gotd_session.repo_child_iev, > - GOTD_IMSG_PACKFILE_PIPE, PROC_SESSION_WRITE, pipe[0], > - NULL, 0) == -1) { > + /* > + * Send pack data pipe end 0 to gotsh(1) (expects just an fd, no data). > + * > + * We will forward the other pipe end to the repo_write process only > + * once we have confirmation that gotsh(1) has received its end. > + */ > + if (gotd_imsg_compose_event(&client->iev, > + GOTD_IMSG_PACKFILE_PIPE, PROC_GOTD, pipe[0], NULL, 0) == -1) { > err = got_error_from_errno("imsg compose PACKFILE_PIPE"); > pipe[0] = -1; > goto done; > } > pipe[0] = -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_SESSION_WRITE, pipe[1], > - NULL, 0) == -1) > - err = got_error_from_errno("imsg compose PACKFILE_PIPE"); > + gotd_session.repo_child_packfd = pipe[1]; > pipe[1] = -1; > +done: > + if (pipe[0] != -1 && close(pipe[0]) == -1 && err == NULL) > + err = got_error_from_errno("close"); > + if (pipe[1] != -1 && close(pipe[1]) == -1 && err == NULL) > + err = got_error_from_errno("close"); > + return err; > +} > > +static const struct got_error * > +send_packfds_to_repo_child(void) > +{ > + const struct got_error *err = NULL; > + struct gotd_session_client *client = &gotd_session_client; > + char *basepath = NULL, *pack_path = NULL, *idx_path = NULL; > + int packfd = -1, idxfd = -1; > + struct gotd_imsg_recv_packfile ipack; > + > + /* > + * gotsh(1) has received its end of the pack pipe. > + * Send pack pipe end 1 to repo child process. > + */ > + if (gotd_imsg_compose_event( > + &gotd_session.repo_child_iev, > + GOTD_IMSG_PACKFILE_PIPE, PROC_SESSION_WRITE, > + gotd_session.repo_child_packfd, NULL, 0) == -1) > + return got_error_from_errno("imsg compose PACKFILE_PIPE"); > + > + gotd_session.repo_child_packfd = -1; > + > if (asprintf(&basepath, "%s/%s/receiving-from-uid-%d.pack", > got_repo_get_path(gotd_session.repo), GOT_OBJECTS_PACK_DIR, > client->euid) == -1) { > @@ -1177,13 +1203,8 @@ recv_packfile(struct gotd_session_client *client) > goto done; > } > packfd = -1; > - > done: > free(basepath); > - if (pipe[0] != -1 && close(pipe[0]) == -1 && err == NULL) > - err = got_error_from_errno("close"); > - if (pipe[1] != -1 && close(pipe[1]) == -1 && err == NULL) > - err = got_error_from_errno("close"); > if (packfd != -1 && close(packfd) == -1 && err == NULL) > err = got_error_from_errno("close"); > if (idxfd != -1 && close(idxfd) == -1 && err == NULL) > @@ -1343,6 +1364,14 @@ session_dispatch_client(int fd, short events, void *ar > break; > } > break; > + case GOTD_IMSG_PACKFILE_READY: > + if (gotd_session.state != GOTD_STATE_EXPECT_PACKFILE || > + gotd_session.repo_child_packfd == -1) { > + err = got_error(GOT_ERR_PRIVSEP_MSG); > + break; > + } > + err = send_packfds_to_repo_child(); > + break; > default: > log_debug("unexpected imsg %d", imsg.hdr.type); > err = got_error(GOT_ERR_PRIVSEP_MSG); > @@ -1692,6 +1721,7 @@ session_write_main(const char *title, const char *repo > memcpy(&gotd_session.request_timeout, request_timeout, > sizeof(gotd_session.request_timeout)); > gotd_session.repo_cfg = repo_cfg; > + gotd_session.repo_child_packfd = -1; > > if (imsgbuf_init(&gotd_session.notifier_iev.ibuf, -1) == -1) { > err = got_error_from_errno("imsgbuf_init"); > blob - bf8e3261ba0241ac634ea7756c6793870799a9ed > blob + b2eaac52554ef197cab3ee63518c4a9c0bf19009 > --- lib/serve.c > +++ lib/serve.c > @@ -1069,7 +1069,7 @@ done: > } > > static const struct got_error * > -recv_packfile(struct imsg *imsg, int infd) > +recv_packfile(struct imsg *imsg, struct imsgbuf *ibuf, int infd) > { > const struct got_error *err = NULL; > size_t datalen; > @@ -1085,6 +1085,15 @@ recv_packfile(struct imsg *imsg, int infd) > if (packfd == -1) > return got_error(GOT_ERR_PRIVSEP_NO_FD); > > + if (imsg_compose(ibuf, GOTD_IMSG_PACKFILE_READY, > + 0, 0, -1, NULL, 0) == -1) { > + return got_error_from_errno("imsg_compose PACKFILE_READY"); > + > + } > + err = gotd_imsg_flush(ibuf); > + if (err) > + return err; > + > while (!pack_done) { > ssize_t r = 0; > > @@ -1329,7 +1338,7 @@ serve_write(int infd, int outfd, int gotd_sock, const > err = gotd_imsg_recv_error(NULL, &imsg); > goto done; > case GOTD_IMSG_PACKFILE_PIPE: > - err = recv_packfile(&imsg, infd); > + err = recv_packfile(&imsg, &ibuf, infd); > if (err) { > if (err->code != GOT_ERR_EOF) > goto done;