Merge pull request #2212 from libgit2/rb/submodule-use-after-free Fix use-after-free in submodule reload code and other memory leaks
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
diff --git a/src/submodule.c b/src/submodule.c
index b07c9d9..e1500b8 100644
--- a/src/submodule.c
+++ b/src/submodule.c
@@ -852,10 +852,13 @@ int git_submodule_reload_all(git_repository *repo, int force)
git_strmap_foreach_value(repo->submodules, sm, {
git_strmap *cache = repo->submodules;
- if ((sm->flags & GIT_SUBMODULE_STATUS__IN_FLAGS) == 0) {
- submodule_cache_remove_item(cache, sm->name, sm, true);
+ if (sm && (sm->flags & GIT_SUBMODULE_STATUS__IN_FLAGS) == 0) {
+ /* we must check path != name before first remove, in case
+ * that call frees the submodule */
+ bool free_as_path = (sm->path != sm->name);
- if (sm->path != sm->name)
+ submodule_cache_remove_item(cache, sm->name, sm, true);
+ if (free_as_path)
submodule_cache_remove_item(cache, sm->path, sm, true);
}
});
@@ -1123,6 +1126,7 @@ static void submodule_release(git_submodule *sm)
git__free(sm->path);
git__free(sm->name);
git__free(sm->url);
+ git__free(sm->branch);
git__memzero(sm, sizeof(*sm));
git__free(sm);
}
diff --git a/tests/diff/submodules.c b/tests/diff/submodules.c
index 80dfcaa..ead5c71 100644
--- a/tests/diff/submodules.c
+++ b/tests/diff/submodules.c
@@ -449,7 +449,6 @@ void test_diff_submodules__skips_empty_includes_used(void)
git_diff_options opts = GIT_DIFF_OPTIONS_INIT;
git_diff *diff = NULL;
diff_expects exp;
- git_repository *r2;
/* A side effect of of Git's handling of untracked directories and
* auto-ignoring of ".git" entries is that a newly initialized Git
@@ -469,7 +468,11 @@ void test_diff_submodules__skips_empty_includes_used(void)
cl_assert_equal_i(0, exp.files);
git_diff_free(diff);
- cl_git_pass(git_repository_init(&r2, "empty_standard_repo/subrepo", 0));
+ {
+ git_repository *r2;
+ cl_git_pass(git_repository_init(&r2, "empty_standard_repo/subrepo", 0));
+ git_repository_free(r2);
+ }
cl_git_pass(git_diff_index_to_workdir(&diff, g_repo, NULL, &opts));
memset(&exp, 0, sizeof(exp));