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

From:
Stefan Sperling <stsp@stsp.name>
Subject:
open tempfiles in got_repo_open()
To:
gameoftrees@openbsd.org
Date:
Sat, 12 Mar 2022 13:52:51 +0100

Download raw body.

Thread
  • Stefan Sperling:

    open tempfiles in got_repo_open()

Programs like gotwebd(8) and perhaps a future gotd(8) server need
to manage resources such as open files upfront, rather than having
code in lib/ open files as needed.

The patch below is a first step towards fixing one such issue for gotwebd.
The issue here is that gotwebd runs in a chroot environment that may lack
a /tmp directory, which breaks got_opentemp() at run-time.

ok?

-----------------------------------------------
commit bd9b38c590a5a0c4e6b47264b667fc5e3e6c4b93 (pack-opentemp)
from: Stefan Sperling <stsp@stsp.name>
date: Sat Mar 12 12:43:24 2022 UTC
 
 open temporary files needed for delta application in got_repo_open()
 
 This prepares for callers of got_repo_open() that cannot afford to
 open files in /tmp, such as gotwebd. In a follow-up change, we could
 ask such callers to pass in the required amount of open temporary files.
 
 One consequence is that got_repo_open() now requires the "cpath" pledge
 promise. Add the "cpath" promise to affected callers and remove it once
 the repository has been opened.
 
diff f95923a581dde77a840de5e9f56060e84118b413 9eff67c819a8cc0b0c4db3579999877cde09b335
blob - 17d637cabb59cb76c220d33c371c8c2a25da94cf
blob + fbda8cac06e23e0d902b849b50b650436a9dcc11
--- got/got.c
+++ got/got.c
@@ -5810,15 +5810,9 @@ cmd_ref(int argc, char *argv[])
 		got_path_strip_trailing_slashes(refname);
 
 #ifndef PROFILE
-	if (do_list) {
-		if (pledge("stdio rpath wpath flock proc exec sendfd unveil",
-		    NULL) == -1)
-			err(1, "pledge");
-	} else {
-		if (pledge("stdio rpath wpath cpath fattr flock proc exec "
-		    "sendfd unveil", NULL) == -1)
-			err(1, "pledge");
-	}
+	if (pledge("stdio rpath wpath cpath fattr flock proc exec "
+	    "sendfd unveil", NULL) == -1)
+		err(1, "pledge");
 #endif
 	cwd = getcwd(NULL, 0);
 	if (cwd == NULL) {
@@ -5852,6 +5846,15 @@ cmd_ref(int argc, char *argv[])
 	if (error != NULL)
 		goto done;
 
+#ifndef PROFILE
+	if (do_list) {
+		/* Remove "cpath" promise. */
+		if (pledge("stdio rpath wpath flock proc exec sendfd unveil",
+		    NULL) == -1)
+			err(1, "pledge");
+	}
+#endif
+
 	error = apply_unveil(got_repo_get_path(repo), do_list,
 	    worktree ? got_worktree_get_root_path(worktree) : NULL);
 	if (error)
@@ -6178,48 +6181,51 @@ cmd_branch(int argc, char *argv[])
 		usage_branch();
 
 #ifndef PROFILE
+	if (pledge("stdio rpath wpath cpath fattr flock proc exec "
+	    "sendfd unveil", NULL) == -1)
+		err(1, "pledge");
+#endif
+	cwd = getcwd(NULL, 0);
+	if (cwd == NULL) {
+		error = got_error_from_errno("getcwd");
+		goto done;
+	}
+
+	if (repo_path == NULL) {
+		error = got_worktree_open(&worktree, cwd);
+		if (error && error->code != GOT_ERR_NOT_WORKTREE)
+			goto done;
+		else
+			error = NULL;
+		if (worktree) {
+			repo_path =
+			    strdup(got_worktree_get_repo_path(worktree));
+			if (repo_path == NULL)
+				error = got_error_from_errno("strdup");
+			if (error)
+				goto done;
+		} else {
+			repo_path = strdup(cwd);
+			if (repo_path == NULL) {
+				error = got_error_from_errno("strdup");
+				goto done;
+			}
+		}
+	}
+
+	error = got_repo_open(&repo, repo_path, NULL);
+	if (error != NULL)
+		goto done;
+
+#ifndef PROFILE
 	if (do_list || do_show) {
+		/* Remove "cpath" promise. */
 		if (pledge("stdio rpath wpath flock proc exec sendfd unveil",
 		    NULL) == -1)
 			err(1, "pledge");
-	} else {
-		if (pledge("stdio rpath wpath cpath fattr flock proc exec "
-		    "sendfd unveil", NULL) == -1)
-			err(1, "pledge");
 	}
 #endif
-	cwd = getcwd(NULL, 0);
-	if (cwd == NULL) {
-		error = got_error_from_errno("getcwd");
-		goto done;
-	}
 
-	if (repo_path == NULL) {
-		error = got_worktree_open(&worktree, cwd);
-		if (error && error->code != GOT_ERR_NOT_WORKTREE)
-			goto done;
-		else
-			error = NULL;
-		if (worktree) {
-			repo_path =
-			    strdup(got_worktree_get_repo_path(worktree));
-			if (repo_path == NULL)
-				error = got_error_from_errno("strdup");
-			if (error)
-				goto done;
-		} else {
-			repo_path = strdup(cwd);
-			if (repo_path == NULL) {
-				error = got_error_from_errno("strdup");
-				goto done;
-			}
-		}
-	}
-
-	error = got_repo_open(&repo, repo_path, NULL);
-	if (error != NULL)
-		goto done;
-
 	error = apply_unveil(got_repo_get_path(repo), do_list,
 	    worktree ? got_worktree_get_root_path(worktree) : NULL);
 	if (error)
@@ -6726,15 +6732,9 @@ cmd_tag(int argc, char *argv[])
 	tag_name = argv[0];
 
 #ifndef PROFILE
-	if (do_list) {
-		if (pledge("stdio rpath wpath flock proc exec sendfd unveil",
-		    NULL) == -1)
-			err(1, "pledge");
-	} else {
-		if (pledge("stdio rpath wpath cpath fattr flock proc exec "
-		    "sendfd unveil", NULL) == -1)
-			err(1, "pledge");
-	}
+	if (pledge("stdio rpath wpath cpath fattr flock proc exec "
+	    "sendfd unveil", NULL) == -1)
+		err(1, "pledge");
 #endif
 	cwd = getcwd(NULL, 0);
 	if (cwd == NULL) {
@@ -6768,6 +6768,12 @@ cmd_tag(int argc, char *argv[])
 		error = got_repo_open(&repo, repo_path, NULL);
 		if (error != NULL)
 			goto done;
+#ifndef PROFILE
+		/* Remove "cpath" promise. */
+		if (pledge("stdio rpath wpath flock proc exec sendfd unveil",
+		    NULL) == -1)
+			err(1, "pledge");
+#endif
 		error = apply_unveil(got_repo_get_path(repo), 1, NULL);
 		if (error)
 			goto done;
@@ -11960,7 +11966,7 @@ cmd_info(int argc, char *argv[])
 	argv += optind;
 
 #ifndef PROFILE
-	if (pledge("stdio rpath wpath flock proc exec sendfd unveil",
+	if (pledge("stdio rpath wpath cpath flock proc exec sendfd unveil",
 	    NULL) == -1)
 		err(1, "pledge");
 #endif
@@ -11977,6 +11983,12 @@ cmd_info(int argc, char *argv[])
 		goto done;
 	}
 
+#ifndef PROFILE
+	/* Remove "cpath" promise. */
+	if (pledge("stdio rpath wpath flock proc exec sendfd unveil",
+	    NULL) == -1)
+		err(1, "pledge");
+#endif
 	error = apply_unveil(NULL, 0, got_worktree_get_root_path(worktree));
 	if (error)
 		goto done;
blob - bd7343dcca754f398aaba30df74ecd27b90d2ee7
blob + 0f9c2137d554258de7f66a0bcdb8c4a945cea69a
--- gotadmin/gotadmin.c
+++ gotadmin/gotadmin.c
@@ -295,7 +295,7 @@ cmd_info(int argc, char *argv[])
 	argv += optind;
 
 #ifndef PROFILE
-	if (pledge("stdio rpath wpath flock proc exec sendfd unveil",
+	if (pledge("stdio rpath wpath cpath flock proc exec sendfd unveil",
 	    NULL) == -1)
 		err(1, "pledge");
 #endif
@@ -307,7 +307,12 @@ cmd_info(int argc, char *argv[])
 	error = got_repo_open(&repo, repo_path, NULL);
 	if (error)
 		goto done;
-
+#ifndef PROFILE
+	/* Remove "cpath" promise. */
+	if (pledge("stdio rpath wpath flock proc exec sendfd unveil",
+	    NULL) == -1)
+		err(1, "pledge");
+#endif
 	error = apply_unveil(got_repo_get_path_git_dir(repo), 1);
 	if (error)
 		goto done;
@@ -899,14 +904,19 @@ cmd_listpack(int argc, char *argv[])
 		return got_error_from_errno2("realpath", argv[0]);
 
 #ifndef PROFILE
-	if (pledge("stdio rpath wpath flock proc exec sendfd unveil",
+	if (pledge("stdio rpath wpath cpath flock proc exec sendfd unveil",
 	    NULL) == -1)
 		err(1, "pledge");
 #endif
 	error = got_repo_open(&repo, packfile_path, NULL);
 	if (error)
 		goto done;
-
+#ifndef PROFILE
+	/* Remove "cpath" promise. */
+	if (pledge("stdio rpath wpath flock proc exec sendfd unveil",
+	    NULL) == -1)
+		err(1, "pledge");
+#endif
 	error = apply_unveil(got_repo_get_path_git_dir(repo), 1);
 	if (error)
 		goto done;
blob - e8fb373e287ee80486d50ed07964d9d39924308d
blob + 61f0c6f6be1ff738e75c196b532adeec0d286f6d
--- lib/got_lib_pack.h
+++ lib/got_lib_pack.h
@@ -21,6 +21,8 @@ struct got_pack {
 	uint8_t *map;
 	size_t filesize;
 	struct got_privsep_child *privsep_child;
+	int basefd;
+	int accumfd;
 	int child_has_tempfiles;
 	int child_has_delta_outfd;
 	struct got_delta_cache *delta_cache;
blob - 8218b2c4c146e832c63f84d4a9f6a43f8b3283f8
blob + aed88977b89e2cbc403071fd2d8fd81478ad6570
--- lib/object.c
+++ lib/object.c
@@ -172,7 +172,7 @@ static const struct got_error *
 pack_child_send_tempfiles(struct imsgbuf *ibuf, struct got_pack *pack)
 {
 	const struct got_error *err;
-	int basefd, accumfd;
+	int basefd = -1, accumfd = -1;
 
 	/* 
 	 * For performance reasons, the child will keep reusing the
@@ -183,23 +183,29 @@ pack_child_send_tempfiles(struct imsgbuf *ibuf, struct
 	if (pack->child_has_tempfiles)
 		return NULL;
 
-	basefd = got_opentempfd();
+	basefd = dup(pack->basefd);
 	if (basefd == -1)
-		return got_error_from_errno("got_opentempfd");
+		return got_error_from_errno("dup");
 
+	accumfd = dup(pack->accumfd);
+	if (accumfd == -1) {
+		err = got_error_from_errno("dup");
+		goto done;
+	}
+
 	err = got_privsep_send_tmpfd(ibuf, basefd);
 	if (err)
-		return err;
+		goto done;
 
-	accumfd = got_opentempfd();
-	if (accumfd == -1)
-		return got_error_from_errno("got_opentempfd");
-
 	err = got_privsep_send_tmpfd(ibuf, accumfd);
-	if (err)
-		return err;
-
-	pack->child_has_tempfiles = 1;
+done:
+	if (err) {
+		if (basefd != -1)
+			close(basefd);
+		if (accumfd != -1)
+			close(accumfd);
+	} else
+		pack->child_has_tempfiles = 1;
 	return NULL;
 }
 
blob - 4aa9467c86b6e0a5f571584951e24abda6d8f5f5
blob + efe03f932898f14a489edd1540f83f72b472413f
--- lib/pack.c
+++ lib/pack.c
@@ -751,6 +751,11 @@ got_pack_close(struct got_pack *pack)
 		pack->delta_cache = NULL;
 	}
 
+	/*
+	 * Leave accumfd and basefd alone. They are managed by the
+	 * repository layer and can be reused.
+	 */
+
 	return err;
 }
 
blob - d4175545865a20ba9a3cff43736298aaae5e8c37
blob + 487889ce6084b085c5bd704c6a005d435c1992de
--- lib/repository.c
+++ lib/repository.c
@@ -50,6 +50,7 @@
 #include "got_path.h"
 #include "got_cancel.h"
 #include "got_object.h"
+#include "got_opentemp.h"
 
 #include "got_lib_delta.h"
 #include "got_lib_inflate.h"
@@ -701,6 +702,19 @@ got_repo_open(struct got_repository **repop, const cha
 	repo->pack_cache_size = GOT_PACK_CACHE_SIZE;
 	if (repo->pack_cache_size > rl.rlim_cur / 8)
 		repo->pack_cache_size = rl.rlim_cur / 8;
+	for (i = 0; i < nitems(repo->packs); i++) {
+		if (i < repo->pack_cache_size) {
+			repo->packs[i].basefd = got_opentempfd();
+			if (repo->packs[i].basefd == -1)
+				return got_error_from_errno("got_opentempfd");
+			repo->packs[i].accumfd = got_opentempfd();
+			if (repo->packs[i].accumfd == -1)
+				return got_error_from_errno("got_opentempfd");
+		} else {
+			repo->packs[i].basefd = -1;
+			repo->packs[i].accumfd = -1;
+		}
+	}
 
 	repo_path = realpath(path, NULL);
 	if (repo_path == NULL) {
@@ -784,6 +798,16 @@ got_repo_close(struct got_repository *repo)
 		if (repo->packs[i].path_packfile == NULL)
 			break;
 		got_pack_close(&repo->packs[i]);
+		if (repo->packs[i].basefd != -1) {
+			if (close(repo->packs[i].basefd) == -1 && err == NULL)
+				err = got_error_from_errno("close");
+			repo->packs[i].basefd = -1;
+		}
+		if (repo->packs[i].accumfd != -1) {
+			if (close(repo->packs[i].accumfd) == -1 && err == NULL)
+				err = got_error_from_errno("close");
+			repo->packs[i].accumfd = -1;
+		}
 	}
 
 	free(repo->path);
@@ -1360,6 +1384,10 @@ got_repo_cache_pack(struct got_pack **packp, struct go
 		err = got_pack_close(&repo->packs[i - 1]);
 		if (err)
 			return err;
+		if (ftruncate(repo->packs[i - 1].basefd, 0L) == -1)
+			return got_error_from_errno("ftruncate");
+		if (ftruncate(repo->packs[i - 1].accumfd, 0L) == -1)
+			return got_error_from_errno("ftruncate");
 		memmove(&repo->packs[1], &repo->packs[0],
 		    sizeof(repo->packs) - sizeof(repo->packs[0]));
 		i = 0;