git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH 4/4] Use git_mkstemp_mode and xmkstemp_mode in odb_mkstemp, not chmod later.

From
Matthieu Moy <matthieu.moy@imag.fr>
Date
Feb 19, 2010, 16:33 UTC
Message-ID
<1266597207-32036-5-git-send-email-Matthieu.Moy@imag.fr>
In-Reply-To
<1266597207-32036-1-git-send-email-Matthieu.Moy@imag.fr>

We used to create 0600 files, and then use chmod to set the group and other permission bits to the umask. This usually has the same effect as a normal file creation with a umask.

But in the presence of ACLs, the group permission plays the role of the ACL mask: the "g" bits of newly created files are chosen according to default ACL mask of the directory, not according to the umask, and doing a chmod() on these "g" bits affect the ACL's mask instead of actual group permission.

In other words, creating files with 0600 and then doing a chmod to the umask creates files which are unreadable by users allowed in the default ACL. To create the files without breaking ACLs, we let the umask do it's job at the file's creation time, and get rid of the later chmod.

Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
---
 builtin-pack-objects.c |   18 ++----------------
 t/t1304-default-acl.sh |    2 +-
 wrapper.c              |   10 +++++++---
 3 files changed, 10 insertions(+), 20 deletions(-)
diff --git a/builtin-pack-objects.c b/builtin-pack-objects.c
index e1d3adf..539e75d 100644
--- a/builtin-pack-objects.c
+++ b/builtin-pack-objects.c
@@ -464,9 +464,6 @@ static int write_one(struct sha1file *f,
 	return 1;
 }
 
-/* forward declaration for write_pack_file */
-static int adjust_perm(const char *path, mode_t mode);
-
 static void write_pack_file(void)
 {
 	uint32_t i = 0, j;
@@ -523,21 +520,17 @@ static void write_pack_file(void)
 		}
 
 		if (!pack_to_stdout) {
-			mode_t mode = umask(0);
 			struct stat st;
 			const char *idx_tmp_name;
 			char tmpname[PATH_MAX];
 
-			umask(mode);
-			mode = 0444 & ~mode;
-
 			idx_tmp_name = write_idx_file(NULL, written_list,
 						      nr_written, sha1);
 
 			snprintf(tmpname, sizeof(tmpname), "%s-%s.pack",
 				 base_name, sha1_to_hex(sha1));
 			free_pack_by_name(tmpname);
-			if (adjust_perm(pack_tmp_name, mode))
+			if (adjust_shared_perm(pack_tmp_name))
 				die_errno("unable to make temporary pack file readable");
 			if (rename(pack_tmp_name, tmpname))
 				die_errno("unable to rename temporary pack file");
@@ -565,7 +558,7 @@ static void write_pack_file(void)
 
 			snprintf(tmpname, sizeof(tmpname), "%s-%s.idx",
 				 base_name, sha1_to_hex(sha1));
-			if (adjust_perm(idx_tmp_name, mode))
+			if (adjust_shared_perm(idx_tmp_name))
 				die_errno("unable to make temporary index file readable");
 			if (rename(idx_tmp_name, tmpname))
 				die_errno("unable to rename temporary index file");
@@ -2125,13 +2118,6 @@ static void get_object_list(int ac, const char **av)
 		loosen_unused_packed_objects(&revs);
 }
 
-static int adjust_perm(const char *path, mode_t mode)
-{
-	if (chmod(path, mode))
-		return -1;
-	return adjust_shared_perm(path);
-}
-
 int cmd_pack_objects(int argc, const char **argv, const char *prefix)
 {
 	int use_internal_rev_list = 0;
diff --git a/t/t1304-default-acl.sh b/t/t1304-default-acl.sh
index 4ee44a1..b8e9c34 100755
--- a/t/t1304-default-acl.sh
+++ b/t/t1304-default-acl.sh
@@ -27,7 +27,7 @@ modebits () {
 	ls -l "$1" | sed -e 's|^\(..........\).*|\1|'
 }
 
-test_expect_failure 'git gc does not break ACLs with restrictive umask' '
+test_expect_success 'git gc does not break ACLs with restrictive umask' '
 	umask 077 &&
 	git gc &&
 	actual=$(modebits .git/objects/pack/*.pack) &&
diff --git a/wrapper.c b/wrapper.c
index 673762f..b2c784d 100644
--- a/wrapper.c
+++ b/wrapper.c
@@ -277,10 +277,14 @@ int git_inflate(z_streamp strm, int flush)
 int odb_mkstemp(char *template, size_t limit, const char *pattern)
 {
 	int fd;
-
+	/*
+	 * we let the umask do it's job, don't try to be more
+	 * restrictive.
+	 */
+	int mode = 0666;
 	snprintf(template, limit, "%s/%s",
 		 get_object_directory(), pattern);
-	fd = mkstemp(template);
+	fd = git_mkstemp_mode(template, mode);
 	if (0 <= fd)
 		return fd;
 
@@ -289,7 +293,7 @@ int odb_mkstemp(char *template, size_t limit, const char *pattern)
 	snprintf(template, limit, "%s/%s",
 		 get_object_directory(), pattern);
 	safe_create_leading_directories(template);
-	return xmkstemp(template);
+	return xmkstemp_mode(template, mode);
 }
 
 int odb_pack_keep(char *name, size_t namesz, unsigned char *sha1)
-- 
1.7.0.rc2.4.gc602c4
Previous: Matthieu MoyNext: Junio C Hamano
Message 5 of 18 in “Allow Git repositories to be shared using POSIX ACLs”
  1. 0/4 Allow Git repositories to be shared using POSIX ACLsMatthieu Moy, Feb 19, 2010
  2. 1/4 Add a testcase for ACL with restrictive umask.Matthieu Moy, Feb 19, 2010
  3. 2/4 Move gitmkstemps to path.cMatthieu Moy, Feb 19, 2010
  4. 3/4 git_mkstemp_mode, xmkstemp_mode: variants of gitmkstemps with mode argument.Matthieu Moy, Feb 19, 2010
  5. 4/4 Use git_mkstemp_mode and xmkstemp_mode in odb_mkstemp, not chmod later.Matthieu Moy, Feb 19, 2010
  6. Junio C HamanoFeb 20, 2010
  7. Matthieu MoyFeb 22, 2010
  8. Junio C HamanoFeb 22, 2010
  9. Junio C HamanoFeb 22, 2010
  10. Matthieu MoyFeb 22, 2010
  11. Junio C HamanoFeb 19, 2010
  12. 0/6 Allow Git repositories to be shared using POSIX ACLsMatthieu Moy, Feb 22, 2010
  13. 1/6 Add a testcase for ACL with restrictive umask.Matthieu Moy, Feb 22, 2010
  14. 2/6 Move gitmkstemps to path.cMatthieu Moy, Feb 22, 2010
  15. 3/6 git_mkstemp_mode, xmkstemp_mode: variants of gitmkstemps with mode argument.Matthieu Moy, Feb 22, 2010
  16. 4/6 Use git_mkstemp_mode and xmkstemp_mode in odb_mkstemp, not chmod later.Matthieu Moy, Feb 22, 2010
  17. 5/6 git_mkstemps_mode: don't set errno to EINVAL on exit.Matthieu Moy, Feb 22, 2010
  18. 6/6 Use git_mkstemp_mode instead of plain mkstemp to create object filesMatthieu Moy, Feb 22, 2010

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.