Skip to content

Commit

Permalink
Merge branch 'dt/name-hash-dir-entry-fix' into maint
Browse files Browse the repository at this point in the history
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
  • Loading branch information
Junio C Hamano committed Nov 3, 2015
2 parents ced2321 + 41284eb commit 3a27eec
Show file tree
Hide file tree
Showing 4 changed files with 35 additions and 60 deletions.
3 changes: 2 additions & 1 deletion cache.h
Original file line number Diff line number Diff line change
Expand Up @@ -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 */
Expand Down
22 changes: 4 additions & 18 deletions dir.c
Original file line number Diff line number Diff line change
Expand Up @@ -1202,29 +1202,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;
}

Expand Down
54 changes: 28 additions & 26 deletions name-hash.c
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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;

Expand All @@ -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 */
Expand Down Expand Up @@ -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)
Expand Down
16 changes: 1 addition & 15 deletions read-cache.c
Original file line number Diff line number Diff line change
Expand Up @@ -679,21 +679,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);
Expand Down

0 comments on commit 3a27eec

Please sign in to comment.