Commit fa51565625902291f278d41c9254f8ab38bd916d

Vicent Martí 2011-12-25T00:22:20

refs: Fix double free Includes relevant Clay test

diff --git a/src/refs.c b/src/refs.c
index 8c3f700..4950fd5 100644
--- a/src/refs.c
+++ b/src/refs.c
@@ -98,7 +98,7 @@ void git_reference_free(git_reference *reference)
 	git__free(reference);
 }
 
-static int reference_create(
+static int reference_alloc(
 	git_reference **ref_out,
 	git_repository *repo,
 	const char *name)
@@ -232,8 +232,10 @@ static int loose_lookup(git_reference *ref)
 	if (!updated)
 		return GIT_SUCCESS;
 
-	if (ref->flags & GIT_REF_SYMBOLIC)
+	if (ref->flags & GIT_REF_SYMBOLIC) {
 		free(ref->target.symbolic);
+		ref->target.symbolic = NULL;
+	}
 
 	ref->flags = 0;
 
@@ -939,8 +941,10 @@ static int packed_lookup(git_reference *ref)
 		ref->mtime == ref->owner->references.packfile_time)
 		return GIT_SUCCESS;
 
-	if (ref->flags & GIT_REF_SYMBOLIC)
+	if (ref->flags & GIT_REF_SYMBOLIC) {
 		free(ref->target.symbolic);
+		ref->target.symbolic = NULL;
+	}
 
 	/* Look up on the packfile */
 	pack_ref = git_hashtable_lookup(ref->owner->references.packfile, ref->name);
@@ -1059,7 +1063,7 @@ int git_reference_lookup(git_reference **ref_out,
 	if (error < GIT_SUCCESS)
 		return git__rethrow(error, "Failed to lookup reference");
 
-	error = reference_create(&ref, repo, normalized_name);
+	error = reference_alloc(&ref, repo, normalized_name);
 	if (error < GIT_SUCCESS)
 		return git__rethrow(error, "Failed to lookup reference");
 
@@ -1147,7 +1151,7 @@ int git_reference_create_symbolic(
 		return git__throw(GIT_EEXISTS,
 			"Failed to create symbolic reference. Reference already exists");
 
-	error = reference_create(&ref, repo, normalized);
+	error = reference_alloc(&ref, repo, normalized);
 	if (error < GIT_SUCCESS)
 		goto cleanup;
 
@@ -1197,7 +1201,7 @@ int git_reference_create_oid(
 	if ((error = reference_available(repo, name, NULL)) < GIT_SUCCESS)
 		return git__rethrow(error, "Failed to create reference");
 
-	error = reference_create(&ref, repo, name);
+	error = reference_alloc(&ref, repo, name);
 	if (error < GIT_SUCCESS)
 		goto cleanup;
 
@@ -1590,10 +1594,8 @@ int git_reference_reload(git_reference *ref)
 {
 	int error = reference_lookup(ref);
 
-	if (error < GIT_SUCCESS) {
-		git_reference_free(ref);
+	if (error < GIT_SUCCESS)
 		return git__rethrow(error, "Failed to reload reference");
-	}
 
 	return GIT_SUCCESS;
 }
diff --git a/tests-clay/clay.h b/tests-clay/clay.h
index f580767..c9fe4c1 100644
--- a/tests-clay/clay.h
+++ b/tests-clay/clay.h
@@ -192,6 +192,7 @@ extern void test_odb_sorting__alternate_backends_sorting(void);
 extern void test_odb_sorting__basic_backends_sorting(void);
 extern void test_odb_sorting__cleanup(void);
 extern void test_odb_sorting__initialize(void);
+extern void test_refs_crashes__double_free(void);
 extern void test_repo_getters__cleanup(void);
 extern void test_repo_getters__empty(void);
 extern void test_repo_getters__head_detached(void);
diff --git a/tests-clay/clay_main.c b/tests-clay/clay_main.c
index d2c954e..318e096 100644
--- a/tests-clay/clay_main.c
+++ b/tests-clay/clay_main.c
@@ -281,6 +281,9 @@ static const struct clay_func _clay_cb_odb_sorting[] = {
     {"alternate_backends_sorting", &test_odb_sorting__alternate_backends_sorting},
 	{"basic_backends_sorting", &test_odb_sorting__basic_backends_sorting}
 };
+static const struct clay_func _clay_cb_refs_crashes[] = {
+    {"double_free", &test_refs_crashes__double_free}
+};
 static const struct clay_func _clay_cb_repo_getters[] = {
     {"empty", &test_repo_getters__empty},
 	{"head_detached", &test_repo_getters__head_detached},
@@ -498,6 +501,12 @@ static const struct clay_suite _clay_suites[] = {
         _clay_cb_odb_sorting, 2
     },
 	{
+        "refs::crashes",
+        {NULL, NULL},
+        {NULL, NULL},
+        _clay_cb_refs_crashes, 1
+    },
+	{
         "repo::getters",
         {"initialize", &test_repo_getters__initialize},
         {"cleanup", &test_repo_getters__cleanup},
@@ -529,8 +538,8 @@ static const struct clay_suite _clay_suites[] = {
     }
 };
 
-static size_t _clay_suite_count = 37;
-static size_t _clay_callback_count = 121;
+static size_t _clay_suite_count = 38;
+static size_t _clay_callback_count = 122;
 
 /* Core test functions */
 static void
diff --git a/tests-clay/refs/crashes.c b/tests-clay/refs/crashes.c
new file mode 100644
index 0000000..51eb15d
--- /dev/null
+++ b/tests-clay/refs/crashes.c
@@ -0,0 +1,15 @@
+#include "clay_libgit2.h"
+
+void test_refs_crashes__double_free(void)
+{
+	git_repository *repo;
+	git_reference *ref, *ref2;
+	const char *REFNAME = "refs/heads/xxx";
+
+	cl_git_pass(git_repository_open(&repo, cl_fixture("testrepo.git")));
+	cl_git_pass(git_reference_create_symbolic(&ref, repo, REFNAME, "refs/heads/master", 0));
+	cl_git_pass(git_reference_lookup(&ref2, repo, REFNAME));
+	cl_git_pass(git_reference_delete(ref));
+	/* reference is gone from disk, so reloading it will fail */
+	cl_must_fail(git_reference_reload(ref2));
+}