Merge branch 'dt/name-hash-dir-entry-fix'

The name-hash subsystem that is used to cope with case insensitive
filesystems keeps track of directories and their on-filesystem
cases for all the paths in the index by holding a pointer to a
randomly chosen cache entry that is inside the directory (for its
ce->ce_name component).  This pointer was not updated even when the
cache entry was removed from the index, leading to use after free.
This was fixed by recording the path for each directory instead of
borrowing cache entries and restructuring the API somewhat.

* dt/name-hash-dir-entry-fix:
  name-hash: don't reuse cache_entry in dir_entry

Author: gitster

cache.h

@@ -521,7 +521,8 @@ extern int write_locked_index(struct index_state *, struct lock_file *lock, unsi
 extern int discard_index(struct index_state *);
 extern int unmerged_index(const struct index_state *);
 extern int verify_path(const char *path);
-extern struct cache_entry *index_dir_exists(struct index_state *istate, const char *name, int namelen);
+extern int index_dir_exists(struct index_state *istate, const char *name, int namelen);
+extern void adjust_dirname_case(struct index_state *istate, char *name);
 extern struct cache_entry *index_file_exists(struct index_state *istate, const char *name, int namelen, int igncase);
 extern int index_name_pos(const struct index_state *, const char *name, int namelen);
 #define ADD_CACHE_OK_TO_ADD 1		/* Ok to add */

dir.c

@@ -1279,29 +1279,15 @@ enum exist_status {
  */
 static enum exist_status directory_exists_in_index_icase(const char *dirname, int len)
 {
-	const struct cache_entry *ce = cache_dir_exists(dirname, len);
-	unsigned char endchar;
+	struct cache_entry *ce;
 
-	if (!ce)
-		return index_nonexistent;
-	endchar = ce->name[len];
-
-	/*
-	 * The cache_entry structure returned will contain this dirname
-	 * and possibly additional path components.
-	 */
-	if (endchar == '/')
+	if (cache_dir_exists(dirname, len))
 		return index_directory;
 
-	/*
-	 * If there are no additional path components, then this cache_entry
-	 * represents a submodule.  Submodules, despite being directories,
-	 * are stored in the cache without a closing slash.
-	 */
-	if (!endchar && S_ISGITLINK(ce->ce_mode))
+	ce = cache_file_exists(dirname, len, ignore_case);
+	if (ce && S_ISGITLINK(ce->ce_mode))
 		return index_gitdir;
 
-	/* This should never be hit, but it exists just in case. */
 	return index_nonexistent;
 }
 

name-hash.c

@@ -11,16 +11,16 @@
 struct dir_entry {
 	struct hashmap_entry ent;
 	struct dir_entry *parent;
-	struct cache_entry *ce;
 	int nr;
 	unsigned int namelen;
+	char name[FLEX_ARRAY];
 };
 
 static int dir_entry_cmp(const struct dir_entry *e1,
 		const struct dir_entry *e2, const char *name)
 {
-	return e1->namelen != e2->namelen || strncasecmp(e1->ce->name,
-			name ? name : e2->ce->name, e1->namelen);
+	return e1->namelen != e2->namelen || strncasecmp(e1->name,
+			name ? name : e2->name, e1->namelen);
 }
 
 static struct dir_entry *find_dir_entry(struct index_state *istate,
@@ -41,14 +41,6 @@ static struct dir_entry *hash_dir_entry(struct index_state *istate,
 	 * closing slash.  Despite submodules being a directory, they never
 	 * reach this point, because they are stored
 	 * in index_state.name_hash (as ordinary cache_entries).
-	 *
-	 * Note that the cache_entry stored with the dir_entry merely
-	 * supplies the name of the directory (up to dir_entry.namelen). We
-	 * track the number of 'active' files in a directory in dir_entry.nr,
-	 * so we can tell if the directory is still relevant, e.g. for git
-	 * status. However, if cache_entries are removed, we cannot pinpoint
-	 * an exact cache_entry that's still active. It is very possible that
-	 * multiple dir_entries point to the same cache_entry.
 	 */
 	struct dir_entry *dir;
 
@@ -63,10 +55,10 @@ static struct dir_entry *hash_dir_entry(struct index_state *istate,
 	dir = find_dir_entry(istate, ce->name, namelen);
 	if (!dir) {
 		/* not found, create it and add to hash table */
-		dir = xcalloc(1, sizeof(struct dir_entry));
+		dir = xcalloc(1, sizeof(struct dir_entry) + namelen + 1);
 		hashmap_entry_init(dir, memihash(ce->name, namelen));
 		dir->namelen = namelen;
-		dir->ce = ce;
+		strncpy(dir->name, ce->name, namelen);
 		hashmap_add(&istate->dir_hash, dir);
 
 		/* recursively add missing parent directories */
@@ -188,26 +180,36 @@ static int same_name(const struct cache_entry *ce, const char *name, int namelen
 	return slow_same_name(name, namelen, ce->name, len);
 }
 
-struct cache_entry *index_dir_exists(struct index_state *istate, const char *name, int namelen)
+int index_dir_exists(struct index_state *istate, const char *name, int namelen)
 {
-	struct cache_entry *ce;
 	struct dir_entry *dir;
 
 	lazy_init_name_hash(istate);
 	dir = find_dir_entry(istate, name, namelen);
-	if (dir && dir->nr)
-		return dir->ce;
+	return dir && dir->nr;
+}
 
-	/*
-	 * It might be a submodule. Unlike plain directories, which are stored
-	 * in the dir-hash, submodules are stored in the name-hash, so check
-	 * there, as well.
-	 */
-	ce = index_file_exists(istate, name, namelen, 1);
-	if (ce && S_ISGITLINK(ce->ce_mode))
-		return ce;
+void adjust_dirname_case(struct index_state *istate, char *name)
+{
+	const char *startPtr = name;
+	const char *ptr = startPtr;
 
-	return NULL;
+	lazy_init_name_hash(istate);
+	while (*ptr) {
+		while (*ptr && *ptr != '/')
+			ptr++;
+
+		if (*ptr == '/') {
+			struct dir_entry *dir;
+
+			ptr++;
+			dir = find_dir_entry(istate, name, ptr - name + 1);
+			if (dir) {
+				memcpy((void *)startPtr, dir->name + (startPtr - name), ptr - startPtr);
+				startPtr = ptr;
+			}
+		}
+	}
 }
 
 struct cache_entry *index_file_exists(struct index_state *istate, const char *name, int namelen, int icase)

read-cache.c

@@ -678,21 +678,7 @@ int add_to_index(struct index_state *istate, const char *path, struct stat *st,
 	 * entry's directory case.
 	 */
 	if (ignore_case) {
-		const char *startPtr = ce->name;
-		const char *ptr = startPtr;
-		while (*ptr) {
-			while (*ptr && *ptr != '/')
-				++ptr;
-			if (*ptr == '/') {
-				struct cache_entry *foundce;
-				++ptr;
-				foundce = index_dir_exists(istate, ce->name, ptr - ce->name - 1);
-				if (foundce) {
-					memcpy((void *)startPtr, foundce->name + (startPtr - ce->name), ptr - startPtr);
-					startPtr = ptr;
-				}
-			}
-		}
+		adjust_dirname_case(istate, ce->name);
 	}
 
 	alias = index_file_exists(istate, ce->name, ce_namelen(ce), ignore_case);