git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH v2 4/5] config: fix a leak in git_config_copy_or_rename_section_in_file

From
Rubén Justo <rjusto@gmail.com>
Date
Jun 16, 2023, 23:34 UTC
Message-ID
<adaa7fa7-befe-ce59-7391-5a4ddd3bb2b8@gmail.com>
In-Reply-To
<5650c4ed-cec0-d11e-4f68-1661b3638786@gmail.com>

A branch can have its configuration spread over several configuration sections. This situation was already foreseen in 52d59cc645 (branch: add a --copy (-c) option to go with --move (-m), 2017-06-18) when "branch -c" was introduced.

Unfortunately, a leak was also introduced:
   $ git branch foo
   $ cat >> .git/config <<EOF
   [branch "foo"]
   	some-key-a = a value
   [branch "foo"]
   	some-key-b = b value
   [branch "foo"]
   	some-key-c = c value
   EOF
   $ git branch -c foo bar
   Direct leak of 130 byte(s) in 2 object(s) allocated from:
       ... in xrealloc wrapper.c
       ... in strbuf_grow strbuf.c
       ... in strbuf_vaddf strbuf.c
       ... in strbuf_addf strbuf.c
       ... in store_create_section config.c
       ... in git_config_copy_or_rename_section_in_file config.c
       ... in git_config_copy_section_in_file config.c
       ... in git_config_copy_section config.c
       ... in copy_or_rename_branch builtin/branch.c
       ... in cmd_branch builtin/branch.c
       ... in run_builtin git.c
Let's fix it.

Let's also modify the function store_create_section() so that it stops returning a strbuf, which is an uncommon pattern in our code base, and instead, starts receiving the strbuf to use from its caller.

Signed-off-by: Rubén Justo <rjusto@gmail.com>
---
 config.c | 29 +++++++++++++++--------------
 1 file changed, 15 insertions(+), 14 deletions(-)
diff --git a/config.c b/config.c
index 39a7d7422c..c5f4b59ef3 100644
--- a/config.c
+++ b/config.c
@@ -3140,37 +3140,36 @@ static int write_error(const char *filename)
 	return 4;
 }
 
-static struct strbuf store_create_section(const char *key,
-					  const struct config_store_data *store)
+static void store_create_section(const char *key,
+				const struct config_store_data *store,
+				struct strbuf *sb)
 {
 	const char *dot;
 	size_t i;
-	struct strbuf sb = STRBUF_INIT;
 
 	dot = memchr(key, '.', store->baselen);
 	if (dot) {
-		strbuf_addf(&sb, "[%.*s \"", (int)(dot - key), key);
+		strbuf_addf(sb, "[%.*s \"", (int)(dot - key), key);
 		for (i = dot - key + 1; i < store->baselen; i++) {
 			if (key[i] == '"' || key[i] == '\\')
-				strbuf_addch(&sb, '\\');
-			strbuf_addch(&sb, key[i]);
+				strbuf_addch(sb, '\\');
+			strbuf_addch(sb, key[i]);
 		}
-		strbuf_addstr(&sb, "\"]\n");
+		strbuf_addstr(sb, "\"]\n");
 	} else {
-		strbuf_addch(&sb, '[');
-		strbuf_add(&sb, key, store->baselen);
-		strbuf_addstr(&sb, "]\n");
+		strbuf_addch(sb, '[');
+		strbuf_add(sb, key, store->baselen);
+		strbuf_addstr(sb, "]\n");
 	}
-
-	return sb;
 }
 
 static ssize_t write_section(int fd, const char *key,
 			     const struct config_store_data *store)
 {
-	struct strbuf sb = store_create_section(key, store);
+	struct strbuf sb = STRBUF_INIT;
 	ssize_t ret;
 
+	store_create_section(key, store, &sb);
 	ret = write_in_full(fd, sb.buf, sb.len);
 	strbuf_release(&sb);
 
@@ -3833,7 +3832,9 @@ static int git_config_copy_or_rename_section_in_file(const char *config_filename
 						output[0] = '\t';
 					}
 				} else {
-					copystr = store_create_section(new_name, &store);
+					strbuf_reset(&copystr);
+					store_create_section(new_name, &store,
+							     &copystr);
 				}
 			}
 			remove = 0;
-- 
2.40.1
Previous: Rubén JustoNext: Rubén Justo
Message 37 of 47 in “tests: mark as passing with SANITIZE=leak”
  1. Rubén JustoJun 11, 2023
  2. 01/11 rev-parse: fix a leak with --abbrev-refRubén Justo, Jun 11, 2023
  3. Jeff KingJun 12, 2023
  4. Rubén JustoJun 16, 2023
  5. 02/11 config: fix a leak in git_config_copy_or_rename_section_in_fileRubén Justo, Jun 11, 2023
  6. Jeff KingJun 12, 2023
  7. 03/11 remote: fix a leak in query_matches_negative_refspecRubén Justo, Jun 11, 2023
  8. Jeff KingJun 12, 2023
  9. Rubén JustoJun 16, 2023
  10. 04/11 branch: fix a leak in dwim_and_setup_trackingRubén Justo, Jun 11, 2023
  11. Jeff KingJun 12, 2023
  12. Rubén JustoJun 16, 2023
  13. 05/11 branch: fix a leak in setup_trackingRubén Justo, Jun 11, 2023
  14. Jeff KingJun 12, 2023
  15. Rubén JustoJun 16, 2023
  16. 06/11 branch: fix a leak in cmd_branchRubén Justo, Jun 11, 2023
  17. Jeff KingJun 12, 2023
  18. Rubén JustoJun 16, 2023
  19. 07/11 branch: fix a leak in inherit_trackingRubén Justo, Jun 11, 2023
  20. Jeff KingJun 12, 2023
  21. 08/11 branch: fix a leak in check_tracking_branchRubén Justo, Jun 11, 2023
  22. Jeff KingJun 12, 2023
  23. 09/11 branch: fix a leak in setup_trackingRubén Justo, Jun 11, 2023
  24. Jeff KingJun 12, 2023
  25. 10/11 config: fix a leak in git_config_copy_or_rename_section_in_fileRubén Justo, Jun 11, 2023
  26. Jeff KingJun 12, 2023
  27. Rubén JustoJun 16, 2023
  28. 11/11 tests: mark as passing with SANITIZE=leakRubén Justo, Jun 11, 2023
  29. Rubén JustoJun 11, 2023
  30. Jeff KingJun 12, 2023
  31. Rubén JustoJun 16, 2023
  32. Junio C HamanoJun 13, 2023
  33. 0/5 tests: mark as passing with SANITIZE=leakRubén Justo, Jun 16, 2023
  34. 1/5 rev-parse: fix a leak with --abbrev-refRubén Justo, Jun 16, 2023
  35. 2/5 branch: fix a leak in setup_trackingRubén Justo, Jun 16, 2023
  36. 3/5 branch: fix a leak in cmd_branchRubén Justo, Jun 16, 2023
  37. 4/5 config: fix a leak in git_config_copy_or_rename_section_in_fileRubén Justo, Jun 16, 2023
  38. 5/5 tests: mark as passing with SANITIZE=leakRubén Justo, Jun 16, 2023
  39. Junio C HamanoJun 16, 2023
  40. Jeff KingJun 17, 2023
  41. Rubén JustoJun 17, 2023
  42. 0/5 tests: mark as passing with SANITIZE=leakRubén Justo, Jun 17, 2023
  43. 1/5 rev-parse: fix a leak with --abbrev-refRubén Justo, Jun 17, 2023
  44. 2/5 branch: fix a leak in setup_trackingRubén Justo, Jun 17, 2023
  45. 3/5 branch: fix a leak in cmd_branchRubén Justo, Jun 17, 2023
  46. 4/5 config: fix a leak in git_config_copy_or_rename_section_in_fileRubén Justo, Jun 17, 2023
  47. 5/5 tests: mark as passing with SANITIZE=leakRubén Justo, Jun 17, 2023

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.