[PATCH 5/8] git-config: fix regexp memory leaks on error conditions

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



The get_value function has a goto label for cleaning up on
errors, but it only cleans up half of what the function
might allocate. Let's also clean up the key and regexp
variables there.

Note that we need to take special care when compiling the
regex fails to clean it up ourselves, since it is in a
half-constructed state (we would want to free it, but not
regfree it).

Similarly, we fix git_config_parse_key to return NULL when it
fails, not a pointer to some already-freed memory.

Signed-off-by: Jeff King <peff@xxxxxxxx>
---
The diff is annoying in an interesting way: what I actually did was move
the regex cleanup code down, but it shows it as moving the bottom bits
up. I think it is just one of those ambiguous cases where either way is
equally valid and minimal.

 builtin/config.c | 23 +++++++++++++----------
 config.c         |  1 +
 2 files changed, 14 insertions(+), 10 deletions(-)

diff --git a/builtin/config.c b/builtin/config.c
index e660d48..60d36e7 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -195,7 +195,8 @@ static int get_value(const char *key_, const char *regex_)
 		key_regexp = (regex_t*)xmalloc(sizeof(regex_t));
 		if (regcomp(key_regexp, key, REG_EXTENDED)) {
 			fprintf(stderr, "Invalid key pattern: %s\n", key_);
-			free(key);
+			free(key_regexp);
+			key_regexp = NULL;
 			ret = CONFIG_INVALID_PATTERN;
 			goto free_strings;
 		}
@@ -215,6 +216,8 @@ static int get_value(const char *key_, const char *regex_)
 		regexp = (regex_t*)xmalloc(sizeof(regex_t));
 		if (regcomp(regexp, regex_, REG_EXTENDED)) {
 			fprintf(stderr, "Invalid pattern: %s\n", regex_);
+			free(regexp);
+			regexp = NULL;
 			ret = CONFIG_INVALID_PATTERN;
 			goto free_strings;
 		}
@@ -247,6 +250,15 @@ static int get_value(const char *key_, const char *regex_)
 	if (!do_all && !seen && system_wide)
 		git_config_from_file(fn, system_wide, data);
 
+	if (do_all)
+		ret = !seen;
+	else
+		ret = (seen == 1) ? 0 : seen > 1 ? 2 : 1;
+
+free_strings:
+	free(repo_config);
+	free(global);
+	free(xdg);
 	free(key);
 	if (key_regexp) {
 		regfree(key_regexp);
@@ -257,15 +269,6 @@ static int get_value(const char *key_, const char *regex_)
 		free(regexp);
 	}
 
-	if (do_all)
-		ret = !seen;
-	else
-		ret = (seen == 1) ? 0 : seen > 1 ? 2 : 1;
-
-free_strings:
-	free(repo_config);
-	free(global);
-	free(xdg);
 	return ret;
 }
 
diff --git a/config.c b/config.c
index 08e47e2..2fbe634 100644
--- a/config.c
+++ b/config.c
@@ -1280,6 +1280,7 @@ out_free_ret_1:
 
 out_free_ret_1:
 	free(*store_key);
+	*store_key = NULL;
 	return -CONFIG_INVALID_KEY;
 }
 
-- 
1.8.0.3.g3456896

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]