Am 18.10.2013 21:09, schrieb Junio C Hamano: > Karsten Blees <karsten.blees@xxxxxxxxx> writes: > >> The coredumps are caused by my patch #10, which free()s >> cache_entries when they are removed, in combination with ... > > Looking at that patch, it makes me wonder if remove_index_entry_at() > and replace_index_entry() should be the ones that frees the old > entry in the first place. A caller may already have a ce pointing > at an old entry and use the information from old_ce to update a new > one after it installed it, e.g. > > old_ce = ... > new_ce = make_cache_entry(... old_ce->name, ...); > replace_index_entry(... new_ce); > new_ce->ce_mode = old_ce->cd_mode; > free(old_ce); > > The same goes for the functions that remove the entry. > Moving free() to the callers or caller's callers would make it much more complicated (more places to change). Besides, most callers don't even have a reference to old_ce and simply remove by position. Of course, this doesn't prevent caller's caller's callers to keep a reference to a removed / replaced entry, as found by Thomas. > > Going forward, I do agree with your patch #10 that removal or > replacing that may make an existing entry unreferenced should free > entries that are no longer used, and "use after free" should be > forbidden. > OK, I'll spend some more time analyzing the call hierarchies to see if there are more uses of removed cache_entries. I'll try to post an updated v4 by the end of the week. Karsten -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html