From 92fda79ed048d2d37e760e7a1b6055b2fc801ee3 Mon Sep 17 00:00:00 2001 From: Jonathan Nieder Date: Wed, 12 Jan 2011 20:26:36 -0600 Subject: [PATCH 1/2] unpack-trees: handle lstat failure for existing directory When check_leading_path notices no file in the way of the new entry to be checked out, verify_absent checks whether there is a directory there or nothing at all. If that lstat call fails (for example due to ENOMEM), it assumes ENOENT, meaning a directory with untracked files would be clobbered in that case. Check errno after calling lstat, and for conditions other than ENOENT, just error out. This is a theoretical race condition. lstat has to succeed moments before it fails for there to be trouble. Signed-off-by: Jonathan Nieder Signed-off-by: Junio C Hamano --- unpack-trees.c | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/unpack-trees.c b/unpack-trees.c index d5a453079a..3011d9b904 100644 --- a/unpack-trees.c +++ b/unpack-trees.c @@ -1197,12 +1197,16 @@ static int verify_absent_1(struct cache_entry *ce, return check_ok_to_remove(path, len, DT_UNKNOWN, NULL, &st, error_type, o); - } else if (!lstat(ce->name, &st)) + } else if (lstat(ce->name, &st)) { + if (errno != ENOENT) + return error("cannot stat '%s': %s", ce->name, + strerror(errno)); + return 0; + } else { return check_ok_to_remove(ce->name, ce_namelen(ce), - ce_to_dtype(ce), ce, &st, - error_type, o); - - return 0; + ce_to_dtype(ce), ce, &st, + error_type, o); + } } static int verify_absent(struct cache_entry *ce, From a93e530184b3f7ae9d9bfb0e569734687f8d1c0b Mon Sep 17 00:00:00 2001 From: Jonathan Nieder Date: Wed, 12 Jan 2011 20:28:09 -0600 Subject: [PATCH 2/2] unpack-trees: handle lstat failure for existing file When check_leading_path notices a file in the way of a new entry to be checked out, verify_absent uses (1) the mode to determine whether it is a directory (2) the rest of the stat information to check if this is actually an old entry, disguised by a change in filename (e.g., README -> Readme) that is significant to git but insignificant to the underlying filesystem. If lstat fails, these checks are performed with an uninitialied stat structure, producing essentially random results. Better to just error out when lstat fails. The easiest way to reproduce this is to remove a file after the check_leading_path call and before the lstat in verify_absent. An lstat failure other than ENOENT in check_leading_path would also trigger the same code path. Signed-off-by: Jonathan Nieder Signed-off-by: Junio C Hamano --- unpack-trees.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/unpack-trees.c b/unpack-trees.c index 3011d9b904..24227fc6c6 100644 --- a/unpack-trees.c +++ b/unpack-trees.c @@ -1193,7 +1193,9 @@ static int verify_absent_1(struct cache_entry *ce, char path[PATH_MAX + 1]; memcpy(path, ce->name, len); path[len] = 0; - lstat(path, &st); + if (lstat(path, &st)) + return error("cannot stat '%s': %s", path, + strerror(errno)); return check_ok_to_remove(path, len, DT_UNKNOWN, NULL, &st, error_type, o);