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

[PATCH 7/7] parse-options: don't leak alias help messages

From
Andrzej Hunt via GitGitGadget <gitgitgadget@gmail.com>
Date
Mar 8, 2021, 18:36 UTC
Message-ID
<fb456bee0f69e0ca5e596b30705c42cc037edecc.1615228580.git.gitgitgadget@gmail.com>
In-Reply-To
<pull.899.git.1615228580.gitgitgadget@gmail.com>
From: Andrzej Hunt <ajrhunt@google.com>

preprocess_options() allocates new strings for help messages for OPTION_ALIAS. Therefore we also need to clean those help messages up when freeing the returned options.

First introduced in:
  7c280589cf (parse-options: teach "git cmd -h" to show alias as alias, 2020-03-16)

The preprocessed options themselves no longer contain any indication that a given option is/was an alias: the easiest and fastest way to figure it out is to look back at the original options. Alternatively we could iterate over the alias_groups list - but that would require nested looping and is likely to be a (little) less efficient.

As far as I can tell, parse_options() is only ever used once per command, and the help messages are small - hence this leak has very little impact.

This leak was found while running t0001. LSAN output can be found below:
Direct leak of 65 byte(s) in 1 object(s) allocated from:
    #0 0x49a859 in realloc /home/abuild/rpmbuild/BUILD/llvm-11.0.0.src/build/../projects/compiler-rt/lib/asan/asan_malloc_linux.cpp:164:3
    #1 0x9aae36 in xrealloc /home/ahunt/oss-fuzz/git/wrapper.c:126:8
    #2 0x939d8d in strbuf_grow /home/ahunt/oss-fuzz/git/strbuf.c:98:2
    #3 0x93b936 in strbuf_vaddf /home/ahunt/oss-fuzz/git/strbuf.c:392:3
    #4 0x93b7ff in strbuf_addf /home/ahunt/oss-fuzz/git/strbuf.c:333:2
    #5 0x86747e in preprocess_options /home/ahunt/oss-fuzz/git/parse-options.c:666:3
    #6 0x866ed2 in parse_options /home/ahunt/oss-fuzz/git/parse-options.c:847:17
    #7 0x51c4a7 in cmd_clone /home/ahunt/oss-fuzz/git/builtin/clone.c:989:9
    #8 0x4cd60d in run_builtin /home/ahunt/oss-fuzz/git/git.c:453:11
    #9 0x4cb2da in handle_builtin /home/ahunt/oss-fuzz/git/git.c:704:3
    #10 0x4ccc37 in run_argv /home/ahunt/oss-fuzz/git/git.c:771:4
    #11 0x4cac29 in cmd_main /home/ahunt/oss-fuzz/git/git.c:902:19
    #12 0x69c9fe in main /home/ahunt/oss-fuzz/git/common-main.c:52:11
    #13 0x7fdac42d4349 in __libc_start_main (/lib64/libc.so.6+0x24349)
Signed-off-by: Andrzej Hunt <ajrhunt@google.com>
---
 parse-options.c | 29 ++++++++++++++++++++++++-----
 1 file changed, 24 insertions(+), 5 deletions(-)
diff --git a/parse-options.c b/parse-options.c
index fbea16eaf5c2..3fe1dacc08cb 100644
--- a/parse-options.c
+++ b/parse-options.c
@@ -625,6 +625,8 @@ static int show_gitcomp(const struct option *opts, int show_all)
  *
  * Right now this is only used to preprocess and substitute
  * OPTION_ALIAS.
+ *
+ * The returned options should be freed using free_preprocessed_options.
  */
 static struct option *preprocess_options(struct parse_opt_ctx_t *ctx,
 					 const struct option *options)
@@ -693,6 +695,21 @@ static struct option *preprocess_options(struct parse_opt_ctx_t *ctx,
 	return newopt;
 }
 
+static void free_preprocessed_options(const struct option ** preprocessed_options, const struct option *original_options)
+{
+	int i;
+
+	if (!*preprocessed_options) {
+		return;
+	}
+	for (i = 0; original_options[i].type != OPTION_END; i++) {
+		if (original_options[i].type == OPTION_ALIAS) {
+			free((void *)(*preprocessed_options)[i].help);
+		}
+	}
+	free((void *)*preprocessed_options);
+}
+
 static int usage_with_options_internal(struct parse_opt_ctx_t *,
 				       const char * const *,
 				       const struct option *, int, int);
@@ -838,15 +855,17 @@ int parse_options(int argc, const char **argv, const char *prefix,
 		  int flags)
 {
 	struct parse_opt_ctx_t ctx;
-	struct option *real_options;
+	const struct option *preprocessed_options, *original_options = NULL;
 
 	disallow_abbreviated_options =
 		git_env_bool("GIT_TEST_DISALLOW_ABBREVIATED_OPTIONS", 0);
 
 	memset(&ctx, 0, sizeof(ctx));
-	real_options = preprocess_options(&ctx, options);
-	if (real_options)
-		options = real_options;
+	preprocessed_options = preprocess_options(&ctx, options);
+	if (preprocessed_options) {
+		original_options = options;
+		options = preprocessed_options;
+	}
 	parse_options_start_1(&ctx, argc, argv, prefix, options, flags);
 	switch (parse_options_step(&ctx, options, usagestr)) {
 	case PARSE_OPT_HELP:
@@ -870,7 +889,7 @@ int parse_options(int argc, const char **argv, const char *prefix,
 	}
 
 	precompose_argv_prefix(argc, argv, NULL);
-	free(real_options);
+	free_preprocessed_options(&preprocessed_options, original_options);
 	free(ctx.alias_groups);
 	return parse_options_end(&ctx);
 }
-- 
gitgitgadget
Previous: Andrzej HuntNext: Jeff King
Message 15 of 52 in “Fix all leaks in t0001”
  1. 0/7 Fix all leaks in t0001Andrzej Hunt via GitGitGadget, Mar 8, 2021
  2. 1/7 symbolic-ref: don't leak shortened refname in check_symref()Andrzej Hunt via GitGitGadget, Mar 8, 2021
  3. Jeff KingMar 8, 2021
  4. Andrzej HuntMar 14, 2021
  5. 2/7 reset: free instead of leaking unneeded refAndrzej Hunt via GitGitGadget, Mar 8, 2021
  6. Jeff KingMar 8, 2021
  7. 3/7 clone: free or UNLEAK further pointers when finishedAndrzej Hunt via GitGitGadget, Mar 8, 2021
  8. Jeff KingMar 8, 2021
  9. Andrzej HuntMar 14, 2021
  10. 6/7 init-db: silence template_dir leak when converting to absolute pathAndrzej Hunt via GitGitGadget, Mar 8, 2021
  11. Jeff KingMar 8, 2021
  12. 4/7 worktree: fix leak in dwim_branch()Andrzej Hunt via GitGitGadget, Mar 8, 2021
  13. Jeff KingMar 8, 2021
  14. Andrzej HuntMar 14, 2021
  15. 7/7 parse-options: don't leak alias help messagesAndrzej Hunt via GitGitGadget, Mar 8, 2021
  16. Jeff KingMar 8, 2021
  17. Andrzej HuntMar 14, 2021
  18. 5/7 init: remove git_init_db_config() while fixing leaksAndrzej Hunt via GitGitGadget, Mar 8, 2021
  19. Jeff KingMar 8, 2021
  20. Jeff KingMar 8, 2021
  21. Junio C HamanoMar 12, 2021
  22. Andrzej HuntMar 14, 2021
  23. Andrzej HuntMar 15, 2021
  24. Junio C HamanoMar 8, 2021
  25. Andrzej HuntMar 14, 2021
  26. 0/9 Fix all leaks in t0001Andrzej Hunt via GitGitGadget, Mar 14, 2021
  27. 8/9 parse-options: don't leak alias help messagesAndrzej Hunt via GitGitGadget, Mar 14, 2021
  28. Eric SunshineMar 14, 2021
  29. Andrzej HuntMar 15, 2021
  30. Andrzej HuntMar 14, 2021
  31. 1/9 symbolic-ref: don't leak shortened refname in check_symref()Andrzej Hunt via GitGitGadget, Mar 14, 2021
  32. 7/9 parse-options: convert bitfield values to use binary shiftAndrzej Hunt via GitGitGadget, Mar 14, 2021
  33. Martin ÅgrenMar 14, 2021
  34. Junio C HamanoMar 14, 2021
  35. Andrzej HuntMar 15, 2021
  36. 4/9 worktree: fix leak in dwim_branch()Andrzej Hunt via GitGitGadget, Mar 14, 2021
  37. 5/9 init: remove git_init_db_config() while fixing leaksAndrzej Hunt via GitGitGadget, Mar 14, 2021
  38. 3/9 clone: free or UNLEAK further pointers when finishedAndrzej Hunt via GitGitGadget, Mar 14, 2021
  39. 2/9 reset: free instead of leaking unneeded refAndrzej Hunt via GitGitGadget, Mar 14, 2021
  40. 9/9 transport: also free remote_refs in transport_disconnect()Andrzej Hunt via GitGitGadget, Mar 14, 2021
  41. 6/9 init-db: silence template_dir leak when converting to absolute pathAndrzej Hunt via GitGitGadget, Mar 14, 2021
  42. 0/9 Fix all leaks in t0001Andrzej Hunt via GitGitGadget, Mar 21, 2021
  43. 1/9 symbolic-ref: don't leak shortened refname in check_symref()Andrzej Hunt via GitGitGadget, Mar 21, 2021
  44. 2/9 reset: free instead of leaking unneeded refAndrzej Hunt via GitGitGadget, Mar 21, 2021
  45. 4/9 worktree: fix leak in dwim_branch()Andrzej Hunt via GitGitGadget, Mar 21, 2021
  46. 3/9 clone: free or UNLEAK further pointers when finishedAndrzej Hunt via GitGitGadget, Mar 21, 2021
  47. 7/9 parse-options: convert bitfield values to use binary shiftAndrzej Hunt via GitGitGadget, Mar 21, 2021
  48. 6/9 init-db: silence template_dir leak when converting to absolute pathAndrzej Hunt via GitGitGadget, Mar 21, 2021
  49. 5/9 init: remove git_init_db_config() while fixing leaksAndrzej Hunt via GitGitGadget, Mar 21, 2021
  50. 9/9 transport: also free remote_refs in transport_disconnect()Andrzej Hunt via GitGitGadget, Mar 21, 2021
  51. 8/9 parse-options: don't leak alias help messagesAndrzej Hunt via GitGitGadget, Mar 21, 2021
  52. Junio C HamanoMar 21, 2021

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.