From: "Shawn O. Pearce" <spearce@spearce.org>
To: Junio C Hamano <junkio@cox.net>
Cc: git@vger.kernel.org
Subject: [PATCH 7/7] Replace mmap with xmmap, better handling MAP_FAILED.
Date: Sun, 24 Dec 2006 00:47:23 -0500 [thread overview]
Message-ID: <20061224054723.GG8146@spearce.org> (raw)
In-Reply-To: <487c7d0ea81f2f82f330e277e0aea38a66ca7cfe.1166939109.git.spearce@spearce.org>
In some cases we did not even bother to check the return value of
mmap() and just assume it worked. This is bad, because if we are
out of virtual address space the kernel returned MAP_FAILED and we
would attempt to dereference that address, segfaulting without any
real error output to the user.
We are replacing all calls to mmap() with xmmap() and moving all
MAP_FAILED checking into that single location. If a mmap call
fails we try to release enough least-recently-used pack windows
to possibly succeed, then retry the mmap() attempt. If we cannot
mmap even after releasing pack memory then we die() as none of our
callers have any reasonable recovery strategy for a failed mmap.
Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
config.c | 2 +-
diff.c | 4 +---
git-compat-util.h | 13 +++++++++++++
read-cache.c | 2 +-
refs.c | 2 +-
sha1_file.c | 18 +++++-------------
6 files changed, 22 insertions(+), 19 deletions(-)
diff --git a/config.c b/config.c
index edc42f4..410d5e9 100644
--- a/config.c
+++ b/config.c
@@ -698,7 +698,7 @@ int git_config_set_multivar(const char* key, const char* value,
}
fstat(in_fd, &st);
- contents = mmap(NULL, st.st_size, PROT_READ,
+ contents = xmmap(NULL, st.st_size, PROT_READ,
MAP_PRIVATE, in_fd, 0);
close(in_fd);
diff --git a/diff.c b/diff.c
index f14288b..244292a 100644
--- a/diff.c
+++ b/diff.c
@@ -1341,10 +1341,8 @@ int diff_populate_filespec(struct diff_filespec *s, int size_only)
fd = open(s->path, O_RDONLY);
if (fd < 0)
goto err_empty;
- s->data = mmap(NULL, s->size, PROT_READ, MAP_PRIVATE, fd, 0);
+ s->data = xmmap(NULL, s->size, PROT_READ, MAP_PRIVATE, fd, 0);
close(fd);
- if (s->data == MAP_FAILED)
- goto err_empty;
s->should_munmap = 1;
}
else {
diff --git a/git-compat-util.h b/git-compat-util.h
index 4a417be..51e8d7a 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -185,6 +185,19 @@ static inline void *xcalloc(size_t nmemb, size_t size)
return ret;
}
+static inline void *xmmap(void *start, size_t length,
+ int prot, int flags, int fd, off_t offset)
+{
+ void *ret = mmap(start, length, prot, flags, fd, offset);
+ if (ret == MAP_FAILED) {
+ release_pack_memory(length);
+ ret = mmap(start, length, prot, flags, fd, offset);
+ if (ret == MAP_FAILED)
+ die("Out of memory? mmap failed: %s", strerror(errno));
+ }
+ return ret;
+}
+
static inline ssize_t xread(int fd, void *buf, size_t len)
{
ssize_t nr;
diff --git a/read-cache.c b/read-cache.c
index b8d83cc..ca3efbb 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -798,7 +798,7 @@ int read_cache_from(const char *path)
cache_mmap_size = st.st_size;
errno = EINVAL;
if (cache_mmap_size >= sizeof(struct cache_header) + 20)
- cache_mmap = mmap(NULL, cache_mmap_size, PROT_READ | PROT_WRITE, MAP_PRIVATE, fd, 0);
+ cache_mmap = xmmap(NULL, cache_mmap_size, PROT_READ | PROT_WRITE, MAP_PRIVATE, fd, 0);
}
close(fd);
if (cache_mmap == MAP_FAILED)
diff --git a/refs.c b/refs.c
index a101ff3..286ae45 100644
--- a/refs.c
+++ b/refs.c
@@ -1025,7 +1025,7 @@ int read_ref_at(const char *ref, unsigned long at_time, int cnt, unsigned char *
fstat(logfd, &st);
if (!st.st_size)
die("Log %s is empty.", logfile);
- logdata = mmap(NULL, st.st_size, PROT_READ, MAP_PRIVATE, logfd, 0);
+ logdata = xmmap(NULL, st.st_size, PROT_READ, MAP_PRIVATE, logfd, 0);
close(logfd);
lastrec = NULL;
diff --git a/sha1_file.c b/sha1_file.c
index fb1032b..84037fe 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -355,10 +355,8 @@ static void read_info_alternates(const char * relative_base, int depth)
close(fd);
return;
}
- map = mmap(NULL, st.st_size, PROT_READ, MAP_PRIVATE, fd, 0);
+ map = xmmap(NULL, st.st_size, PROT_READ, MAP_PRIVATE, fd, 0);
close(fd);
- if (map == MAP_FAILED)
- return;
link_alt_odb_entries(map, map + st.st_size, '\n', relative_base, depth);
@@ -442,10 +440,8 @@ static int check_packed_git_idx(const char *path, unsigned long *idx_size_,
return -1;
}
idx_size = st.st_size;
- idx_map = mmap(NULL, idx_size, PROT_READ, MAP_PRIVATE, fd, 0);
+ idx_map = xmmap(NULL, idx_size, PROT_READ, MAP_PRIVATE, fd, 0);
close(fd);
- if (idx_map == MAP_FAILED)
- return -1;
index = idx_map;
*idx_map_ = idx_map;
@@ -630,7 +626,7 @@ unsigned char* use_pack(struct packed_git *p,
while (packed_git_limit < pack_mapped
&& unuse_one_window(p))
; /* nothing */
- win->base = mmap(NULL, win->len,
+ win->base = xmmap(NULL, win->len,
PROT_READ, MAP_PRIVATE,
p->pack_fd, win->offset);
if (win->base == MAP_FAILED)
@@ -828,10 +824,8 @@ void *map_sha1_file(const unsigned char *sha1, unsigned long *size)
*/
sha1_file_open_flag = 0;
}
- map = mmap(NULL, st.st_size, PROT_READ, MAP_PRIVATE, fd, 0);
+ map = xmmap(NULL, st.st_size, PROT_READ, MAP_PRIVATE, fd, 0);
close(fd);
- if (map == MAP_FAILED)
- return NULL;
*size = st.st_size;
return map;
}
@@ -1987,10 +1981,8 @@ int index_fd(unsigned char *sha1, int fd, struct stat *st, int write_object, con
buf = "";
if (size)
- buf = mmap(NULL, size, PROT_READ, MAP_PRIVATE, fd, 0);
+ buf = xmmap(NULL, size, PROT_READ, MAP_PRIVATE, fd, 0);
close(fd);
- if (buf == MAP_FAILED)
- return -1;
if (!type)
type = blob_type;
--
1.4.4.3.g2e63
next prev parent reply other threads:[~2006-12-24 5:47 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <487c7d0ea81f2f82f330e277e0aea38a66ca7cfe.1166939109.git.spearce@spearce.org>
2006-12-24 5:45 ` [PATCH 2/7] Switch git_mmap to use pread Shawn O. Pearce
2006-12-24 13:09 ` Johannes Schindelin
2006-12-24 19:30 ` Alon Ziv
2006-12-24 20:13 ` Shawn Pearce
2006-12-24 20:14 ` Linus Torvalds
2006-12-24 5:46 ` [PATCH 3/7] Ensure packed_git.next is initialized to NULL Shawn O. Pearce
2006-12-24 5:46 ` [PATCH 4/7] Default core.packdGitWindowSize to 1 MiB if NO_MMAP Shawn O. Pearce
2006-12-24 5:46 ` [PATCH 5/7] Don't exit successfully on EPIPE in read_or_die Shawn O. Pearce
2006-12-24 5:47 ` [PATCH 6/7] Release pack windows before reporting out of memory Shawn O. Pearce
2006-12-24 13:10 ` Johannes Schindelin
2006-12-24 5:47 ` Shawn O. Pearce [this message]
2006-12-24 13:22 ` [PATCH 7/7] Replace mmap with xmmap, better handling MAP_FAILED Johannes Schindelin
2006-12-24 20:34 ` Shawn Pearce
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20061224054723.GG8146@spearce.org \
--to=spearce@spearce.org \
--cc=git@vger.kernel.org \
--cc=junkio@cox.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).