linear-assignment: fix potential out of bounds memory access

diff --git a/split-index.c b/split-index.c
index 660c75f31fb970af06af3aad48b4ecc57e8cc136..84f067e10d2cec1d77ec1a51f0f38cc5a29dd89f 100644 (file)
--- a/split-index.c
+++ b/split-index.c
@@ -73,16 +73,31 @@ void move_cache_to_base_index(struct index_state *istate)
        int i;
 
        /*
-        * do not delete old si->base, its index entries may be shared
-        * with istate->cache[]. Accept a bit of leaking here because
-        * this code is only used by short-lived update-index.
+        * If there was a previous base index, then transfer ownership of allocated
+        * entries to the parent index.
         */
+       if (si->base &&
+               si->base->ce_mem_pool) {
+
+               if (!istate->ce_mem_pool)
+                       mem_pool_init(&istate->ce_mem_pool, 0);
+
+               mem_pool_combine(istate->ce_mem_pool, istate->split_index->base->ce_mem_pool);
+       }
+
        si->base = xcalloc(1, sizeof(*si->base));
        si->base->version = istate->version;
        /* zero timestamp disables racy test in ce_write_index() */
        si->base->timestamp = istate->timestamp;
        ALLOC_GROW(si->base->cache, istate->cache_nr, si->base->cache_alloc);
        si->base->cache_nr = istate->cache_nr;
+
+       /*
+        * The mem_pool needs to move with the allocated entries.
+        */
+       si->base->ce_mem_pool = istate->ce_mem_pool;
+       istate->ce_mem_pool = NULL;
+
        COPY_ARRAY(si->base->cache, istate->cache, istate->cache_nr);
        mark_base_index_entries(si->base);
        for (i = 0; i < si->base->cache_nr; i++)
@@ -123,7 +138,7 @@ static void replace_entry(size_t pos, void *data)
        src->ce_flags |= CE_UPDATE_IN_BASE;
        src->ce_namelen = dst->ce_namelen;
        copy_cache_entry(dst, src);
-       free(src);
+       discard_cache_entry(src);
        si->nr_replacements++;
 }
 
@@ -224,7 +239,7 @@ void prepare_to_write_split_index(struct index_state *istate)
                        base->ce_flags = base_flags;
                        if (ret)
                                ce->ce_flags |= CE_UPDATE_IN_BASE;
-                       free(base);
+                       discard_cache_entry(base);
                        si->base->cache[ce->index - 1] = ce;
                }
                for (i = 0; i < si->base->cache_nr; i++) {
@@ -301,7 +316,7 @@ void save_or_free_index_entry(struct index_state *istate, struct cache_entry *ce
            ce == istate->split_index->base->cache[ce->index - 1])
                ce->ce_flags |= CE_REMOVE;
        else
-               free(ce);
+               discard_cache_entry(ce);
 }
 
 void replace_index_entry_in_base(struct index_state *istate,
@@ -314,7 +329,7 @@ void replace_index_entry_in_base(struct index_state *istate,
            old_entry->index <= istate->split_index->base->cache_nr) {
                new_entry->index = old_entry->index;
                if (old_entry != istate->split_index->base->cache[new_entry->index - 1])
-                       free(istate->split_index->base->cache[new_entry->index - 1]);
+                       discard_cache_entry(istate->split_index->base->cache[new_entry->index - 1]);
                istate->split_index->base->cache[new_entry->index - 1] = new_entry;
        }
 }
@@ -331,12 +346,31 @@ void remove_split_index(struct index_state *istate)
 {
        if (istate->split_index) {
                /*
-                * can't discard_split_index(&the_index); because that
-                * will destroy split_index->base->cache[], which may
-                * be shared with the_index.cache[]. So yeah we're
-                * leaking a bit here.
+                * 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[].
                 */
-               istate->split_index = NULL;
+               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)
+                       istate->split_index->base->cache_nr = 0;
+
+               /*
+                * We can discard the split index because its
+                * memory pool has been incorporated into the
+                * memory pool associated with the the_index.
+                */
+               discard_split_index(istate);
+
                istate->cache_changed |= SOMETHING_CHANGED;
        }
 }