name-hash: don't reuse cache_entry in dir_entry
Stop reusing cache_entry in dir_entry; doing so causes a use-after-free bug. During merges, we free entries that we no longer need in the destination index. But those entries might have also been stored in the dir_entry cache, and when a later call to add_to_index found them, they would be used after being freed. To prevent this, change dir_entry to store a copy of the name instead of a pointer to a cache_entry. This entails some refactoring of code that expects the cache_entry. Keith McGuigan <kmcguigan@twitter.com> diagnosed this bug and wrote the initial patch, but this version does not use any of Keith's code. Helped-by: Keith McGuigan <kmcguigan@twitter.com> Helped-by: Junio C Hamano <gitster@pobox.com> Signed-off-by: David Turner <dturner@twopensource.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
David Turner committed
Oct 21, 2015 at 13:54 UTC
41284eb0f944fe2d73708bb4105a8e3ccd0297df
4 files changed
+35
-60
cache.h
+2
-1
@@ -501,7 +501,8 @@ extern int write_locked_index(struct index_state *, struct lock_file *lock, unsi
501
extern int discard_index(struct index_state *);
502
extern int unmerged_index(const struct index_state *);
503
extern int verify_path(const char *path);
504
-extern struct cache_entry *index_dir_exists(struct index_state *istate, const char *name, int namelen);
504
+extern int index_dir_exists(struct index_state *istate, const char *name, int namelen);
505
+extern void adjust_dirname_case(struct index_state *istate, char *name);
506
extern struct cache_entry *index_file_exists(struct index_state *istate, const char *name, int namelen, int igncase);
507
extern int index_name_pos(const struct index_state *, const char *name, int namelen);
508
#define ADD_CACHE_OK_TO_ADD 1 /* Ok to add */
dir.c
+4
-18
@@ -964,29 +964,15 @@ enum exist_status {
964
*/
965
static enum exist_status directory_exists_in_index_icase(const char *dirname, int len)
966
{
967
- const struct cache_entry *ce = cache_dir_exists(dirname, len);
968
- unsigned char endchar;
967
+ struct cache_entry *ce;
968
970
- if (!ce)
971
- return index_nonexistent;
972
- endchar = ce->name[len];
973
-
974
- /*
975
- * The cache_entry structure returned will contain this dirname
976
- * and possibly additional path components.
977
- */
978
- if (endchar == '/')
969
+ if (cache_dir_exists(dirname, len))
970
return index_directory;
971
981
- /*
982
- * If there are no additional path components, then this cache_entry
983
- * represents a submodule. Submodules, despite being directories,
984
- * are stored in the cache without a closing slash.
985
- */
986
- if (!endchar && S_ISGITLINK(ce->ce_mode))
972
+ ce = cache_file_exists(dirname, len, ignore_case);
973
+ if (ce && S_ISGITLINK(ce->ce_mode))
974
return index_gitdir;
975
989
- /* This should never be hit, but it exists just in case. */
976
return index_nonexistent;
977
}
978
name-hash.c
+28
-26
@@ -11,16 +11,16 @@
11
struct dir_entry {
12
struct hashmap_entry ent;
13
struct dir_entry *parent;
14
- struct cache_entry *ce;
14
int nr;
15
unsigned int namelen;
16
+ char name[FLEX_ARRAY];
17
};
18
19
static int dir_entry_cmp(const struct dir_entry *e1,
20
const struct dir_entry *e2, const char *name)
21
{
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);
24
}
25
26
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,
41
* closing slash. Despite submodules being a directory, they never
42
* reach this point, because they are stored
43
* 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.
44
*/
45
struct dir_entry *dir;
46
@@ -63,10 +55,10 @@ static struct dir_entry *hash_dir_entry(struct index_state *istate,
55
dir = find_dir_entry(istate, ce->name, namelen);
56
if (!dir) {
57
/* 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);
59
hashmap_entry_init(dir, memihash(ce->name, namelen));
60
dir->namelen = namelen;
69
- dir->ce = ce;
61
+ strncpy(dir->name, ce->name, namelen);
62
hashmap_add(&istate->dir_hash, dir);
63
64
/* recursively add missing parent directories */
@@ -188,26 +180,36 @@ static int same_name(const struct cache_entry *ce, const char *name, int namelen
180
return slow_same_name(name, namelen, ce->name, len);
181
}
182
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)
184
{
193
- struct cache_entry *ce;
185
struct dir_entry *dir;
186
187
lazy_init_name_hash(istate);
188
dir = find_dir_entry(istate, name, namelen);
198
- if (dir && dir->nr)
199
- return dir->ce;
189
+ return dir && dir->nr;
190
+}
191
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;
196
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
+ }
213
}
214
215
struct cache_entry *index_file_exists(struct index_state *istate, const char *name, int namelen, int icase)
read-cache.c
+1
-15
@@ -661,21 +661,7 @@ int add_to_index(struct index_state *istate, const char *path, struct stat *st,
661
* entry's directory case.
662
*/
663
if (ignore_case) {
664
- const char *startPtr = ce->name;
665
- const char *ptr = startPtr;
666
- while (*ptr) {
667
- while (*ptr && *ptr != '/')
668
- ++ptr;
669
- if (*ptr == '/') {
670
- struct cache_entry *foundce;
671
- ++ptr;
672
- foundce = index_dir_exists(istate, ce->name, ptr - ce->name - 1);
673
- if (foundce) {
674
- memcpy((void *)startPtr, foundce->name + (startPtr - ce->name), ptr - startPtr);
675
- startPtr = ptr;
676
- }
677
- }
678
- }
664
+ adjust_dirname_case(istate, ce->name);
665
}
666
667
alias = index_file_exists(istate, ce->name, ce_namelen(ce), ignore_case);