config_file: keep reference to config entries when creating iterator

When creating a configuration file iterator, then we first refresh the
backend and then afterwards duplicate all refreshed configuration
entries into the iterator in order to avoid seeing any concurrent
modifications of the entries while iterating. The duplication of entries
is not guarded, though, as we do not increase the refcount of the
entries that we duplicate right now. This opens us up for a race, as
another thread may concurrently refresh the repository configuration and
thus swap out the current set of entries. As we didn't increase the
refcount, this may lead to the entries being free'd while we iterate
over them in the first thread.

Fix the issue by properly handling the lifecycle of the backend's
entries via `config_file_entries_take` and `git_config_entries_free`,
respectively.

1 files changed, 5 insertions, 3 deletions

diff --git a/src/config_file.c b/src/config_file.c
index 4d1bcde7b..c9e36493e 100644
--- a/src/config_file.c
+++ b/src/config_file.c
@@ -250,17 +250,19 @@ static int config_file_iterator(
 	struct git_config_backend *backend)
 {
 	config_file_backend *b = GIT_CONTAINER_OF(backend, config_file_backend, parent);
-	git_config_entries *entries = NULL;
+	git_config_entries *dupped = NULL, *entries = NULL;
 	int error;
 
 	if ((error = config_file_refresh(backend)) < 0 ||
-	    (error = git_config_entries_dup(&entries, b->entries)) < 0 ||
-	    (error = git_config_entries_iterator_new(iter, entries)) < 0)
+	    (error = config_file_entries_take(&entries, b)) < 0 ||
+	    (error = git_config_entries_dup(&dupped, entries)) < 0 ||
+	    (error = git_config_entries_iterator_new(iter, dupped)) < 0)
 		goto out;
 
 out:
 	/* Let iterator delete duplicated entries when it's done */
 	git_config_entries_free(entries);
+	git_config_entries_free(dupped);
 	return error;
 }