Re: [PATCH 0/1] merge: fix cache_entry use-after-free

2015-10-09 Thread Junio C Hamano
David Turner writes: >> > + assert(removed == dir); >> > + drop_ce_ref(dir->ce); >> >> This is curious. In remove_name_hash() you do not have the >> corresponding assert. Why is it necessary here (or is it >> unnecessary over there)? > > It is unnecessary in any case: it's an

Re: [PATCH 0/1] merge: fix cache_entry use-after-free

2015-10-09 Thread David Turner
+Duy Nguyen, who knows the split index better. On Thu, 2015-10-08 at 13:00 -0700, Junio C Hamano wrote: > David Turner writes: > > > From: Keith McGuigan > > > > During merges, we would previously free entries that we no longer need > > in the destination index. But those entries might also be

Re: [PATCH 0/1] merge: fix cache_entry use-after-free

2015-10-08 Thread Junio C Hamano
David Turner writes: > From: Keith McGuigan > > During merges, we would previously free entries that we no longer need > in the destination index. But those entries might also be stored in > the dir_entry cache, and when a later call to add_to_index found them, > they would be used after being

[PATCH 0/1] merge: fix cache_entry use-after-free

2015-10-08 Thread David Turner
Keith diagnosed the problem and wrote the patch. I wrote the commit message and am sending it upstream with his OK. Keith McGuigan (1): merge: fix cache_entry use-after-free cache.h| 27 +++ name-hash.c| 7 ++- read-cache.c | 5 - split-index.c

[PATCH 0/1] merge: fix cache_entry use-after-free

2015-10-08 Thread David Turner
From: Keith McGuigan During merges, we would previously free entries that we no longer need in the destination index. But those entries might also be 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, add a ref