Skip to content

Commit 3a27eec

Browse files
committed
Merge branch 'dt/name-hash-dir-entry-fix' into maint
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
2 parents ced2321 + 41284eb commit 3a27eec

File tree

4 files changed

+35
-60
lines changed

4 files changed

+35
-60
lines changed

cache.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -521,7 +521,8 @@ extern int write_locked_index(struct index_state *, struct lock_file *lock, unsi
521521
extern int discard_index(struct index_state *);
522522
extern int unmerged_index(const struct index_state *);
523523
extern int verify_path(const char *path);
524-
extern struct cache_entry *index_dir_exists(struct index_state *istate, const char *name, int namelen);
524+
extern int index_dir_exists(struct index_state *istate, const char *name, int namelen);
525+
extern void adjust_dirname_case(struct index_state *istate, char *name);
525526
extern struct cache_entry *index_file_exists(struct index_state *istate, const char *name, int namelen, int igncase);
526527
extern int index_name_pos(const struct index_state *, const char *name, int namelen);
527528
#define ADD_CACHE_OK_TO_ADD 1 /* Ok to add */

dir.c

Lines changed: 4 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1202,29 +1202,15 @@ enum exist_status {
12021202
*/
12031203
static enum exist_status directory_exists_in_index_icase(const char *dirname, int len)
12041204
{
1205-
const struct cache_entry *ce = cache_dir_exists(dirname, len);
1206-
unsigned char endchar;
1205+
struct cache_entry *ce;
12071206

1208-
if (!ce)
1209-
return index_nonexistent;
1210-
endchar = ce->name[len];
1211-
1212-
/*
1213-
* The cache_entry structure returned will contain this dirname
1214-
* and possibly additional path components.
1215-
*/
1216-
if (endchar == '/')
1207+
if (cache_dir_exists(dirname, len))
12171208
return index_directory;
12181209

1219-
/*
1220-
* If there are no additional path components, then this cache_entry
1221-
* represents a submodule. Submodules, despite being directories,
1222-
* are stored in the cache without a closing slash.
1223-
*/
1224-
if (!endchar && S_ISGITLINK(ce->ce_mode))
1210+
ce = cache_file_exists(dirname, len, ignore_case);
1211+
if (ce && S_ISGITLINK(ce->ce_mode))
12251212
return index_gitdir;
12261213

1227-
/* This should never be hit, but it exists just in case. */
12281214
return index_nonexistent;
12291215
}
12301216

name-hash.c

Lines changed: 28 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -11,16 +11,16 @@
1111
struct dir_entry {
1212
struct hashmap_entry ent;
1313
struct dir_entry *parent;
14-
struct cache_entry *ce;
1514
int nr;
1615
unsigned int namelen;
16+
char name[FLEX_ARRAY];
1717
};
1818

1919
static int dir_entry_cmp(const struct dir_entry *e1,
2020
const struct dir_entry *e2, const char *name)
2121
{
22-
return e1->namelen != e2->namelen || strncasecmp(e1->ce->name,
23-
name ? name : e2->ce->name, e1->namelen);
22+
return e1->namelen != e2->namelen || strncasecmp(e1->name,
23+
name ? name : e2->name, e1->namelen);
2424
}
2525

2626
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,
4141
* closing slash. Despite submodules being a directory, they never
4242
* reach this point, because they are stored
4343
* in index_state.name_hash (as ordinary cache_entries).
44-
*
45-
* Note that the cache_entry stored with the dir_entry merely
46-
* supplies the name of the directory (up to dir_entry.namelen). We
47-
* track the number of 'active' files in a directory in dir_entry.nr,
48-
* so we can tell if the directory is still relevant, e.g. for git
49-
* status. However, if cache_entries are removed, we cannot pinpoint
50-
* an exact cache_entry that's still active. It is very possible that
51-
* multiple dir_entries point to the same cache_entry.
5244
*/
5345
struct dir_entry *dir;
5446

@@ -63,10 +55,10 @@ static struct dir_entry *hash_dir_entry(struct index_state *istate,
6355
dir = find_dir_entry(istate, ce->name, namelen);
6456
if (!dir) {
6557
/* not found, create it and add to hash table */
66-
dir = xcalloc(1, sizeof(struct dir_entry));
58+
dir = xcalloc(1, sizeof(struct dir_entry) + namelen + 1);
6759
hashmap_entry_init(dir, memihash(ce->name, namelen));
6860
dir->namelen = namelen;
69-
dir->ce = ce;
61+
strncpy(dir->name, ce->name, namelen);
7062
hashmap_add(&istate->dir_hash, dir);
7163

7264
/* recursively add missing parent directories */
@@ -188,26 +180,36 @@ static int same_name(const struct cache_entry *ce, const char *name, int namelen
188180
return slow_same_name(name, namelen, ce->name, len);
189181
}
190182

191-
struct cache_entry *index_dir_exists(struct index_state *istate, const char *name, int namelen)
183+
int index_dir_exists(struct index_state *istate, const char *name, int namelen)
192184
{
193-
struct cache_entry *ce;
194185
struct dir_entry *dir;
195186

196187
lazy_init_name_hash(istate);
197188
dir = find_dir_entry(istate, name, namelen);
198-
if (dir && dir->nr)
199-
return dir->ce;
189+
return dir && dir->nr;
190+
}
200191

201-
/*
202-
* It might be a submodule. Unlike plain directories, which are stored
203-
* in the dir-hash, submodules are stored in the name-hash, so check
204-
* there, as well.
205-
*/
206-
ce = index_file_exists(istate, name, namelen, 1);
207-
if (ce && S_ISGITLINK(ce->ce_mode))
208-
return ce;
192+
void adjust_dirname_case(struct index_state *istate, char *name)
193+
{
194+
const char *startPtr = name;
195+
const char *ptr = startPtr;
209196

210-
return NULL;
197+
lazy_init_name_hash(istate);
198+
while (*ptr) {
199+
while (*ptr && *ptr != '/')
200+
ptr++;
201+
202+
if (*ptr == '/') {
203+
struct dir_entry *dir;
204+
205+
ptr++;
206+
dir = find_dir_entry(istate, name, ptr - name + 1);
207+
if (dir) {
208+
memcpy((void *)startPtr, dir->name + (startPtr - name), ptr - startPtr);
209+
startPtr = ptr;
210+
}
211+
}
212+
}
211213
}
212214

213215
struct cache_entry *index_file_exists(struct index_state *istate, const char *name, int namelen, int icase)

read-cache.c

Lines changed: 1 addition & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -679,21 +679,7 @@ int add_to_index(struct index_state *istate, const char *path, struct stat *st,
679679
* entry's directory case.
680680
*/
681681
if (ignore_case) {
682-
const char *startPtr = ce->name;
683-
const char *ptr = startPtr;
684-
while (*ptr) {
685-
while (*ptr && *ptr != '/')
686-
++ptr;
687-
if (*ptr == '/') {
688-
struct cache_entry *foundce;
689-
++ptr;
690-
foundce = index_dir_exists(istate, ce->name, ptr - ce->name - 1);
691-
if (foundce) {
692-
memcpy((void *)startPtr, foundce->name + (startPtr - ce->name), ptr - startPtr);
693-
startPtr = ptr;
694-
}
695-
}
696-
}
682+
adjust_dirname_case(istate, ce->name);
697683
}
698684

699685
alias = index_file_exists(istate, ce->name, ce_namelen(ce), ignore_case);

0 commit comments

Comments
 (0)