commit d907758cbe632fc02ea32fa9363f1b427ed21c32 from: Stefan Sperling date: Mon Jun 02 08:50:59 2025 UTC fix bogus "unexpected privsep message" from gotsh during "got send" Ensure that gotsh receives its end of the pack file data pipe before repo_write starts sending pack file status messages. Messages of type GOTD_IMSG_PACKFILE_StATUS would end up being received in gotsh's serve_write() function too early. This race is similar to the one fixed for repo_read back in commit c2274a511a7415078e2628f969b8459f69432411 Reported by martijn@, who pin-pointed the problematic case in the code, thanks! commit - 277ed191b926ae1e6b7ca037f3304903aa162828 commit + d907758cbe632fc02ea32fa9363f1b427ed21c32 blob - 21d505ad565f7c05cb872d5c7eda294089cb44db blob + 2b2318d81f74d2c1bfb5dd384acc1b61b4d31c91 --- gotd/session_write.c +++ gotd/session_write.c @@ -91,6 +91,7 @@ static struct gotd_session_write { size_t num_notification_refs_received; struct got_pathlist_head *notification_refs_cur; struct gotd_notification_targets notification_targets; + int repo_child_packfd; } gotd_session; static struct gotd_session_client { @@ -1263,9 +1264,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) { @@ -1276,23 +1274,49 @@ 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, GOTD_PROC_SESSION_WRITE, pipe[0], - NULL, 0) == -1) { - err = got_error_from_errno("imsg compose PACKFILE_PIPE"); - goto done; - } - pipe[0] = -1; - - /* Send pack pipe end 1 to gotsh(1) (expects just an fd, no data). */ + /* + * 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, GOTD_PROC_SESSION_WRITE, pipe[1], - NULL, 0) == -1) { + GOTD_IMSG_PACKFILE_PIPE, GOTD_PROC_GOTD, pipe[0], NULL, 0) == -1) { err = got_error_from_errno("imsg compose PACKFILE_PIPE"); goto done; } + pipe[0] = -1; + + 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, GOTD_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, @@ -1344,13 +1368,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) @@ -1510,6 +1529,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); @@ -1986,6 +2013,7 @@ session_write_main(const char *title, const char *repo RB_INIT(&gotd_session.notification_refs); RB_INIT(&gotd_session.notification_ref_namespaces); STAILQ_INIT(&gotd_session.notification_targets); + gotd_session.repo_child_packfd = -1; if (imsgbuf_init(&gotd_session.notifier_iev.ibuf, -1) == -1) { err = got_error_from_errno("imsgbuf_init"); blob - a9c52c79856ada5a27ff4408e73766c864eb939c blob + e36084ae30ef3cd5c8813f82e04ba0c23a52a299 --- lib/serve.c +++ lib/serve.c @@ -1081,7 +1081,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; @@ -1096,6 +1096,14 @@ recv_packfile(struct imsg *imsg, int infd) packfd = imsg_get_fd(imsg); 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; @@ -1341,7 +1349,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;