block alloc: add validations around cache_entry lifecyle
Add an option (controlled by an environment variable) perform extra validations on mem_pool allocated cache entries. When set: 1) Invalidate cache_entry memory when discarding cache_entry. 2) When discarding index_state struct, verify that all cache_entries were allocated from expected mem_pool. 3) When discarding mem_pools, invalidate mem_pool memory. This should provide extra checks that mem_pools and their allocated cache_entries are being used as expected. Signed-off-by: Jameson Miller <jamill@microsoft.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Jameson Miller committed
Jul 2, 2018 at 19:49 UTC
8616a2d0cb57865540f1c00ac2e5385a6cc5d84e
5 files changed
+68
-4
cache.h
+6
@@ -380,6 +380,12 @@ struct cache_entry *make_empty_transient_cache_entry(size_t name_len);
380
*/
381
void discard_cache_entry(struct cache_entry *ce);
382
383
+/*
384
+ * Check configuration if we should perform extra validation on cache
385
+ * entries.
386
+ */
387
+int should_validate_cache_entries(void);
388
+
389
/*
390
* Duplicate a cache_entry. Allocate memory for the new entry from a
391
* memory_pool. Takes into account cache_entry fields that are meant
git.c
+3
@@ -414,7 +414,10 @@ static int run_builtin(struct cmd_struct *p, int argc, const char **argv)
414
415
trace_argv_printf(argv, "trace: built-in: git");
416
417
+ validate_cache_entries(&the_index);
418
status = p->fn(argc, argv, prefix);
419
+ validate_cache_entries(&the_index);
420
+
421
if (status)
422
return status;
423
mem-pool.c
+5
-1
@@ -50,7 +50,7 @@ void mem_pool_init(struct mem_pool **mem_pool, size_t initial_size)
50
*mem_pool = pool;
51
}
52
53
-void mem_pool_discard(struct mem_pool *mem_pool)
53
+void mem_pool_discard(struct mem_pool *mem_pool, int invalidate_memory)
54
{
55
struct mp_block *block, *block_to_free;
56
@@ -59,6 +59,10 @@ void mem_pool_discard(struct mem_pool *mem_pool)
59
{
60
block_to_free = block;
61
block = block->next_block;
62
+
63
+ if (invalidate_memory)
64
+ memset(block_to_free->space, 0xDD, ((char *)block_to_free->end) - ((char *)block_to_free->space));
65
+
66
free(block_to_free);
67
}
68
mem-pool.h
+1
-1
@@ -29,7 +29,7 @@ void mem_pool_init(struct mem_pool **mem_pool, size_t initial_size);
29
/*
30
* Discard a memory pool and free all the memory it is responsible for.
31
*/
32
-void mem_pool_discard(struct mem_pool *mem_pool);
32
+void mem_pool_discard(struct mem_pool *mem_pool, int invalidate_memory);
33
34
/*
35
* Alloc memory from the mem_pool.
read-cache.c
+53
-2
@@ -2050,8 +2050,10 @@ int discard_index(struct index_state *istate)
2050
* Cache entries in istate->cache[] should have been allocated
2051
* from the memory pool associated with this index, or from an
2052
* associated split_index. There is no need to free individual
2053
- * cache entries.
2053
+ * cache entries. validate_cache_entries can detect when this
2054
+ * assertion does not hold.
2055
*/
2056
+ validate_cache_entries(istate);
2057
2058
resolve_undo_clear_index(istate);
2059
istate->cache_nr = 0;
@@ -2068,13 +2070,45 @@ int discard_index(struct index_state *istate)
2070
istate->untracked = NULL;
2071
2072
if (istate->ce_mem_pool) {
2071
- mem_pool_discard(istate->ce_mem_pool);
2073
+ mem_pool_discard(istate->ce_mem_pool, should_validate_cache_entries());
2074
istate->ce_mem_pool = NULL;
2075
}
2076
2077
return 0;
2078
}
2079
2080
+/*
2081
+ * Validate the cache entries of this index.
2082
+ * All cache entries associated with this index
2083
+ * should have been allocated by the memory pool
2084
+ * associated with this index, or by a referenced
2085
+ * split index.
2086
+ */
2087
+void validate_cache_entries(const struct index_state *istate)
2088
+{
2089
+ int i;
2090
+
2091
+ if (!should_validate_cache_entries() ||!istate || !istate->initialized)
2092
+ return;
2093
+
2094
+ for (i = 0; i < istate->cache_nr; i++) {
2095
+ if (!istate) {
2096
+ die("internal error: cache entry is not allocated from expected memory pool");
2097
+ } else if (!istate->ce_mem_pool ||
2098
+ !mem_pool_contains(istate->ce_mem_pool, istate->cache[i])) {
2099
+ if (!istate->split_index ||
2100
+ !istate->split_index->base ||
2101
+ !istate->split_index->base->ce_mem_pool ||
2102
+ !mem_pool_contains(istate->split_index->base->ce_mem_pool, istate->cache[i])) {
2103
+ die("internal error: cache entry is not allocated from expected memory pool");
2104
+ }
2105
+ }
2106
+ }
2107
+
2108
+ if (istate->split_index)
2109
+ validate_cache_entries(istate->split_index->base);
2110
+}
2111
+
2112
int unmerged_index(const struct index_state *istate)
2113
{
2114
int i;
@@ -2878,8 +2912,25 @@ struct cache_entry *dup_cache_entry(const struct cache_entry *ce,
2912
2913
void discard_cache_entry(struct cache_entry *ce)
2914
{
2915
+ if (ce && should_validate_cache_entries())
2916
+ memset(ce, 0xCD, cache_entry_size(ce->ce_namelen));
2917
+
2918
if (ce && ce->mem_pool_allocated)
2919
return;
2920
2921
free(ce);
2922
}
2923
+
2924
+int should_validate_cache_entries(void)
2925
+{
2926
+ static int validate_index_cache_entries = -1;
2927
+
2928
+ if (validate_index_cache_entries < 0) {
2929
+ if (getenv("GIT_TEST_VALIDATE_INDEX_CACHE_ENTRIES"))
2930
+ validate_index_cache_entries = 1;
2931
+ else
2932
+ validate_index_cache_entries = 0;
2933
+ }
2934
+
2935
+ return validate_index_cache_entries;
2936
+}