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

[PATCH v2 1/2] sha1_file: open window into packfiles with CLOEXEC

From
larsxschneider@gmail.com <larsxschneider@gmail.com>
Date
Oct 24, 2016, 18:02 UTC
Message-ID
<20161024180300.52359-2-larsxschneider@gmail.com>
In-Reply-To
<20161024180300.52359-1-larsxschneider@gmail.com>
From: Lars Schneider <larsxschneider@gmail.com>

All processes that the Git main process spawns inherit the open file descriptors of the main process. These leaked file descriptors can cause problems.

Use the CLOEXEC flag similar to 05d1ed61 to fix the leaked file descriptors. Since `git_open_noatime` does not describe the function properly anymore rename it to `git_open`.

Signed-off-by: Lars Schneider <larsxschneider@gmail.com>
---
 builtin/pack-objects.c |  2 +-
 cache.h                |  2 +-
 pack-bitmap.c          |  2 +-
 sha1_file.c            | 26 ++++++++++++++++----------
 4 files changed, 19 insertions(+), 13 deletions(-)
diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index 1e7c2a9..0fd52bd 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -720,7 +720,7 @@ static off_t write_reused_pack(struct sha1file *f)
 	if (!is_pack_valid(reuse_packfile))
 		die("packfile is invalid: %s", reuse_packfile->pack_name);
 
-	fd = git_open_noatime(reuse_packfile->pack_name);
+	fd = git_open(reuse_packfile->pack_name);
 	if (fd < 0)
 		die_errno("unable to open packfile for reuse: %s",
 			  reuse_packfile->pack_name);
diff --git a/cache.h b/cache.h
index b7f34b4..419b7a0 100644
--- a/cache.h
+++ b/cache.h
@@ -1125,7 +1125,7 @@ extern int write_sha1_file(const void *buf, unsigned long len, const char *type,
 extern int hash_sha1_file_literally(const void *buf, unsigned long len, const char *type, unsigned char *sha1, unsigned flags);
 extern int pretend_sha1_file(void *, unsigned long, enum object_type, unsigned char *);
 extern int force_object_loose(const unsigned char *sha1, time_t mtime);
-extern int git_open_noatime(const char *name);
+extern int git_open(const char *name);
 extern void *map_sha1_file(const unsigned char *sha1, unsigned long *size);
 extern int unpack_sha1_header(git_zstream *stream, unsigned char *map, unsigned long mapsize, void *buffer, unsigned long bufsiz);
 extern int parse_sha1_header(const char *hdr, unsigned long *sizep);
diff --git a/pack-bitmap.c b/pack-bitmap.c
index b949e51..39bcc16 100644
--- a/pack-bitmap.c
+++ b/pack-bitmap.c
@@ -266,7 +266,7 @@ static int open_pack_bitmap_1(struct packed_git *packfile)
 		return -1;
 
 	idx_name = pack_bitmap_filename(packfile);
-	fd = git_open_noatime(idx_name);
+	fd = git_open(idx_name);
 	free(idx_name);
 
 	if (fd < 0)
diff --git a/sha1_file.c b/sha1_file.c
index 1e41954..93b836b 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -370,7 +370,7 @@ void read_info_alternates(const char * relative_base, int depth)
 	int fd;
 
 	path = xstrfmt("%s/info/alternates", relative_base);
-	fd = git_open_noatime(path);
+	fd = git_open(path);
 	free(path);
 	if (fd < 0)
 		return;
@@ -663,7 +663,7 @@ static int check_packed_git_idx(const char *path, struct packed_git *p)
 	struct pack_idx_header *hdr;
 	size_t idx_size;
 	uint32_t version, nr, i, *index;
-	int fd = git_open_noatime(path);
+	int fd = git_open(path);
 	struct stat st;
 
 	if (fd < 0)
@@ -1069,7 +1069,7 @@ static int open_packed_git_1(struct packed_git *p)
 	while (pack_max_fds <= pack_open_fds && close_one_pack())
 		; /* nothing */
 
-	p->pack_fd = git_open_noatime(p->pack_name);
+	p->pack_fd = git_open(p->pack_name);
 	if (p->pack_fd < 0 || fstat(p->pack_fd, &st))
 		return -1;
 	pack_open_fds++;
@@ -1586,9 +1586,9 @@ int check_sha1_signature(const unsigned char *sha1, void *map,
 	return hashcmp(sha1, real_sha1) ? -1 : 0;
 }
 
-int git_open_noatime(const char *name)
+int git_open(const char *name)
 {
-	static int sha1_file_open_flag = O_NOATIME;
+	static int sha1_file_open_flag = O_NOATIME | O_CLOEXEC;
 
 	for (;;) {
 		int fd;
@@ -1598,12 +1598,18 @@ int git_open_noatime(const char *name)
 		if (fd >= 0)
 			return fd;
 
-		/* Might the failure be due to O_NOATIME? */
-		if (errno != ENOENT && sha1_file_open_flag) {
-			sha1_file_open_flag = 0;
+		/* Try again w/o O_CLOEXEC: the kernel might not support it */
+		if (O_CLOEXEC && errno == EINVAL &&
+			(sha1_file_open_flag & O_CLOEXEC)) {
+			sha1_file_open_flag &= ~O_CLOEXEC;
 			continue;
 		}
 
+		/* Might the failure be due to O_NOATIME? */
+		if (errno != ENOENT && (sha1_file_open_flag & O_NOATIME)) {
+			sha1_file_open_flag &= ~O_NOATIME;
+			continue;
+		}
 		return -1;
 	}
 }
@@ -1632,7 +1638,7 @@ static int open_sha1_file(const unsigned char *sha1)
 	struct alternate_object_database *alt;
 	int most_interesting_errno;
 
-	fd = git_open_noatime(sha1_file_name(sha1));
+	fd = git_open(sha1_file_name(sha1));
 	if (fd >= 0)
 		return fd;
 	most_interesting_errno = errno;
@@ -1640,7 +1646,7 @@ static int open_sha1_file(const unsigned char *sha1)
 	prepare_alt_odb();
 	for (alt = alt_odb_list; alt; alt = alt->next) {
 		const char *path = alt_sha1_path(alt, sha1);
-		fd = git_open_noatime(path);
+		fd = git_open(path);
 		if (fd >= 0)
 			return fd;
 		if (most_interesting_errno == ENOENT)

-- 
2.10.0
Previous: larsxschneider@gmail.comNext: Johannes Schindelin
Message 2 of 54 in “Use CLOEXEC to avoid fd leaks”
  1. 0/2 Use CLOEXEC to avoid fd leakslarsxschneider@gmail.com, Oct 24, 2016
  2. 1/2 sha1_file: open window into packfiles with CLOEXEClarsxschneider@gmail.com, Oct 24, 2016
  3. Johannes SchindelinOct 25, 2016
  4. Junio C HamanoOct 25, 2016
  5. 2/2 read-cache: make sure file handles are not inherited by child processeslarsxschneider@gmail.com, Oct 24, 2016
  6. Eric WongOct 24, 2016
  7. Junio C HamanoOct 24, 2016
  8. Johannes SchindelinOct 25, 2016
  9. Junio C HamanoOct 25, 2016
  10. Johannes SixtOct 24, 2016
  11. Lars SchneiderOct 24, 2016
  12. Johannes SixtOct 25, 2016
  13. Junio C HamanoOct 24, 2016
  14. Johannes SchindelinOct 25, 2016
  15. 0/3 quick reroll of Lars's git_open() w/ O_CLOEXECJunio C Hamano, Oct 25, 2016
  16. 2/3 sha1_file: open window into packfiles with O_CLOEXECJunio C Hamano, Oct 25, 2016
  17. Jeff KingOct 26, 2016
  18. Junio C HamanoOct 26, 2016
  19. Jeff KingOct 26, 2016
  20. Junio C HamanoOct 26, 2016
  21. Jeff KingOct 26, 2016
  22. Junio C HamanoOct 26, 2016
  23. Jeff KingOct 27, 2016
  24. Junio C HamanoOct 27, 2016
  25. Linus TorvaldsOct 27, 2016
  26. Junio C HamanoOct 27, 2016
  27. Linus TorvaldsOct 27, 2016
  28. Linus TorvaldsOct 27, 2016
  29. Junio C HamanoOct 27, 2016
  30. Linus TorvaldsOct 27, 2016
  31. Junio C HamanoOct 28, 2016
  32. Junio C HamanoOct 28, 2016
  33. Eric WongOct 28, 2016
  34. Johannes SchindelinOct 28, 2016
  35. Linus TorvaldsOct 28, 2016
  36. Junio C HamanoOct 28, 2016
  37. Linus TorvaldsOct 28, 2016
  38. Junio C HamanoOct 28, 2016
  39. Junio C HamanoOct 29, 2016
  40. Johannes SchindelinOct 29, 2016
  41. Linus TorvaldsOct 29, 2016
  42. Junio C HamanoOct 31, 2016
  43. Jeff KingOct 31, 2016
  44. Junio C HamanoOct 31, 2016
  45. Jeff KingOct 31, 2016
  46. Junio C HamanoOct 28, 2016
  47. Junio C HamanoOct 28, 2016
  48. Jeff KingOct 28, 2016
  49. 1/3 sha1_file: rename git_open_noatime() to git_open()Junio C Hamano, Oct 25, 2016
  50. 3/3 read-cache: make sure file handles are not inherited by child processesJunio C Hamano, Oct 25, 2016
  51. Eric WongOct 25, 2016
  52. Junio C HamanoOct 25, 2016
  53. Lars SchneiderOct 25, 2016
  54. Junio C HamanoOct 25, 2016

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.