refs: Fix double free Includes relevant Clay test
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 48 49 50 51 52 53 54 55 56 57 58 59 60 61 62 63 64 65 66 67 68 69 70 71 72 73 74 75 76 77 78 79 80 81 82 83 84 85 86 87 88 89 90 91 92 93 94 95 96 97 98 99 100 101 102 103 104 105 106 107 108 109 110 111 112 113 114 115 116 117 118 119 120 121 122 123 124 125 126 127 128 129 130 131 132 133 134 135 136 137 138 139 140 141 142 143 144 145 146 147
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));
+}