Commit 41496140bd5c2fd2df772feec4b1eee2cbf0073a

Stefan Sperling 2019-02-21T15:17:36

prevent double-close(2) of file descriptors passed in imsg

diff --git a/lib/object.c b/lib/object.c
index 64f1a99..a630dcc 100644
--- a/lib/object.c
+++ b/lib/object.c
@@ -440,8 +440,6 @@ got_object_open(struct got_object **obj, struct got_repository *repo,
 	err = got_repo_cache_object(repo, id, *obj);
 done:
 	free(path);
-	if (fd != -1 && close(fd) != 0 && errno != EBADF && err == NULL)
-		err = got_error_from_errno();
 	return err;
 
 }
@@ -605,8 +603,6 @@ open_commit(struct got_commit_object **commit,
 		if (err)
 			return err;
 		err = read_commit_privsep(commit, fd, repo);
-		if (close(fd) != 0 && errno != EBADF && err == NULL)
-			err = got_error_from_errno();
 	}
 
 	if (err == NULL) {
@@ -785,8 +781,6 @@ open_tree(struct got_tree_object **tree, struct got_repository *repo,
 		if (err)
 			return err;
 		err = read_tree_privsep(tree, fd, repo);
-		if (close(fd) != 0 && errno != EBADF && err == NULL)
-			err = got_error_from_errno();
 	}
 
 	if (err == NULL) {
@@ -850,25 +844,22 @@ request_packed_blob(uint8_t **outbuf, size_t *size, size_t *hdrlen, int outfd,
 	err = got_privsep_send_blob_outfd(pack->privsep_child->ibuf,
 	    outfd_child);
 	if (err) {
-		close(outfd_child);
+		close(basefd);
+		close(accumfd);
 		return err;
 	}
+
 	err = got_privsep_send_tmpfd(pack->privsep_child->ibuf,
 	    basefd);
 	if (err) {
-		close(basefd);
 		close(accumfd);
-		close(outfd_child);
 		return err;
 	}
 
 	err = got_privsep_send_tmpfd(pack->privsep_child->ibuf,
 	    accumfd);
-	if (err) {
-		close(accumfd);
-		close(outfd_child);
+	if (err)
 		return err;
-	}
 
 	err = got_privsep_recv_blob(outbuf, size, hdrlen,
 	    pack->privsep_child->ibuf);
@@ -914,10 +905,8 @@ request_blob(uint8_t **outbuf, size_t *size, size_t *hdrlen, int outfd,
 		return err;
 
 	err = got_privsep_send_blob_outfd(ibuf, outfd_child);
-	if (err) {
-		close(outfd_child);
+	if (err)
 		return err;
-	}
 
 	err = got_privsep_recv_blob(outbuf, size, hdrlen, ibuf);
 	if (err)
@@ -1021,8 +1010,6 @@ open_blob(struct got_blob_object **blob, struct got_repository *repo,
 			goto done;
 		err = read_blob_privsep(&outbuf, &size, &hdrlen, outfd, infd,
 		    repo);
-		if (close(infd) != 0 && errno != EBADF && err == NULL)
-			err = got_error_from_errno();
 	}
 	if (err)
 		goto done;
@@ -1310,8 +1297,6 @@ open_tag(struct got_tag_object **tag, struct got_repository *repo,
 		if (err)
 			return err;
 		err = read_tag_privsep(tag, fd, repo);
-		if (close(fd) != 0 && errno != EBADF && err == NULL)
-			err = got_error_from_errno();
 	}
 
 	if (err == NULL) {
diff --git a/lib/privsep.c b/lib/privsep.c
index fd10467..a9a2ff8 100644
--- a/lib/privsep.c
+++ b/lib/privsep.c
@@ -234,6 +234,7 @@ const struct got_error *
 got_privsep_send_commit_req(struct imsgbuf *ibuf, int fd,
     struct got_object_id *id, int pack_idx)
 {
+	const struct got_error *err = NULL;
 	struct got_imsg_packed_object iobj, *iobjp;
 	size_t len;
 
@@ -248,8 +249,11 @@ got_privsep_send_commit_req(struct imsgbuf *ibuf, int fd,
 	}
 
 	if (imsg_compose(ibuf, GOT_IMSG_COMMIT_REQUEST, 0, 0, fd, iobjp, len)
-	    == -1)
-		return got_error_from_errno();
+	    == -1) {
+		err = got_error_from_errno();
+		close(fd);
+		return err;
+	}
 
 	return flush_imsg(ibuf);
 }
@@ -258,6 +262,7 @@ const struct got_error *
 got_privsep_send_tree_req(struct imsgbuf *ibuf, int fd,
     struct got_object_id *id, int pack_idx)
 {
+	const struct got_error *err = NULL;
 	struct got_imsg_packed_object iobj, *iobjp;
 	size_t len;
 
@@ -272,8 +277,11 @@ got_privsep_send_tree_req(struct imsgbuf *ibuf, int fd,
 	}
 
 	if (imsg_compose(ibuf, GOT_IMSG_TREE_REQUEST, 0, 0, fd, iobjp, len)
-	    == -1)
-		return got_error_from_errno();
+	    == -1) {
+		err = got_error_from_errno();
+		close(fd);
+		return err;
+	}
 
 	return flush_imsg(ibuf);
 }
@@ -306,6 +314,7 @@ const struct got_error *
 got_privsep_send_blob_req(struct imsgbuf *ibuf, int infd,
     struct got_object_id *id, int pack_idx)
 {
+	const struct got_error *err = NULL;
 	struct got_imsg_packed_object iobj, *iobjp;
 	size_t len;
 
@@ -320,8 +329,11 @@ got_privsep_send_blob_req(struct imsgbuf *ibuf, int infd,
 	}
 
 	if (imsg_compose(ibuf, GOT_IMSG_BLOB_REQUEST, 0, 0, infd, iobjp, len)
-	    == -1)
-		return got_error_from_errno();
+	    == -1) {
+		err = got_error_from_errno();
+		close(infd);
+		return err;
+	}
 
 	return flush_imsg(ibuf);
 }
@@ -329,9 +341,14 @@ got_privsep_send_blob_req(struct imsgbuf *ibuf, int infd,
 const struct got_error *
 got_privsep_send_blob_outfd(struct imsgbuf *ibuf, int outfd)
 {
+	const struct got_error *err = NULL;
+
 	if (imsg_compose(ibuf, GOT_IMSG_BLOB_OUTFD, 0, 0, outfd, NULL, 0)
-	    == -1)
-		return got_error_from_errno();
+	    == -1) {
+		err = got_error_from_errno();
+		close(outfd);
+		return err;
+	}
 
 	return flush_imsg(ibuf);
 }
@@ -339,9 +356,14 @@ got_privsep_send_blob_outfd(struct imsgbuf *ibuf, int outfd)
 const struct got_error *
 got_privsep_send_tmpfd(struct imsgbuf *ibuf, int fd)
 {
+	const struct got_error *err = NULL;
+
 	if (imsg_compose(ibuf, GOT_IMSG_TMPFD, 0, 0, fd, NULL, 0)
-	    == -1)
-		return got_error_from_errno();
+	    == -1) {
+		err = got_error_from_errno();
+		close(fd);
+		return err;
+	}
 
 	return flush_imsg(ibuf);
 }
@@ -1132,6 +1154,7 @@ const struct got_error *
 got_privsep_init_pack_child(struct imsgbuf *ibuf, struct got_pack *pack,
     struct got_packidx *packidx)
 {
+	const struct got_error *err = NULL;
 	struct got_imsg_packidx ipackidx;
 	struct got_imsg_pack ipack;
 	int fd;
@@ -1142,8 +1165,11 @@ got_privsep_init_pack_child(struct imsgbuf *ibuf, struct got_pack *pack,
 		return got_error_from_errno();
 
 	if (imsg_compose(ibuf, GOT_IMSG_PACKIDX, 0, 0, fd, &ipackidx,
-	    sizeof(ipackidx)) == -1)
-		return got_error_from_errno();
+	    sizeof(ipackidx)) == -1) {
+		err = got_error_from_errno();
+		close(fd);
+		return err;
+	}
 
 	if (strlcpy(ipack.path_packfile, pack->path_packfile,
 	    sizeof(ipack.path_packfile)) >= sizeof(ipack.path_packfile))
@@ -1155,8 +1181,11 @@ got_privsep_init_pack_child(struct imsgbuf *ibuf, struct got_pack *pack,
 		return got_error_from_errno();
 
 	if (imsg_compose(ibuf, GOT_IMSG_PACK, 0, 0, fd, &ipack, sizeof(ipack))
-	    == -1)
-		return got_error_from_errno();
+	    == -1) {
+		err = got_error_from_errno();
+		close(fd);
+		return err;
+	}
 
 	return flush_imsg(ibuf);
 }