From: Sebastien Marie <semarie@online.fr>
Subject: Re: Packfile permissions
To: Alisdair MacLeod <got@alisdairmacleod.co.uk>, gameoftrees@openbsd.org
Date: Wed, 28 Oct 2020 17:58:04 +0100

On Wed, Oct 28, 2020 at 04:56:56PM +0100, Stefan Sperling wrote:
> On Wed, Oct 28, 2020 at 12:38:15PM +0000, Alisdair MacLeod wrote:
> > Here is a patch that I am currently running successfully on my server.
> 
> Thank you! Your patch is now in the main repository.

Per habit, I am not really fan of chmod(2) usage, specially when it is
as simple to use fchmod(2) (we opened the files for writing just
before).

The diff below should be equivalent in result. It uses fchmod(2) just
after opening the file.

Thanks.
-- 
Sebastien Marie

diff 1c25ba414b5c74d25a8745f25673abb74be2dd42 /home/semarie/repos/openbsd/got
blob - 0e60b2156e5a9ef85594f7ddccaba178ac16f667
file + lib/fetch.c
--- lib/fetch.c
+++ lib/fetch.c
@@ -491,37 +491,45 @@ got_fetch_pack(struct got_object_id **pack_hash, struc
 	} else {
 		if (asprintf(&path, "%s/%s/fetching.pack",
 		    repo_path, GOT_OBJECTS_PACK_DIR) == -1) {
 			err = got_error_from_errno("asprintf");
 			goto done;
 		}
 		err = got_opentemp_named_fd(&tmppackpath, &packfd, path);
 		free(path);
 		if (err)
 			goto done;
+		if (fchmod(packfd, GOT_DEFAULT_FILE_MODE) != 0) {
+			err = got_error_from_errno2("fchmod", tmppackpath);
+			goto done;
+		}
 	}
 	if (list_refs_only) {
 		idxfd = got_opentempfd();
 		if (idxfd == -1) {
 			err = got_error_from_errno("got_opentempfd");
 			goto done;
 		}
 	} else {
 		if (asprintf(&path, "%s/%s/fetching.idx",
 		    repo_path, GOT_OBJECTS_PACK_DIR) == -1) {
 			err = got_error_from_errno("asprintf");
 			goto done;
 		}
 		err = got_opentemp_named_fd(&tmpidxpath, &idxfd, path);
 		free(path);
 		if (err)
 			goto done;
+		if (fchmod(idxfd, GOT_DEFAULT_FILE_MODE) != 0) {
+			err = got_error_from_errno2("fchmod", tmpidxpath);
+			goto done;
+		}
 	}
 	nidxfd = dup(idxfd);
 	if (nidxfd == -1) {
 		err = got_error_from_errno("dup");
 		goto done;
 	}
 
 	for (i = 0; i < nitems(tmpfds); i++) {
 		tmpfds[i] = got_opentempfd();
 		if (tmpfds[i] == -1) {
@@ -792,29 +800,20 @@ got_fetch_pack(struct got_object_id **pack_hash, struc
 		goto done;
 	}
 	free(tmppackpath);
 	tmppackpath = NULL;
 	if (rename(tmpidxpath, idxpath) == -1) {
 		err = got_error_from_errno3("rename", tmpidxpath, idxpath);
 		goto done;
 	}
 	free(tmpidxpath);
 	tmpidxpath = NULL;
-
-	if (chmod(packpath, GOT_DEFAULT_FILE_MODE) != 0) {
-		err = got_error_from_errno2("chmod", packpath);
-		goto done;
-	}
-	if (chmod(idxpath, GOT_DEFAULT_FILE_MODE) != 0) {
-		err = got_error_from_errno2("chmod", idxpath);
-		goto done;
-	}
 
 done:
 	if (tmppackpath && unlink(tmppackpath) == -1 && err == NULL)
 		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)
 		err = got_error_from_errno("close");
 	if (npackfd != -1 && close(npackfd) == -1 && err == NULL)
 		err = got_error_from_errno("close");