commit 984dc60f4f97b89e1e9fe13698f2e234bffeef31
from: Stefan Sperling <stsp@stsp.name>
via: Thomas Adam <thomas@xteddy.org>
date: Sun Feb 16 13:44:27 2025 UTC

fix a bogus "received unexpected privsep message" error from gotsh

Ensure that gotsh receives its end of the pack file data pipe before
repo_read starts sending pack file creation progress messages.
Messages of type GOTD_IMSG_PACKFILE_PROGRESS would end up being
received in gotsh's recv_done() function, where such messages are
not expected.

commit - bb958056527fca453ea20b8f92f2cb03dd41a45b
commit + 984dc60f4f97b89e1e9fe13698f2e234bffeef31
blob - 6f748c4d6c02f54e8f2e13dc13e92789711bbb4e
blob + 2c84aed50abbc6aa5cbbd482f12a65d71cd780dd
--- gotd/session_read.c
+++ gotd/session_read.c
@@ -72,6 +72,7 @@ static struct gotd_session_read {
 	struct timeval request_timeout;
 	enum gotd_session_read_state state;
 	struct gotd_imsgev repo_child_iev;
+	int repo_child_packfd;
 } gotd_session;
 
 static struct gotd_session_client {
@@ -430,20 +431,24 @@ send_packfile(struct gotd_session_client *client)
 		return err;
 	}
 
-	/* Send pack pipe end 0 to repo child process. */
-	if (gotd_imsg_compose_event(&gotd_session.repo_child_iev,
-	    GOTD_IMSG_PACKFILE_PIPE, PROC_GOTD, pipe[0], NULL, 0) == -1) {
-		err = got_error_from_errno("imsg compose PACKFILE_PIPE");
-		close(pipe[1]);
-		return err;
-	}
-
-	/* 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_read process only
+	 * once we have confirmation that gotsh(1) has received its end.
+	 * It is important that we send a pipe end to gotsh(1) before the
+	 * repo_read process starts sending pack progress messages via imsg
+	 * to prevent spurious "unexpected privsep message" errors from gotsh.
+	 */
 	if (gotd_imsg_compose_event(&client->iev,
-	    GOTD_IMSG_PACKFILE_PIPE, PROC_GOTD, pipe[1], NULL, 0) == -1)
+	    GOTD_IMSG_PACKFILE_PIPE, PROC_GOTD, pipe[0], NULL, 0) == -1) {
 		err = got_error_from_errno("imsg compose PACKFILE_PIPE");
+		close(pipe[1]);
+		return err;
+	}
 
-	return err;
+	gotd_session.repo_child_packfd = pipe[1];
+	return NULL;
 }
 
 static void
@@ -597,6 +602,24 @@ session_dispatch_client(int fd, short events, void *ar
 			client->accept_flush_pkt = 1;
 			err = send_packfile(client);
 			break;
+		case GOTD_IMSG_PACKFILE_READY:
+			if (gotd_session.repo_child_packfd == -1) {
+				err = got_error(GOT_ERR_PRIVSEP_MSG);
+				break;
+			}
+			/*
+			 * gotsh(1) has received its send 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_GOTD,
+			    gotd_session.repo_child_packfd, NULL, 0) == -1) {
+				err = got_error_from_errno("imsg compose "
+				    "PACKFILE_PIPE");
+			} else
+				gotd_session.repo_child_packfd = -1;
+			break;
 		default:
 			log_debug("unexpected imsg %d", imsg.hdr.type);
 			err = got_error(GOT_ERR_PRIVSEP_MSG);
@@ -831,6 +854,7 @@ session_read_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");
@@ -907,5 +931,7 @@ session_read_shutdown(void)
 	got_repo_pack_fds_close(gotd_session.pack_fds);
 	got_repo_temp_fds_close(gotd_session.temp_fds);
 	free(gotd_session_client.username);
+	if (gotd_session.repo_child_packfd != -1)
+		close(gotd_session.repo_child_packfd);
 	exit(0);
 }
blob - 4a83c3046bc674b4bd18cbe3b46d1a82cd292035
blob + 225d06547c70a7eb1d80a957364916a8af27ffb4
--- lib/serve.c
+++ lib/serve.c
@@ -693,10 +693,16 @@ recv_done(int *packfd, int outfd, struct imsgbuf *ibuf
 			break;
 		case GOTD_IMSG_PACKFILE_PIPE:
 			fd = imsg_get_fd(&imsg);
-			if (fd != -1)
-				*packfd = fd;
-			else
+			if (fd == -1) {
 				err = got_error(GOT_ERR_PRIVSEP_NO_FD);
+				break;
+			}
+			*packfd = fd;
+			if (imsg_compose(ibuf, GOTD_IMSG_PACKFILE_READY,
+			    0, 0, -1, NULL, 0) == -1)
+				err = got_error_from_errno("imsg_compose "
+				    "PACKFILE_READY");
+			err = gotd_imsg_flush(ibuf);
 			break;
 		default:
 			err = got_error(GOT_ERR_PRIVSEP_MSG);