From 6e37c8ed3c899385651f5beac1f1588fe3c1f5fb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nguy=E1=BB=85n=20Th=C3=A1i=20Ng=E1=BB=8Dc=20Duy?= Date: Wed, 13 Feb 2019 16:51:29 +0700 Subject: read-cache.c: fix writing "link" index ext with null base oid MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Since commit 7db118303a (unpack_trees: fix breakage when o->src_index != o->dst_index - 2018-04-23) and changes in merge code to use separate index_state for source and destination, when doing a merge with split index activated, we may run into this line in unpack_trees(): o->result.split_index = init_split_index(&o->result); This is by itself not wrong. But this split index information is not fully populated (and it's only so when move_cache_to_base_index() is called, aka force splitting the index, or loading index_state from a file). Both "base_oid" and "base" in this case remain null. So when writing the main index down, we link to this index with null oid (default value after init_split_index()), which also means "no split index" internally. This triggers an incorrect base index refresh: warning: could not freshen shared index '.../sharedindex.0{40}' This patch makes sure we will not refresh null base_oid (because the file is never there). It also makes sure not to write "link" extension with null base_oid in the first place (no point having it at all). Read code already has protection against null base_oid. There is also another side fix in remove_split_index() that causes a crash when doing "git update-index --no-split-index" when base_oid in the index file is null. In this case we will not load istate->split_index->base but we dereference it anyway and are rewarded with a segfault. This should not happen anymore, but it's still wrong to dereference a potential NULL pointer, especially when we do check for NULL pointer in the next code. Reported-by: Luke Diamand Signed-off-by: Nguyễn Thái Ngọc Duy Signed-off-by: Junio C Hamano --- split-index.c | 34 ++++++++++++++++++---------------- 1 file changed, 18 insertions(+), 16 deletions(-) (limited to 'split-index.c') diff --git a/split-index.c b/split-index.c index 5820412dc5..a9d13611a4 100644 --- a/split-index.c +++ b/split-index.c @@ -440,24 +440,26 @@ void add_split_index(struct index_state *istate) void remove_split_index(struct index_state *istate) { if (istate->split_index) { - /* - * When removing the split index, we need to move - * ownership of the mem_pool associated with the - * base index to the main index. There may be cache entries - * allocated from the base's memory pool that are shared with - * the_index.cache[]. - */ - mem_pool_combine(istate->ce_mem_pool, istate->split_index->base->ce_mem_pool); + if (istate->split_index->base) { + /* + * When removing the split index, we need to move + * ownership of the mem_pool associated with the + * base index to the main index. There may be cache entries + * allocated from the base's memory pool that are shared with + * the_index.cache[]. + */ + mem_pool_combine(istate->ce_mem_pool, + istate->split_index->base->ce_mem_pool); - /* - * The split index no longer owns the mem_pool backing - * its cache array. As we are discarding this index, - * mark the index as having no cache entries, so it - * will not attempt to clean up the cache entries or - * validate them. - */ - if (istate->split_index->base) + /* + * The split index no longer owns the mem_pool backing + * its cache array. As we are discarding this index, + * mark the index as having no cache entries, so it + * will not attempt to clean up the cache entries or + * validate them. + */ istate->split_index->base->cache_nr = 0; + } /* * We can discard the split index because its -- cgit v1.2.3