"GOT", but the "O" is a cute, smiling pufferfish. Index | Thread | Search

From:
Stefan Sperling <stsp@stsp.name>
Subject:
fix non-portable use of pipe(2)
To:
gameoftrees@openbsd.org
Date:
Thu, 17 Jun 2021 14:30:42 +0200

Download raw body.

Thread
The 'got fetch' command relies on bi-directional communication over
a pipe.

POSIX doesn't require bi-directional communication, and while there
isn't a problem on OpenBSD this is a breaking issue for the Linux
portable version: On Linux, 'got fetch' doesn't work over SSH.
It always errors out with "bad file descriptor".

This patch shuffles things around such that got-fetch-pack will use two
distinct file descriptors for reading and writing Git protocol messages.
It makes clone over SSH succeed on Linux and makes on practical difference
on OpenBSD apart from using up an additional file descriptor.

Note that this patch won't apply to the portable version as-is.
The portable version is lagging behind and would need a slightly
different patch until it catches up with the latest changes.
I think we should commit this to 'main' and let the linux version catch up.

I suspect that the linux version is lagging behind in part because of this
particular issue. So I've decided to try to give it a small push :)

Another small fix in here is that we now check POLLHUP before POLLERR
because Linux sometimes sets both of these bits when the other end closes
the pipe. This avoids spurious errors during 'got clone/fetch' on Linux.

ok?

Note to reviewers: I am repurposing the GOT_IMSG_FETCH_OUTFD imsg and am
replacing its current use with the more suitably named GOT_IMSG_FETCH_PACKFD.
This was done on purpose. There is no problem with changing imsg IDs since
all the binaries are supposed to be rebuilt together.

diff 1d0f405485b02cc4480ea188879e4122e0ea32bd /home/stsp/src/got
blob - 097226da2568c25ff532d61bf72b41de086e3af9
file + got/got.c
--- got/got.c
+++ got/got.c
@@ -1466,7 +1466,7 @@ cmd_clone(int argc, char *argv[])
 	struct got_pathlist_head refs, symrefs, wanted_branches, wanted_refs;
 	struct got_pathlist_entry *pe;
 	struct got_object_id *pack_hash = NULL;
-	int ch, fetchfd = -1, fetchstatus;
+	int ch, fetchfd_in = -1, fetchfd_out = -1, fetchstatus;
 	pid_t fetchpid = -1;
 	struct got_fetch_progress_arg fpa;
 	char *git_url = NULL;
@@ -1610,8 +1610,8 @@ cmd_clone(int argc, char *argv[])
 		printf("Connecting to %s%s%s\n", host,
 		    port ? ":" : "", port ? port : "");
 
-	error = got_fetch_connect(&fetchpid, &fetchfd, proto, host, port,
-	    server_path, verbosity);
+	error = got_fetch_connect(&fetchpid, &fetchfd_in, &fetchfd_out,
+	    proto, host, port, server_path, verbosity);
 	if (error)
 		goto done;
 
@@ -1644,8 +1644,8 @@ cmd_clone(int argc, char *argv[])
 	error = got_fetch_pack(&pack_hash, &refs, &symrefs,
 	    GOT_FETCH_DEFAULT_REMOTE_NAME, mirror_references,
 	    fetch_all_branches, &wanted_branches, &wanted_refs,
-	    list_refs_only, verbosity, fetchfd, repo,
-	    fetch_progress, &fpa);
+	    list_refs_only, verbosity, fetchfd_in, fetchfd_out,
+	    repo, fetch_progress, &fpa);
 	if (error)
 		goto done;
 
@@ -1798,8 +1798,11 @@ done:
 		if (waitpid(fetchpid, &fetchstatus, 0) == -1 && error == NULL)
 			error = got_error_from_errno("waitpid");
 	}
-	if (fetchfd != -1 && close(fetchfd) == -1 && error == NULL)
+	if (fetchfd_in != -1 && close(fetchfd_in) == -1 && error == NULL)
 		error = got_error_from_errno("close");
+	if (fetchfd_out != fetchfd_in &&
+	    fetchfd_out != -1 && close(fetchfd_out) == -1 && error == NULL)
+		error = got_error_from_errno("close");
 	if (repo) {
 		const struct got_error *close_err = got_repo_close(repo);
 		if (error == NULL)
@@ -2132,7 +2135,7 @@ cmd_fetch(int argc, char *argv[])
 	struct got_pathlist_head refs, symrefs, wanted_branches, wanted_refs;
 	struct got_pathlist_entry *pe;
 	struct got_object_id *pack_hash = NULL;
-	int i, ch, fetchfd = -1, fetchstatus;
+	int i, ch, fetchfd_in = -1, fetchfd_out = -1, fetchstatus;
 	pid_t fetchpid = -1;
 	struct got_fetch_progress_arg fpa;
 	int verbosity = 0, fetch_all_branches = 0, list_refs_only = 0;
@@ -2340,8 +2343,8 @@ cmd_fetch(int argc, char *argv[])
 		printf("Connecting to \"%s\" %s%s%s\n", remote->name, host,
 		    port ? ":" : "", port ? port : "");
 
-	error = got_fetch_connect(&fetchpid, &fetchfd, proto, host, port,
-	    server_path, verbosity);
+	error = got_fetch_connect(&fetchpid, &fetchfd_in, &fetchfd_out,
+	    proto, host, port, server_path, verbosity);
 	if (error)
 		goto done;
 
@@ -2355,8 +2358,8 @@ cmd_fetch(int argc, char *argv[])
 	memset(&fpa.config_info, 0, sizeof(fpa.config_info));
 	error = got_fetch_pack(&pack_hash, &refs, &symrefs, remote->name,
 	    remote->mirror_references, fetch_all_branches, &wanted_branches,
-	    &wanted_refs, list_refs_only, verbosity, fetchfd, repo,
-	    fetch_progress, &fpa);
+	    &wanted_refs, list_refs_only, verbosity, fetchfd_in, fetchfd_out,
+	    repo, fetch_progress, &fpa);
 	if (error)
 		goto done;
 
@@ -2516,8 +2519,11 @@ done:
 		if (waitpid(fetchpid, &fetchstatus, 0) == -1 && error == NULL)
 			error = got_error_from_errno("waitpid");
 	}
-	if (fetchfd != -1 && close(fetchfd) == -1 && error == NULL)
+	if (fetchfd_in != -1 && close(fetchfd_in) == -1 && error == NULL)
 		error = got_error_from_errno("close");
+	if (fetchfd_in != fetchfd_out &&
+	    fetchfd_out != -1 && close(fetchfd_out) == -1 && error == NULL)
+		error = got_error_from_errno("close");
 	if (repo) {
 		const struct got_error *close_err = got_repo_close(repo);
 		if (error == NULL)
blob - ec4d2dc496a45f2a0804737eeb32ad4991f4c8ba
file + include/got_fetch.h
--- include/got_fetch.h
+++ include/got_fetch.h
@@ -44,14 +44,16 @@ const struct got_error *got_fetch_parse_uri(char **, c
  * of -v options passed to ssh(1). If the level is -1 ssh(1) will be run
  * with the -q option.
  *
- * If successful return an open file descriptor for the connection which can
- * be passed to other functions below, and must be disposed of with close(2).
+ * If successful return two open in/out file descriptors for the connection
+ * which can be passed to other functions below, and must be disposed of
+ * with close(2). In cases where one file descriptor is sufficient to
+ * support a read/write connection, the two file descriptors will be equal.
  *
  * If an ssh(1) process was started return its PID as well, in which case
  * the caller should eventually send SIGTERM to the procress and wait for
  * the process to exit with waitpid(2). Otherwise, return PID -1.
  */
-const struct got_error *got_fetch_connect(pid_t *, int *, const char *,
+const struct got_error *got_fetch_connect(pid_t *, int *, int *, const char *,
     const char *, const char *, const char *, int);
 
 /* A callback function which gets invoked with progress information to print. */
@@ -67,4 +69,4 @@ typedef const struct got_error *(*got_fetch_progress_c
 const struct got_error *got_fetch_pack(struct got_object_id **,
 	struct got_pathlist_head *, struct got_pathlist_head *, const char *,
 	int, int, struct got_pathlist_head *, struct got_pathlist_head *,
-	int, int, int, struct got_repository *, got_fetch_progress_cb, void *);
+	int, int, int, int, struct got_repository *, got_fetch_progress_cb, void *);
blob - 0c3e36643b33aa891a6ae0ca06eac2c739842c4c
file + lib/fetch.c
--- lib/fetch.c
+++ lib/fetch.c
@@ -88,17 +88,19 @@ hassuffix(char *base, char *suf)
 }
 
 static const struct got_error *
-dial_ssh(pid_t *fetchpid, int *fetchfd, const char *host, const char *port,
-    const char *path, const char *direction, int verbosity)
+dial_ssh(pid_t *fetchpid, int *fetchfd_in, int *fetchfd_out,
+    const char *host, const char *port, const char *path,
+    const char *direction, int verbosity)
 {
 	const struct got_error *error = NULL;
-	int pid, pfd[2];
+	int pid, pfd_in[2], pfd_out[2];
 	char cmd[64];
 	char *argv[11];
 	int i = 0, j;
 
 	*fetchpid = -1;
-	*fetchfd = -1;
+	*fetchfd_in = -1;
+	*fetchfd_out = -1;
 
 	argv[i++] = GOT_FETCH_PATH_SSH;
 	if (port != NULL) {
@@ -118,22 +120,32 @@ dial_ssh(pid_t *fetchpid, int *fetchfd, const char *ho
 	argv[i++] = (char *)path;
 	argv[i++] = NULL;
 
-	if (pipe(pfd) == -1)
+	if (pipe(pfd_in) == -1)
 		return got_error_from_errno("pipe");
+	if (pipe(pfd_out) == -1)
+ 		return got_error_from_errno("pipe");
 
 	pid = fork();
 	if (pid == -1) {
 		error = got_error_from_errno("fork");
-		close(pfd[0]);
-		close(pfd[1]);
+		close(pfd_in[0]);
+		close(pfd_in[1]);
+		close(pfd_out[0]);
+		close(pfd_out[1]);
 		return error;
 	} else if (pid == 0) {
 		int n;
-		if (close(pfd[1]) == -1)
+		/*
+		 * The child writes its standard output to 'in' and reads
+		 * its standard input from 'out'.
+		 */
+		if (close(pfd_in[0]) == -1)
 			err(1, "close");
-		if (dup2(pfd[0], 0) == -1)
+		if (close(pfd_out[1]) == -1)
+			err(1, "close");
+		if (dup2(pfd_in[1], STDOUT_FILENO) == -1)
 			err(1, "dup2");
-		if (dup2(pfd[0], 1) == -1)
+		if (dup2(pfd_out[0], STDIN_FILENO) == -1)
 			err(1, "dup2");
 		n = snprintf(cmd, sizeof(cmd), "git-%s-pack", direction);
 		if (n < 0 || n >= ssizeof(cmd))
@@ -142,24 +154,29 @@ dial_ssh(pid_t *fetchpid, int *fetchfd, const char *ho
 			err(1, "execv");
 		abort(); /* not reached */
 	} else {
-		if (close(pfd[0]) == -1)
+		/* The parent reads from 'in' and writes to 'out'. */
+		if (close(pfd_in[1]) == -1)
 			return got_error_from_errno("close");
+		if (close(pfd_out[0]) == -1)
+			return got_error_from_errno("close");
 		*fetchpid = pid;
-		*fetchfd = pfd[1];
+		*fetchfd_in = pfd_in[0];
+		*fetchfd_out = pfd_out[1];
 		return NULL;
 	}
 }
 
 static const struct got_error *
-dial_git(int *fetchfd, const char *host, const char *port, const char *path,
-    const char *direction)
+dial_git(int *fetchfd_in, int *fetchfd_out, const char *host, const char *port,
+    const char *path, const char *direction)
 {
 	const struct got_error *err = NULL;
 	struct addrinfo hints, *servinfo, *p;
 	char *cmd = NULL;
 	int fd = -1, len, r, eaicode;
 
-	*fetchfd = -1;
+	*fetchfd_in = -1;
+	*fetchfd_out = -1;
 
 	if (port == NULL)
 		port = GOT_DEFAULT_GIT_PORT_STR;
@@ -202,25 +219,30 @@ done:
 	if (err) {
 		if (fd != -1)
 			close(fd);
-	} else
-		*fetchfd = fd;
+	} else {
+		*fetchfd_in = fd;
+		*fetchfd_out = fd;
+	}
 	return err;
 }
 
 const struct got_error *
-got_fetch_connect(pid_t *fetchpid, int *fetchfd, const char *proto,
-    const char *host, const char *port, const char *server_path, int verbosity)
+got_fetch_connect(pid_t *fetchpid, int *fetchfd_in, int *fetchfd_out,
+    const char *proto, const char *host, const char *port,
+    const char *server_path, int verbosity)
 {
 	const struct got_error *err = NULL;
 
 	*fetchpid = -1;
-	*fetchfd = -1;
+	*fetchfd_in = -1;
+	*fetchfd_out = -1;
 
 	if (strcmp(proto, "ssh") == 0 || strcmp(proto, "git+ssh") == 0)
-		err = dial_ssh(fetchpid, fetchfd, host, port, server_path,
-		    "upload", verbosity);
+		err = dial_ssh(fetchpid, fetchfd_in, fetchfd_out, host, port,
+		    server_path, "upload", verbosity);
 	else if (strcmp(proto, "git") == 0)
-		err = dial_git(fetchfd, host, port, server_path, "upload");
+		err = dial_git(fetchfd_in, fetchfd_out, host, port,
+		    server_path, "upload");
 	else if (strcmp(proto, "http") == 0 || strcmp(proto, "git+http") == 0)
 		err = got_error_path(proto, GOT_ERR_NOT_IMPL);
 	else
@@ -348,12 +370,13 @@ got_fetch_pack(struct got_object_id **pack_hash, struc
     int mirror_references, int fetch_all_branches,
     struct got_pathlist_head *wanted_branches,
     struct got_pathlist_head *wanted_refs, int list_refs_only, int verbosity,
-    int fetchfd, struct got_repository *repo,
+    int fetchfd_in, int fetchfd_out, struct got_repository *repo,
     got_fetch_progress_cb progress_cb, void *progress_arg)
 {
 	size_t i;
 	int imsg_fetchfds[2], imsg_idxfds[2];
-	int packfd = -1, npackfd = -1, idxfd = -1, nidxfd = -1, nfetchfd = -1;
+	int packfd = -1, npackfd = -1, idxfd = -1, nidxfd = -1;
+	int nfetchfd_in = -1, nfetchfd_out = -1;
 	int tmpfds[3];
 	int fetchstatus, idxstatus, done = 0;
 	const struct got_error *err;
@@ -550,22 +573,38 @@ got_fetch_pack(struct got_object_id **pack_hash, struc
 		goto done;
 	}
 	imsg_init(&fetchibuf, imsg_fetchfds[0]);
-	nfetchfd = dup(fetchfd);
-	if (nfetchfd == -1) {
+	nfetchfd_in = dup(fetchfd_in);
+	if (nfetchfd_in == -1) {
 		err = got_error_from_errno("dup");
 		goto done;
 	}
-	err = got_privsep_send_fetch_req(&fetchibuf, nfetchfd, &have_refs,
+	err = got_privsep_send_fetch_req(&fetchibuf, nfetchfd_in, &have_refs,
 	    fetch_all_branches, wanted_branches, wanted_refs,
 	    list_refs_only, verbosity);
 	if (err != NULL)
 		goto done;
-	nfetchfd = -1;
+	nfetchfd_in = -1;
+
+	nfetchfd_out = dup(fetchfd_out);
+	if (nfetchfd_out == -1) {
+		err = got_error_from_errno("dup");
+		goto done;
+	}
+	err = got_privsep_send_fetch_outfd(&fetchibuf, nfetchfd_out);
+	if (err != NULL)
+		goto done;
+	nfetchfd_out = -1;
+
 	npackfd = dup(packfd);
 	if (npackfd == -1) {
 		err = got_error_from_errno("dup");
 		goto done;
 	}
+	err = got_privsep_send_fetch_packfd(&fetchibuf, npackfd);
+	if (err != NULL)
+		goto done;
+	npackfd = -1;
+
 	err = got_privsep_send_fetch_outfd(&fetchibuf, npackfd);
 	if (err != NULL)
 		goto done;
@@ -806,8 +845,10 @@ done:
 		err = got_error_from_errno2("unlink", tmppackpath);
 	if (tmpidxpath && unlink(tmpidxpath) == -1 && err == NULL)
 		err = got_error_from_errno2("unlink", tmpidxpath);
-	if (nfetchfd != -1 && close(nfetchfd) == -1 && err == NULL)
+	if (nfetchfd_in != -1 && close(nfetchfd_in) == -1 && err == NULL)
 		err = got_error_from_errno("close");
+	if (nfetchfd_out != -1 && close(nfetchfd_out) == -1 && err == NULL)
+		err = got_error_from_errno("close");
 	if (npackfd != -1 && close(npackfd) == -1 && err == NULL)
 		err = got_error_from_errno("close");
 	if (packfd != -1 && close(packfd) == -1 && err == NULL)
blob - dc1687fd0232a98260fd1d1f91425364de21521b
file + lib/got_lib_privsep.h
--- lib/got_lib_privsep.h
+++ lib/got_lib_privsep.h
@@ -117,6 +117,7 @@ enum got_imsg_type {
 	GOT_IMSG_FETCH_WANTED_BRANCH,
 	GOT_IMSG_FETCH_WANTED_REF,
 	GOT_IMSG_FETCH_OUTFD,
+	GOT_IMSG_FETCH_PACKFD,
 	GOT_IMSG_FETCH_SYMREFS,
 	GOT_IMSG_FETCH_REF,
 	GOT_IMSG_FETCH_SERVER_PROGRESS,
@@ -453,6 +454,7 @@ const struct got_error *got_privsep_send_fetch_req(str
     struct got_pathlist_head *, int, struct got_pathlist_head *,
     struct got_pathlist_head *, int, int);
 const struct got_error *got_privsep_send_fetch_outfd(struct imsgbuf *, int);
+const struct got_error *got_privsep_send_fetch_packfd(struct imsgbuf *, int);
 const struct got_error *got_privsep_recv_fetch_progress(int *,
     struct got_object_id **, char **, struct got_pathlist_head *, char **,
     off_t *, uint8_t *, struct imsgbuf *);
blob - 61891c554c130e0e597d44f689d9d93fec69b64d
file + lib/privsep.c
--- lib/privsep.c
+++ lib/privsep.c
@@ -80,10 +80,10 @@ poll_fd(int fd, int events, int timeout)
 		return got_error_from_errno("ppoll");
 	if (n == 0)
 		return got_error(GOT_ERR_TIMEOUT);
-	if (pfd[0].revents & (POLLERR | POLLNVAL))
-		return got_error_from_errno("poll error");
 	if (pfd[0].revents & (events | POLLHUP))
 		return NULL;
+	if (pfd[0].revents & (POLLERR | POLLNVAL))
+		return got_error_from_errno("poll error");
 
 	return got_error(GOT_ERR_INTERRUPT);
 }
@@ -698,6 +698,12 @@ got_privsep_send_fetch_outfd(struct imsgbuf *ibuf, int
 }
 
 const struct got_error *
+got_privsep_send_fetch_packfd(struct imsgbuf *ibuf, int fd)
+{
+	return send_fd(ibuf, GOT_IMSG_FETCH_PACKFD, fd);
+}
+
+const struct got_error *
 got_privsep_recv_fetch_progress(int *done, struct got_object_id **id,
     char **refname, struct got_pathlist_head *symrefs, char **server_progress,
     off_t *packfile_size, uint8_t *pack_sha1, struct imsgbuf *ibuf)
blob - dd739faa1ee3882e18ce87a0815acc3e0cc8e7c4
file + libexec/got-fetch-pack/got-fetch-pack.c
--- libexec/got-fetch-pack/got-fetch-pack.c
+++ libexec/got-fetch-pack/got-fetch-pack.c
@@ -650,7 +650,7 @@ send_fetch_ref(struct imsgbuf *ibuf, struct got_object
 }
 
 static const struct got_error *
-fetch_pack(int fd, int packfd, uint8_t *pack_sha1,
+fetch_pack(int fd_in, int fd_out, int packfd, uint8_t *pack_sha1,
     struct got_pathlist_head *have_refs, int fetch_all_branches,
     struct got_pathlist_head *wanted_branches,
     struct got_pathlist_head *wanted_refs, int list_refs_only,
@@ -687,7 +687,7 @@ fetch_pack(int fd, int packfd, uint8_t *pack_sha1,
 		goto done;
 	}
 	while (1) {
-		err = readpkt(&n, fd, buf, sizeof(buf));
+		err = readpkt(&n, fd_in, buf, sizeof(buf));
 		if (err)
 			goto done;
 		if (n == 0)
@@ -849,13 +849,13 @@ fetch_pack(int fd, int packfd, uint8_t *pack_sha1,
 			err = got_error(GOT_ERR_NO_SPACE);
 			goto done;
 		}
-		err = writepkt(fd, buf, n);
+		err = writepkt(fd_out, buf, n);
 		if (err)
 			goto done;
 		sent_my_capabilites = 1;
 		nwant++;
 	}
-	err = flushpkt(fd);
+	err = flushpkt(fd_out);
 	if (err)
 		goto done;
 
@@ -871,7 +871,7 @@ fetch_pack(int fd, int packfd, uint8_t *pack_sha1,
 			err = got_error(GOT_ERR_NO_SPACE);
 			goto done;
 		}
-		err = writepkt(fd, buf, n);
+		err = writepkt(fd_out, buf, n);
 		if (err)
 			goto done;
 		nhave++;
@@ -881,7 +881,7 @@ fetch_pack(int fd, int packfd, uint8_t *pack_sha1,
 		struct got_object_id common_id;
 
 		/* The server should ACK the object IDs we need. */
-		err = readpkt(&n, fd, buf, sizeof(buf));
+		err = readpkt(&n, fd_in, buf, sizeof(buf));
 		if (err)
 			goto done;
 		if (n >= 4 && strncmp(buf, "ERR ", 4) == 0) {
@@ -907,12 +907,12 @@ fetch_pack(int fd, int packfd, uint8_t *pack_sha1,
 	}
 
 	n = snprintf(buf, sizeof(buf), "done\n");
-	err = writepkt(fd, buf, n);
+	err = writepkt(fd_out, buf, n);
 	if (err)
 		goto done;
 
 	if (nhave == 0) {
-		err = readpkt(&n, fd, buf, sizeof(buf));
+		err = readpkt(&n, fd_in, buf, sizeof(buf));
 		if (err)
 			goto done;
 		if (n != 4 || strncmp(buf, "NAK\n", n) != 0) {
@@ -934,14 +934,14 @@ fetch_pack(int fd, int packfd, uint8_t *pack_sha1,
 		int datalen = -1;
 
 		if (have_sidebands) {
-			err = read_pkthdr(&datalen, fd);
+			err = read_pkthdr(&datalen, fd_in);
 			if (err)
 				goto done;
 			if (datalen <= 0)
 				break;
 
 			/* Read sideband channel ID (one byte). */
-			r = read(fd, buf, 1);
+			r = read(fd_in, buf, 1);
 			if (r == -1) {
 				err = got_error_from_errno("read");
 				goto done;
@@ -959,7 +959,7 @@ fetch_pack(int fd, int packfd, uint8_t *pack_sha1,
 			datalen--; /* sideband ID has been read */
 			if (buf[0] == GOT_SIDEBAND_PACKFILE_DATA) {
 				/* Read packfile data. */
-				err = readn(&r, fd, buf, datalen);
+				err = readn(&r, fd_in, buf, datalen);
 				if (err)
 					goto done;
 				if (r != datalen) {
@@ -968,7 +968,7 @@ fetch_pack(int fd, int packfd, uint8_t *pack_sha1,
 					goto done;
 				}
 			} else if (buf[0] == GOT_SIDEBAND_PROGRESS_INFO) {
-				err = readn(&r, fd, buf, datalen);
+				err = readn(&r, fd_in, buf, datalen);
 				if (err)
 					goto done;
 				if (r != datalen) {
@@ -981,7 +981,7 @@ fetch_pack(int fd, int packfd, uint8_t *pack_sha1,
 					goto done;
 				continue;
 			} else if (buf[0] == GOT_SIDEBAND_ERROR_INFO) {
-				err = readn(&r, fd, buf, datalen);
+				err = readn(&r, fd_in, buf, datalen);
 				if (err)
 					goto done;
 				if (r != datalen) {
@@ -992,7 +992,7 @@ fetch_pack(int fd, int packfd, uint8_t *pack_sha1,
 				err = fetch_error(buf, r);
 				goto done;
 			} else if (buf[0] == 'A') {
-				err = readn(&r, fd, buf, datalen);
+				err = readn(&r, fd_in, buf, datalen);
 				if (err)
 					goto done;
 				if (r != datalen) {
@@ -1017,7 +1017,7 @@ fetch_pack(int fd, int packfd, uint8_t *pack_sha1,
 			}
 		} else {
 			/* No sideband channel. Every byte is packfile data. */
-			err = readn(&r, fd, buf, sizeof buf);
+			err = readn(&r, fd_in, buf, sizeof buf);
 			if (err)
 				goto done;
 			if (r <= 0)
@@ -1125,7 +1125,7 @@ int
 main(int argc, char **argv)
 {
 	const struct got_error *err = NULL;
-	int fetchfd, packfd = -1, i;
+	int fetchfd_in, fetchfd_out, packfd = -1, i;
 	uint8_t pack_sha1[SHA1_DIGEST_LENGTH];
 	struct imsgbuf ibuf;
 	struct imsg imsg;
@@ -1174,7 +1174,7 @@ main(int argc, char **argv)
 		goto done;
 	}
 	memcpy(&fetch_req, imsg.data, sizeof(fetch_req));
-	fetchfd = imsg.fd;
+	fetchfd_in = imsg.fd;
 	imsg_free(&imsg);
 
 	if (fetch_req.verbosity > 0)
@@ -1327,10 +1327,27 @@ main(int argc, char **argv)
 		err = got_error(GOT_ERR_PRIVSEP_LEN);
 		goto done;
 	}
+	fetchfd_out = imsg.fd;
+
+	if ((err = got_privsep_recv_imsg(&imsg, &ibuf, 0)) != 0) {
+		if (err->code == GOT_ERR_PRIVSEP_PIPE)
+			err = NULL;
+		goto done;
+	}
+	if (imsg.hdr.type == GOT_IMSG_STOP)
+		goto done;
+	if (imsg.hdr.type != GOT_IMSG_FETCH_PACKFD) {
+		err = got_error(GOT_ERR_PRIVSEP_MSG);
+		goto done;
+	}
+	if (imsg.hdr.len - IMSG_HEADER_SIZE != 0) {
+		err = got_error(GOT_ERR_PRIVSEP_LEN);
+		goto done;
+	}
 	packfd = imsg.fd;
 
-	err = fetch_pack(fetchfd, packfd, pack_sha1, &have_refs,
-	    fetch_req.fetch_all_branches, &wanted_branches,
+	err = fetch_pack(fetchfd_in, fetchfd_out, packfd, pack_sha1,
+	    &have_refs, fetch_req.fetch_all_branches, &wanted_branches,
 	    &wanted_refs, fetch_req.list_refs_only, &ibuf);
 done:
 	TAILQ_FOREACH(pe, &have_refs, entry) {
@@ -1341,8 +1358,10 @@ done:
 	TAILQ_FOREACH(pe, &wanted_branches, entry)
 		free((char *)pe->path);
 	got_pathlist_free(&wanted_branches);
-	if (fetchfd != -1 && close(fetchfd) == -1 && err == NULL)
+	if (fetchfd_in != -1 && close(fetchfd_in) == -1 && err == NULL)
 		err = got_error_from_errno("close");
+	if (fetchfd_out != -1 && close(fetchfd_out) == -1 && err == NULL)
+		err = got_error_from_errno("close");
 	if (packfd != -1 && close(packfd) == -1 && err == NULL)
 		err = got_error_from_errno("close");
 	if (err != NULL)