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

[PATCH v2 05/12] diffcore-rename: move old_dir/new_dir definition to plug leak

From
andrzej@ahunt.org <andrzej@ahunt.org>
Date
Jul 25, 2021, 13:08 UTC
Message-ID
<20210725130830.5145-6-andrzej@ahunt.org>
In-Reply-To
<20210725130830.5145-1-andrzej@ahunt.org>
From: Andrzej Hunt <ajrhunt@google.com>

old_dir/new_dir are free()'d at the end of update_dir_rename_counts, however if we return early we'll never free those strings. Therefore we should move all new allocations after the possible early return, avoiding a leak.

This seems like a fairly recent leak, that started happening since the
early-return was added in:
  1ad69eb0dc (diffcore-rename: compute dir_rename_counts in stages, 2021-02-27)
LSAN output from t0022:
Direct leak of 7 byte(s) in 1 object(s) allocated from:
    #0 0x486804 in strdup ../projects/compiler-rt/lib/asan/asan_interceptors.cpp:452:3
    #1 0xa71e48 in xstrdup wrapper.c:29:14
    #2 0x7db9c7 in update_dir_rename_counts diffcore-rename.c:464:12
    #3 0x7db6ae in find_renames diffcore-rename.c:1062:3
    #4 0x7d76c3 in diffcore_rename_extended diffcore-rename.c:1472:18
    #5 0x7b4cfc in diffcore_std diff.c:6705:4
    #6 0x855e46 in log_tree_diff_flush log-tree.c:846:2
    #7 0x856574 in log_tree_diff log-tree.c:955:3
    #8 0x856574 in log_tree_commit log-tree.c:986:10
    #9 0x9a9c67 in print_commit_summary sequencer.c:1329:7
    #10 0x52e623 in cmd_commit builtin/commit.c:1862:3
    #11 0x4ce83e in run_builtin git.c:475:11
    #12 0x4ccafe in handle_builtin git.c:729:3
    #13 0x4cb01c in run_argv git.c:818:4
    #14 0x4cb01c in cmd_main git.c:949:19
    #15 0x6b3f3d in main common-main.c:52:11
    #16 0x7fe397c7a349 in __libc_start_main (/lib64/libc.so.6+0x24349)
Direct leak of 7 byte(s) in 1 object(s) allocated from:
    #0 0x486804 in strdup ../projects/compiler-rt/lib/asan/asan_interceptors.cpp:452:3
    #1 0xa71e48 in xstrdup wrapper.c:29:14
    #2 0x7db9bc in update_dir_rename_counts diffcore-rename.c:463:12
    #3 0x7db6ae in find_renames diffcore-rename.c:1062:3
    #4 0x7d76c3 in diffcore_rename_extended diffcore-rename.c:1472:18
    #5 0x7b4cfc in diffcore_std diff.c:6705:4
    #6 0x855e46 in log_tree_diff_flush log-tree.c:846:2
    #7 0x856574 in log_tree_diff log-tree.c:955:3
    #8 0x856574 in log_tree_commit log-tree.c:986:10
    #9 0x9a9c67 in print_commit_summary sequencer.c:1329:7
    #10 0x52e623 in cmd_commit builtin/commit.c:1862:3
    #11 0x4ce83e in run_builtin git.c:475:11
    #12 0x4ccafe in handle_builtin git.c:729:3
    #13 0x4cb01c in run_argv git.c:818:4
    #14 0x4cb01c in cmd_main git.c:949:19
    #15 0x6b3f3d in main common-main.c:52:11
    #16 0x7fe397c7a349 in __libc_start_main (/lib64/libc.so.6+0x24349)
SUMMARY: AddressSanitizer: 14 byte(s) leaked in 2 allocation(s).
Signed-off-by: Andrzej Hunt <andrzej@ahunt.org>
---
 diffcore-rename.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/diffcore-rename.c b/diffcore-rename.c
index 2618bb07c1..c95857b51f 100644
--- a/diffcore-rename.c
+++ b/diffcore-rename.c
@@ -448,9 +448,9 @@ static void update_dir_rename_counts(struct dir_rename_info *info,
 				     const char *oldname,
 				     const char *newname)
 {
-	char *old_dir = xstrdup(oldname);
-	char *new_dir = xstrdup(newname);
-	char new_dir_first_char = new_dir[0];
+	char *old_dir;
+	char *new_dir;
+	const char new_dir_first_char = newname[0];
 	int first_time_in_loop = 1;
 
 	if (!info->setup)
@@ -475,6 +475,10 @@ static void update_dir_rename_counts(struct dir_rename_info *info,
 		 */
 		return;
 
+
+	old_dir = xstrdup(oldname);
+	new_dir = xstrdup(newname);
+
 	while (1) {
 		int drd_flag = NOT_RELEVANT;
 
-- 
2.26.2
Previous: Junio C HamanoNext: Junio C Hamano
Message 43 of 51 in “Fix all leaks in tests t0002-t0099: Part 2”
  1. 00/12 Fix all leaks in tests t0002-t0099: Part 2andrzej@ahunt.org, Jun 20, 2021
  2. 01/12 fmt-merge-msg: free newly allocated temporary strings when doneandrzej@ahunt.org, Jun 20, 2021
  3. Elijah NewrenJun 21, 2021
  4. 02/12 environment: move strbuf into block to plug leakandrzej@ahunt.org, Jun 20, 2021
  5. Elijah NewrenJun 21, 2021
  6. René ScharfeJun 26, 2021
  7. 03/12 builtin/submodule--helper: release unused strbuf to avoid leakandrzej@ahunt.org, Jun 20, 2021
  8. 04/12 builtin/for-each-repo: remove unnecessary argv copy to plug leakandrzej@ahunt.org, Jun 20, 2021
  9. Elijah NewrenJun 21, 2021
  10. 05/12 diffcore-rename: move old_dir/new_dir definition to plug leakandrzej@ahunt.org, Jun 20, 2021
  11. Elijah NewrenJun 21, 2021
  12. 06/12 ref-filter: also free head for ATOM_HEAD to avoid leakandrzej@ahunt.org, Jun 20, 2021
  13. Elijah NewrenJun 21, 2021
  14. 07/12 read-cache: call diff_setup_done to avoid leakandrzej@ahunt.org, Jun 20, 2021
  15. Elijah NewrenJun 21, 2021
  16. 08/12 convert: release strbuf to avoid leakandrzej@ahunt.org, Jun 20, 2021
  17. Elijah NewrenJun 21, 2021
  18. 09/12 builtin/mv: free or UNLEAK multiple pointers at end of cmd_mvandrzej@ahunt.org, Jun 20, 2021
  19. 10/12 builtin/merge: free found_ref when doneandrzej@ahunt.org, Jun 20, 2021
  20. Elijah NewrenJun 21, 2021
  21. 11/12 builtin/rebase: fix options.strategy memory lifecycleandrzej@ahunt.org, Jun 20, 2021
  22. Phillip WoodJun 20, 2021
  23. Elijah NewrenJun 21, 2021
  24. Phillip WoodJun 22, 2021
  25. Andrzej HuntJul 25, 2021
  26. Phillip WoodJul 27, 2021
  27. 12/12 reset: clear_unpack_trees_porcelain to plug leakandrzej@ahunt.org, Jun 20, 2021
  28. Elijah NewrenJun 21, 2021
  29. Elijah NewrenJun 21, 2021
  30. Andrzej HuntJul 25, 2021
  31. Christian CouderJul 26, 2021
  32. 00/12 Fix all leaks in tests t0002-t0099: Part 2andrzej@ahunt.org, Jul 25, 2021
  33. 01/12 fmt-merge-msg: free newly allocated temporary strings when doneandrzej@ahunt.org, Jul 25, 2021
  34. Junio C HamanoJul 26, 2021
  35. 02/12 environment: move strbuf into block to plug leakandrzej@ahunt.org, Jul 25, 2021
  36. 03/12 builtin/submodule--helper: release unused strbuf to avoid leakandrzej@ahunt.org, Jul 25, 2021
  37. 04/12 builtin/for-each-repo: remove unnecessary argv copy to plug leakandrzej@ahunt.org, Jul 25, 2021
  38. Junio C HamanoJul 26, 2021
  39. 06/12 ref-filter: also free head for ATOM_HEAD to avoid leakandrzej@ahunt.org, Jul 25, 2021
  40. Junio C HamanoJul 26, 2021
  41. 07/12 read-cache: call diff_setup_done to avoid leakandrzej@ahunt.org, Jul 25, 2021
  42. Junio C HamanoJul 26, 2021
  43. 05/12 diffcore-rename: move old_dir/new_dir definition to plug leakandrzej@ahunt.org, Jul 25, 2021
  44. Junio C HamanoJul 26, 2021
  45. 09/12 builtin/mv: free or UNLEAK multiple pointers at end of cmd_mvandrzej@ahunt.org, Jul 25, 2021
  46. 08/12 convert: release strbuf to avoid leakandrzej@ahunt.org, Jul 25, 2021
  47. Junio C HamanoJul 26, 2021
  48. 10/12 builtin/merge: free found_ref when doneandrzej@ahunt.org, Jul 25, 2021
  49. 12/12 reset: clear_unpack_trees_porcelain to plug leakandrzej@ahunt.org, Jul 25, 2021
  50. 11/12 builtin/rebase: fix options.strategy memory lifecycleandrzej@ahunt.org, Jul 25, 2021
  51. Junio C HamanoJul 26, 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.