From f658dc433cae351e72b1c8b245724eafb43f5844 Mon Sep 17 00:00:00 2001
From: Russell Belfer <rb@github.com>
Date: Fri, 31 May 2013 14:09:58 -0700
Subject: [PATCH 1/3] Zero memory for major objects before freeing

By zeroing out the memory when we free larger objects (i.e. those
that serve as collections of other data, such as repos, odb, refdb),
I'm hoping that it will be easier for libgit2 bindings to find
errors in their object management code.
---
 src/cache.c      | 4 ++--
 src/config.c     | 4 +++-
 src/diff.c       | 2 ++
 src/index.c      | 2 ++
 src/odb.c        | 2 ++
 src/pack.c       | 6 +++++-
 src/refdb.c      | 1 +
 src/repository.c | 1 +
 8 files changed, 18 insertions(+), 4 deletions(-)

diff --git a/src/cache.c b/src/cache.c
index dc3af063a..6205ef8a9 100644
--- a/src/cache.c
+++ b/src/cache.c
@@ -66,7 +66,7 @@ void git_cache_dump_stats(git_cache *cache)
 
 int git_cache_init(git_cache *cache)
 {
-	cache->used_memory = 0;
+	memset(cache, 0, sizeof(*cache));
 	cache->map = git_oidmap_alloc();
 	git_mutex_init(&cache->lock);
 	return 0;
@@ -102,9 +102,9 @@ void git_cache_clear(git_cache *cache)
 void git_cache_free(git_cache *cache)
 {
 	git_cache_clear(cache);
-
 	git_oidmap_free(cache->map);
 	git_mutex_free(&cache->lock);
+	memset(cache, 0, sizeof(*cache));
 }
 
 /* Called with lock */
diff --git a/src/config.c b/src/config.c
index 9491d267a..2c4b15540 100644
--- a/src/config.c
+++ b/src/config.c
@@ -40,12 +40,14 @@ static void config_free(git_config *cfg)
 	size_t i;
 	file_internal *internal;
 
-	for(i = 0; i < cfg->files.length; ++i){
+	for (i = 0; i < cfg->files.length; ++i) {
 		internal = git_vector_get(&cfg->files, i);
 		GIT_REFCOUNT_DEC(internal, file_internal_free);
 	}
 
 	git_vector_free(&cfg->files);
+
+	memset(cfg, 0, sizeof(*cfg));
 	git__free(cfg);
 }
 
diff --git a/src/diff.c b/src/diff.c
index b96ff4705..f1d1010b4 100644
--- a/src/diff.c
+++ b/src/diff.c
@@ -463,6 +463,8 @@ static void diff_list_free(git_diff_list *diff)
 
 	git_pathspec_free(&diff->pathspec);
 	git_pool_clear(&diff->pool);
+
+	memset(diff, 0, sizeof(*diff));
 	git__free(diff);
 }
 
diff --git a/src/index.c b/src/index.c
index 45ce2f3d6..abc9495bd 100644
--- a/src/index.c
+++ b/src/index.c
@@ -348,6 +348,8 @@ static void index_free(git_index *index)
 	git_vector_free(&index->reuc);
 
 	git__free(index->index_file_path);
+
+	memset(index, 0, sizeof(*index));
 	git__free(index);
 }
 
diff --git a/src/odb.c b/src/odb.c
index 9f18b5153..246f7d1ea 100644
--- a/src/odb.c
+++ b/src/odb.c
@@ -589,6 +589,8 @@ static void odb_free(git_odb *db)
 
 	git_vector_free(&db->backends);
 	git_cache_free(&db->own_cache);
+
+	memset(db, 0, sizeof(*db));
 	git__free(db);
 }
 
diff --git a/src/pack.c b/src/pack.c
index 417d225f3..a9b835fca 100644
--- a/src/pack.c
+++ b/src/pack.c
@@ -85,13 +85,17 @@ static void cache_free(git_pack_cache *cache)
 		git_offmap_free(cache->entries);
 		git_mutex_free(&cache->lock);
 	}
+
+	memset(cache, 0, sizeof(*cache));
 }
 
 static int cache_init(git_pack_cache *cache)
 {
-	memset(cache, 0, sizeof(git_pack_cache));
+	memset(cache, 0, sizeof(*cache));
+
 	cache->entries = git_offmap_alloc();
 	GITERR_CHECK_ALLOC(cache->entries);
+
 	cache->memory_limit = GIT_PACK_CACHE_MEMORY_LIMIT;
 	git_mutex_init(&cache->lock);
 
diff --git a/src/refdb.c b/src/refdb.c
index 9f9037ce7..02244c908 100644
--- a/src/refdb.c
+++ b/src/refdb.c
@@ -89,6 +89,7 @@ int git_refdb_compress(git_refdb *db)
 static void refdb_free(git_refdb *db)
 {
 	refdb_free_backend(db);
+	memset(db, 0, sizeof(*db));
 	git__free(db);
 }
 
diff --git a/src/repository.c b/src/repository.c
index 28505e822..8b16f00a4 100644
--- a/src/repository.c
+++ b/src/repository.c
@@ -113,6 +113,7 @@ void git_repository_free(git_repository *repo)
 	git__free(repo->workdir);
 	git__free(repo->namespace);
 
+	memset(repo, 0, sizeof(*repo));
 	git__free(repo);
 }
 

From 1a42dd17eb2c35fa572418f5958595cbe297d229 Mon Sep 17 00:00:00 2001
From: Russell Belfer <rb@github.com>
Date: Fri, 31 May 2013 14:13:11 -0700
Subject: [PATCH 2/3] Mutex init can fail

It is obviously quite a serious problem if this happens, but mutex
initialization can fail and we should detect it.  It's a bit like
a memory allocation failure, in that you're probably pretty screwed
if this occurs, but at least we'll catch it.
---
 src/cache.c        |  5 ++++-
 src/global.c       |  6 ++++--
 src/pack-objects.c |  3 +++
 src/pack.c         | 16 ++++++++++++++--
 src/thread-utils.h |  2 +-
 5 files changed, 26 insertions(+), 6 deletions(-)

diff --git a/src/cache.c b/src/cache.c
index 6205ef8a9..afc7c5b3a 100644
--- a/src/cache.c
+++ b/src/cache.c
@@ -68,7 +68,10 @@ int git_cache_init(git_cache *cache)
 {
 	memset(cache, 0, sizeof(*cache));
 	cache->map = git_oidmap_alloc();
-	git_mutex_init(&cache->lock);
+	if (git_mutex_init(&cache->lock)) {
+		giterr_set(GITERR_OS, "Failed to initialize cache mutex");
+		return -1;
+	}
 	return 0;
 }
 
diff --git a/src/global.c b/src/global.c
index a0571d127..2d40ca2fc 100644
--- a/src/global.c
+++ b/src/global.c
@@ -61,7 +61,8 @@ int git_threads_init(void)
 		return 0;
 
 	_tls_index = TlsAlloc();
-	git_mutex_init(&git__mwindow_mutex);
+	if (git_mutex_init(&git__mwindow_mutex))
+		return -1;
 
 	/* Initialize any other subsystems that have global state */
 	if ((error = git_hash_global_init()) >= 0)
@@ -121,7 +122,8 @@ int git_threads_init(void)
 	if (_tls_init)
 		return 0;
 
-	git_mutex_init(&git__mwindow_mutex);
+	if (git_mutex_init(&git__mwindow_mutex))
+		return -1;
 	pthread_key_create(&_tls_key, &cb__free_status);
 
 	/* Initialize any other subsystems that have global state */
diff --git a/src/pack-objects.c b/src/pack-objects.c
index 3d382026e..500104c55 100644
--- a/src/pack-objects.c
+++ b/src/pack-objects.c
@@ -132,7 +132,10 @@ int git_packbuilder_new(git_packbuilder **out, git_repository *repo)
 	if (git_mutex_init(&pb->cache_mutex) ||
 		git_mutex_init(&pb->progress_mutex) ||
 		git_cond_init(&pb->progress_cond))
+	{
+		giterr_set(GITERR_OS, "Failed to initialize packbuilder mutex");
 		goto on_error;
+	}
 
 #endif
 
diff --git a/src/pack.c b/src/pack.c
index a9b835fca..7ce7099e0 100644
--- a/src/pack.c
+++ b/src/pack.c
@@ -97,7 +97,15 @@ static int cache_init(git_pack_cache *cache)
 	GITERR_CHECK_ALLOC(cache->entries);
 
 	cache->memory_limit = GIT_PACK_CACHE_MEMORY_LIMIT;
-	git_mutex_init(&cache->lock);
+
+	if (git_mutex_init(&cache->lock)) {
+		giterr_set(GITERR_OS, "Failed to initialize pack cache mutex");
+
+		git__free(cache->entries);
+		cache->entries = NULL;
+
+		return -1;
+	}
 
 	return 0;
 }
@@ -948,7 +956,11 @@ int git_packfile_alloc(struct git_pack_file **pack_out, const char *path)
 	p->mtime = (git_time_t)st.st_mtime;
 	p->index_version = -1;
 
-	git_mutex_init(&p->lock);
+	if (git_mutex_init(&p->lock)) {
+		giterr_set(GITERR_OS, "Failed to initialize packfile mutex");
+		git__free(p);
+		return -1;
+	}
 
 	/* see if we can parse the sha1 oid in the packfile name */
 	if (path_len < 40 ||
diff --git a/src/thread-utils.h b/src/thread-utils.h
index 49b5f3b5e..d4ed7e73b 100644
--- a/src/thread-utils.h
+++ b/src/thread-utils.h
@@ -140,7 +140,7 @@ GIT_INLINE(int64_t) git_atomic64_add(git_atomic64 *a, int64_t addend)
 
 /* Pthreads Mutex */
 #define git_mutex unsigned int
-#define git_mutex_init(a) (void)0
+#define git_mutex_init(a) 0
 #define git_mutex_lock(a) 0
 #define git_mutex_unlock(a) (void)0
 #define git_mutex_free(a) (void)0

From 3e9e6cdaff8acb11399736abbf793bf2d000d037 Mon Sep 17 00:00:00 2001
From: Russell Belfer <rb@github.com>
Date: Fri, 7 Jun 2013 09:54:33 -0700
Subject: [PATCH 3/3] Add safe memset and use it

This adds a `git__memset` routine that will not be optimized away
and updates the places where I memset() right before a free() call
to use it.
---
 src/cache.c      |  2 +-
 src/config.c     |  2 +-
 src/diff.c       |  2 +-
 src/index.c      |  2 +-
 src/odb.c        |  2 +-
 src/refdb.c      |  2 +-
 src/repository.c |  6 ++----
 src/util.c       | 10 ++++++++++
 src/util.h       | 11 ++++++++---
 9 files changed, 26 insertions(+), 13 deletions(-)

diff --git a/src/cache.c b/src/cache.c
index afc7c5b3a..570838e44 100644
--- a/src/cache.c
+++ b/src/cache.c
@@ -107,7 +107,7 @@ void git_cache_free(git_cache *cache)
 	git_cache_clear(cache);
 	git_oidmap_free(cache->map);
 	git_mutex_free(&cache->lock);
-	memset(cache, 0, sizeof(*cache));
+	git__memset(cache, 0, sizeof(*cache));
 }
 
 /* Called with lock */
diff --git a/src/config.c b/src/config.c
index 2c4b15540..75cbe348c 100644
--- a/src/config.c
+++ b/src/config.c
@@ -47,7 +47,7 @@ static void config_free(git_config *cfg)
 
 	git_vector_free(&cfg->files);
 
-	memset(cfg, 0, sizeof(*cfg));
+	git__memset(cfg, 0, sizeof(*cfg));
 	git__free(cfg);
 }
 
diff --git a/src/diff.c b/src/diff.c
index f1d1010b4..982d64051 100644
--- a/src/diff.c
+++ b/src/diff.c
@@ -464,7 +464,7 @@ static void diff_list_free(git_diff_list *diff)
 	git_pathspec_free(&diff->pathspec);
 	git_pool_clear(&diff->pool);
 
-	memset(diff, 0, sizeof(*diff));
+	git__memset(diff, 0, sizeof(*diff));
 	git__free(diff);
 }
 
diff --git a/src/index.c b/src/index.c
index abc9495bd..2bb7d6ee6 100644
--- a/src/index.c
+++ b/src/index.c
@@ -349,7 +349,7 @@ static void index_free(git_index *index)
 
 	git__free(index->index_file_path);
 
-	memset(index, 0, sizeof(*index));
+	git__memset(index, 0, sizeof(*index));
 	git__free(index);
 }
 
diff --git a/src/odb.c b/src/odb.c
index 246f7d1ea..5e27edacd 100644
--- a/src/odb.c
+++ b/src/odb.c
@@ -590,7 +590,7 @@ static void odb_free(git_odb *db)
 	git_vector_free(&db->backends);
 	git_cache_free(&db->own_cache);
 
-	memset(db, 0, sizeof(*db));
+	git__memset(db, 0, sizeof(*db));
 	git__free(db);
 }
 
diff --git a/src/refdb.c b/src/refdb.c
index 02244c908..4271b58e4 100644
--- a/src/refdb.c
+++ b/src/refdb.c
@@ -89,7 +89,7 @@ int git_refdb_compress(git_refdb *db)
 static void refdb_free(git_refdb *db)
 {
 	refdb_free_backend(db);
-	memset(db, 0, sizeof(*db));
+	git__memset(db, 0, sizeof(*db));
 	git__free(db);
 }
 
diff --git a/src/repository.c b/src/repository.c
index 8b16f00a4..ee6c5bad4 100644
--- a/src/repository.c
+++ b/src/repository.c
@@ -113,7 +113,7 @@ void git_repository_free(git_repository *repo)
 	git__free(repo->workdir);
 	git__free(repo->namespace);
 
-	memset(repo, 0, sizeof(*repo));
+	git__memset(repo, 0, sizeof(*repo));
 	git__free(repo);
 }
 
@@ -140,12 +140,10 @@ static bool valid_repository_path(git_buf *repository_path)
 
 static git_repository *repository_alloc(void)
 {
-	git_repository *repo = git__malloc(sizeof(git_repository));
+	git_repository *repo = git__calloc(1, sizeof(git_repository));
 	if (!repo)
 		return NULL;
 
-	memset(repo, 0x0, sizeof(git_repository));
-
 	if (git_cache_init(&repo->objects) < 0) {
 		git__free(repo);
 		return NULL;
diff --git a/src/util.c b/src/util.c
index da15a039d..248cf4c42 100644
--- a/src/util.c
+++ b/src/util.c
@@ -722,3 +722,13 @@ void git__insertsort_r(
 	if (freeswap)
 		git__free(swapel);
 }
+
+void git__memset(void *data, int c, size_t size)
+{
+	volatile uint8_t *scan = data;
+	uint8_t *end = scan + size;
+	uint8_t val = (uint8_t)c;
+
+	while (scan < end)
+		*scan++ = val;
+}
diff --git a/src/util.h b/src/util.h
index 5ae87ac10..fd3ea22ed 100644
--- a/src/util.h
+++ b/src/util.h
@@ -293,8 +293,7 @@ GIT_INLINE(bool) git__iswildcard(int c)
 }
 
 /*
- * Parse a string value as a boolean, just like Core Git
- * does.
+ * Parse a string value as a boolean, just like Core Git does.
  *
  * Valid values for true are: 'true', 'yes', 'on'
  * Valid values for false are: 'false', 'no', 'off'
@@ -309,7 +308,7 @@ extern int git__parse_bool(int *out, const char *value);
  * - "July 17, 2003"
  * - "2003-7-17 08:23"
  */
-int git__date_parse(git_time_t *out, const char *date);
+extern int git__date_parse(git_time_t *out, const char *date);
 
 /*
  * Unescapes a string in-place.
@@ -320,4 +319,10 @@ int git__date_parse(git_time_t *out, const char *date);
  */
 extern size_t git__unescape(char *str);
 
+/*
+ * Memset that will not be optimized away by the compiler.
+ * You usually should just use regular `memset()`.
+ */
+extern void git__memset(void *data, int c, size_t size);
+
 #endif /* INCLUDE_util_h__ */