Merge branch 'jk/diagnose-config-mmap-failure'

gitster · gitster · commit dee47925c1d0 · 2015-06-11T09:29:55.000-07:00
The configuration reader/writer uses mmap(2) interface to access
the files; when we find a directory, it barfed with "Out of memory?".

* jk/diagnose-config-mmap-failure:
  xmmap(): drop "Out of memory?"
  config.c: rewrite ENODEV into EISDIR when mmap fails
  config.c: avoid xmmap error messages
  config.c: fix mmap leak when writing config
  read-cache.c: drop PROT_WRITE from mmap of index
diff --git a/config.c b/config.c
@@ -1939,6 +1939,8 @@ int git_config_set_multivar_in_file(const char *config_filename,
 	int ret;
 	struct lock_file *lock = NULL;
 	char *filename_buf = NULL;
+	char *contents = NULL;
+	size_t contents_sz;
 
 	/* parse-key returns negative; flip the sign to feed exit(3) */
 	ret = 0 - git_config_parse_key(key, &store.key, &store.baselen);
@@ -1988,8 +1990,7 @@ int git_config_set_multivar_in_file(const char *config_filename,
 			goto write_err_out;
 	} else {
 		struct stat st;
-		char *contents;
-		size_t contents_sz, copy_begin, copy_end;
+		size_t copy_begin, copy_end;
 		int i, new_line = 0;
 
 		if (value_regex == NULL)
@@ -2052,8 +2053,17 @@ int git_config_set_multivar_in_file(const char *config_filename,
 
 		fstat(in_fd, &st);
 		contents_sz = xsize_t(st.st_size);
-		contents = xmmap(NULL, contents_sz, PROT_READ,
-			MAP_PRIVATE, in_fd, 0);
+		contents = xmmap_gently(NULL, contents_sz, PROT_READ,
+					MAP_PRIVATE, in_fd, 0);
+		if (contents == MAP_FAILED) {
+			if (errno == ENODEV && S_ISDIR(st.st_mode))
+				errno = EISDIR;
+			error("unable to mmap '%s': %s",
+			      config_filename, strerror(errno));
+			ret = CONFIG_INVALID_FILE;
+			contents = NULL;
+			goto out_free;
+		}
 		close(in_fd);
 
 		if (chmod(lock->filename.buf, st.st_mode & 07777) < 0) {
@@ -2108,8 +2118,6 @@ int git_config_set_multivar_in_file(const char *config_filename,
 					  contents_sz - copy_begin) <
 			    contents_sz - copy_begin)
 				goto write_err_out;
-
-		munmap(contents, contents_sz);
 	}
 
 	if (commit_lock_file(lock) < 0) {
@@ -2135,6 +2143,8 @@ int git_config_set_multivar_in_file(const char *config_filename,
 	if (lock)
 		rollback_lock_file(lock);
 	free(filename_buf);
+	if (contents)
+		munmap(contents, contents_sz);
 	return ret;
 
 write_err_out:
diff --git a/git-compat-util.h b/git-compat-util.h
@@ -718,6 +718,7 @@ extern char *xstrndup(const char *str, size_t len);
 extern void *xrealloc(void *ptr, size_t size);
 extern void *xcalloc(size_t nmemb, size_t size);
 extern void *xmmap(void *start, size_t length, int prot, int flags, int fd, off_t offset);
+extern void *xmmap_gently(void *start, size_t length, int prot, int flags, int fd, off_t offset);
 extern ssize_t xread(int fd, void *buf, size_t len);
 extern ssize_t xwrite(int fd, const void *buf, size_t len);
 extern ssize_t xpread(int fd, void *buf, size_t len, off_t offset);
diff --git a/read-cache.c b/read-cache.c
@@ -1562,7 +1562,7 @@ int do_read_index(struct index_state *istate, const char *path, int must_exist)
 	if (mmap_size < sizeof(struct cache_header) + 20)
 		die("index file smaller than expected");
 
-	mmap = xmmap(NULL, mmap_size, PROT_READ | PROT_WRITE, MAP_PRIVATE, fd, 0);
+	mmap = xmmap(NULL, mmap_size, PROT_READ, MAP_PRIVATE, fd, 0);
 	if (mmap == MAP_FAILED)
 		die_errno("unable to map index file");
 	close(fd);
diff --git a/sha1_file.c b/sha1_file.c
@@ -707,8 +707,8 @@ static void mmap_limit_check(size_t length)
 		    (uintmax_t)length, (uintmax_t)limit);
 }
 
-void *xmmap(void *start, size_t length,
-	int prot, int flags, int fd, off_t offset)
+void *xmmap_gently(void *start, size_t length,
+		  int prot, int flags, int fd, off_t offset)
 {
 	void *ret;
 
@@ -719,12 +719,19 @@ void *xmmap(void *start, size_t length,
 			return NULL;
 		release_pack_memory(length);
 		ret = mmap(start, length, prot, flags, fd, offset);
-		if (ret == MAP_FAILED)
-			die_errno("Out of memory? mmap failed");
 	}
 	return ret;
 }
 
+void *xmmap(void *start, size_t length,
+	int prot, int flags, int fd, off_t offset)
+{
+	void *ret = xmmap_gently(start, length, prot, flags, fd, offset);
+	if (ret == MAP_FAILED)
+		die_errno("mmap failed");
+	return ret;
+}
+
 void close_pack_windows(struct packed_git *p)
 {
 	while (p->windows) {

Original file line number	Diff line number	Diff line change
`@@ -707,8 +707,8 @@ static void mmap_limit_check(size_t length)`
`707`	`707`	`(uintmax_t)length, (uintmax_t)limit);`
`708`	`708`	`}`
`709`	`709`
`710`		`-void xmmap(void start, size_t length,`
`711`		`- int prot, int flags, int fd, off_t offset)`
	`710`	`+void xmmap_gently(void start, size_t length,`
	`711`	`+ int prot, int flags, int fd, off_t offset)`
`712`	`712`	`{`
`713`	`713`	`void *ret;`
`714`	`714`
`@@ -719,12 +719,19 @@ void xmmap(void start, size_t length,`
`719`	`719`	`return NULL;`
`720`	`720`	`release_pack_memory(length);`
`721`	`721`	`ret = mmap(start, length, prot, flags, fd, offset);`
`722`		`- if (ret == MAP_FAILED)`
`723`		`- die_errno("Out of memory? mmap failed");`
`724`	`722`	`}`
`725`	`723`	`return ret;`
`726`	`724`	`}`
`727`	`725`
	`726`	`+void xmmap(void start, size_t length,`
	`727`	`+ int prot, int flags, int fd, off_t offset)`
	`728`	`+{`
	`729`	`+ void *ret = xmmap_gently(start, length, prot, flags, fd, offset);`
	`730`	`+ if (ret == MAP_FAILED)`
	`731`	`+ die_errno("mmap failed");`
	`732`	`+ return ret;`
	`733`	`+}`
	`734`	`+`
`728`	`735`	`void close_pack_windows(struct packed_git *p)`
`729`	`736`	`{`
`730`	`737`	`while (p->windows) {`