From 13019979b811d26f4838d09331c7ddd8223d270d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20=C3=85gren?= Date: Tue, 22 Jan 2019 22:45:47 +0100 Subject: setup: free old value before setting `work_tree` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Before assigning to `data->work_tree` in `read_worktree_config()`, free any value we might already have picked up, so that we do not leak it. Signed-off-by: Martin Ågren Signed-off-by: Junio C Hamano --- setup.c | 1 + 1 file changed, 1 insertion(+) (limited to 'setup.c') diff --git a/setup.c b/setup.c index 1be5037f12..bb633942bb 100644 --- a/setup.c +++ b/setup.c @@ -411,6 +411,7 @@ static int read_worktree_config(const char *var, const char *value, void *vdata) } else if (strcmp(var, "core.worktree") == 0) { if (!value) return config_error_nonbool(var); + free(data->work_tree); data->work_tree = xstrdup(value); } return 0; -- cgit v1.2.1 From e8805af1c33d79750a979014c021cd63d780c720 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20=C3=85gren?= Date: Thu, 28 Feb 2019 21:36:28 +0100 Subject: setup: fix memory leaks with `struct repository_format` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit After we set up a `struct repository_format`, it owns various pieces of allocated memory. We then either use those members, because we decide we want to use the "candidate" repository format, or we discard the candidate / scratch space. In the first case, we transfer ownership of the memory to a few global variables. In the latter case, we just silently drop the struct and end up leaking memory. Introduce an initialization macro `REPOSITORY_FORMAT_INIT` and a function `clear_repository_format()`, to be used on each side of `read_repository_format()`. To have a clear and simple memory ownership, let all users of `struct repository_format` duplicate the strings that they take from it, rather than stealing the pointers. Call `clear_...()` at the start of `read_...()` instead of just zeroing the struct, since we sometimes enter the function multiple times. Thus, it is important to initialize the struct before calling `read_...()`, so document that. It's also important because we might not even call `read_...()` before we call `clear_...()`, see, e.g., builtin/init-db.c. Teach `read_...()` to clear the struct on error, so that it is reset to a safe state, and document this. (In `setup_git_directory_gently()`, we look at `repo_fmt.hash_algo` even if `repo_fmt.version` is -1, which we weren't actually supposed to do per the API. After this commit, that's ok.) We inherit the existing code's combining "error" and "no version found". Both are signalled through `version == -1` and now both cause us to clear any partial configuration we have picked up. For "extensions.*", that's fine, since they require a positive version number. For "core.bare" and "core.worktree", we're already verifying that we have a non-negative version number before using them. Signed-off-by: Martin Ågren Signed-off-by: Junio C Hamano --- setup.c | 39 +++++++++++++++++++++++++++------------ 1 file changed, 27 insertions(+), 12 deletions(-) (limited to 'setup.c') diff --git a/setup.c b/setup.c index bb633942bb..ab6e8fd4c3 100644 --- a/setup.c +++ b/setup.c @@ -477,7 +477,7 @@ static int check_repository_format_gently(const char *gitdir, struct repository_ } repository_format_precious_objects = candidate->precious_objects; - repository_format_partial_clone = candidate->partial_clone; + repository_format_partial_clone = xstrdup_or_null(candidate->partial_clone); repository_format_worktree_config = candidate->worktree_config; string_list_clear(&candidate->unknown_extensions, 0); @@ -500,27 +500,38 @@ static int check_repository_format_gently(const char *gitdir, struct repository_ } if (candidate->work_tree) { free(git_work_tree_cfg); - git_work_tree_cfg = candidate->work_tree; + git_work_tree_cfg = xstrdup(candidate->work_tree); inside_work_tree = -1; } - } else { - free(candidate->work_tree); } return 0; } +static void init_repository_format(struct repository_format *format) +{ + const struct repository_format fresh = REPOSITORY_FORMAT_INIT; + + memcpy(format, &fresh, sizeof(fresh)); +} + int read_repository_format(struct repository_format *format, const char *path) { - memset(format, 0, sizeof(*format)); - format->version = -1; - format->is_bare = -1; - format->hash_algo = GIT_HASH_SHA1; - string_list_init(&format->unknown_extensions, 1); + clear_repository_format(format); git_config_from_file(check_repo_format, path, format); + if (format->version == -1) + clear_repository_format(format); return format->version; } +void clear_repository_format(struct repository_format *format) +{ + string_list_clear(&format->unknown_extensions, 0); + free(format->work_tree); + free(format->partial_clone); + init_repository_format(format); +} + int verify_repository_format(const struct repository_format *format, struct strbuf *err) { @@ -1008,7 +1019,7 @@ int discover_git_directory(struct strbuf *commondir, struct strbuf dir = STRBUF_INIT, err = STRBUF_INIT; size_t gitdir_offset = gitdir->len, cwd_len; size_t commondir_offset = commondir->len; - struct repository_format candidate; + struct repository_format candidate = REPOSITORY_FORMAT_INIT; if (strbuf_getcwd(&dir)) return -1; @@ -1045,9 +1056,11 @@ int discover_git_directory(struct strbuf *commondir, strbuf_release(&err); strbuf_setlen(commondir, commondir_offset); strbuf_setlen(gitdir, gitdir_offset); + clear_repository_format(&candidate); return -1; } + clear_repository_format(&candidate); return 0; } @@ -1056,7 +1069,7 @@ const char *setup_git_directory_gently(int *nongit_ok) static struct strbuf cwd = STRBUF_INIT; struct strbuf dir = STRBUF_INIT, gitdir = STRBUF_INIT; const char *prefix; - struct repository_format repo_fmt; + struct repository_format repo_fmt = REPOSITORY_FORMAT_INIT; /* * We may have read an incomplete configuration before @@ -1146,6 +1159,7 @@ const char *setup_git_directory_gently(int *nongit_ok) strbuf_release(&dir); strbuf_release(&gitdir); + clear_repository_format(&repo_fmt); return prefix; } @@ -1203,9 +1217,10 @@ int git_config_perm(const char *var, const char *value) void check_repository_format(void) { - struct repository_format repo_fmt; + struct repository_format repo_fmt = REPOSITORY_FORMAT_INIT; check_repository_format_gently(get_git_dir(), &repo_fmt, NULL); startup_info->have_repository = 1; + clear_repository_format(&repo_fmt); } /* -- cgit v1.2.1