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

[PATCH v2 0/3] line-log: protect inner strbuf from free

From
Derrick Stolee via GitGitGadget <gitgitgadget@gmail.com>
Date
Oct 3, 2024, 11:58 UTC
Message-ID
<pull.1806.v2.git.1727956724.gitgitgadget@gmail.com>
In-Reply-To
<pull.1806.git.1727885224966.gitgitgadget@gmail.com>
This fixes a regression introduced in 2.46.0.

The change made in 394affd46d (line-log: always allocate the output prefix, 2024-06-07) made the method more consistent in that it did not return a static empty string that would fail to free(), but it does lead to double-frees when a strbuf buffer is freed multiple times.

In v2, I add Peff's test to the first patch. I also split his diff into two more follow-up patches because the additional clarity seems valuable to me. I have forged his sign-off in all three patches.

Note to the maintainer: feel free to take only the first patch, as Peff replied that he may work on the remaining cleanup independently (but I had already prepared patches 2 & 3).

Thanks, -Stolee
Derrick Stolee (1):
  line-log: protect inner strbuf from free
Jeff King (2):
  line-log: remove output_prefix()
  diff: modify output_prefix function pointer
 diff-lib.c          |  4 ++--
 diff.c              |  8 +++-----
 diff.h              |  2 +-
 graph.c             |  4 ++--
 line-log.c          | 16 ++--------------
 log-tree.c          |  4 ++--
 range-diff.c        |  4 ++--
 t/t4211-line-log.sh | 28 ++++++++++++++++++++++++++++
 8 files changed, 42 insertions(+), 28 deletions(-)
base-commit: e9356ba3ea2a6754281ff7697b3e5a1697b21e24
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1806%2Fderrickstolee%2Fline-log-use-after-free-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1806/derrickstolee/line-log-use-after-free-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/1806
Range-diff vs v1:
 1:  5d341e42d83 ! 1:  05c21616c35 line-log: protect inner strbuf from free
     @@ Commit message
      
          [1] https://github.com/git-for-windows/git/issues/5185
      
     +    This issue would have been caught by the new test, when Git is compiled
     +    with ASan to catch these double frees.
     +
     +    Co-authored-by: Jeff King <peff@peff.net>
     +    Signed-off-by: Jeff King <peff@peff.net>
          Signed-off-by: Derrick Stolee <stolee@gmail.com>
      
       ## line-log.c ##
     @@ line-log.c: out:
       
       	while (range) {
       		dump_diff_hacky_one(rev, range);
     +
     + ## t/t4211-line-log.sh ##
     +@@ t/t4211-line-log.sh: test_expect_success 'zero-width regex .* matches any function name' '
     + 	test_cmp expect actual
     + '
     + 
     ++test_expect_success 'show line-log with graph' '
     ++	qz_to_tab_space >expect <<-EOF &&
     ++	* $head_oid Modify func2() in file.c
     ++	|Z
     ++	| diff --git a/file.c b/file.c
     ++	| --- a/file.c
     ++	| +++ b/file.c
     ++	| @@ -6,4 +6,4 @@
     ++	|  int func2()
     ++	|  {
     ++	| -    return F2;
     ++	| +    return F2 + 2;
     ++	|  }
     ++	* $root_oid Add func1() and func2() in file.c
     ++	ZZ
     ++	  diff --git a/file.c b/file.c
     ++	  --- /dev/null
     ++	  +++ b/file.c
     ++	  @@ -0,0 +6,4 @@
     ++	  +int func2()
     ++	  +{
     ++	  +    return F2;
     ++	  +}
     ++	EOF
     ++	git log --graph --oneline -L:func2:file.c >actual &&
     ++	test_cmp expect actual
     ++'
     ++
     + test_done
 -:  ----------- > 2:  94d2c034b4a line-log: remove output_prefix()
 -:  ----------- > 3:  e1d825ad212 diff: modify output_prefix function pointer
-- 
gitgitgadget
Previous: Jeff KingNext: Derrick Stolee via GitGitGadget
Message 8 of 24 in “line-log: protect inner strbuf from free”
  1. line-log: protect inner strbuf from freeDerrick Stolee via GitGitGadget, Oct 2, 2024
  2. Jeff KingOct 2, 2024
  3. Jeff KingOct 3, 2024
  4. Derrick StoleeOct 3, 2024
  5. Jeff KingOct 3, 2024
  6. Derrick StoleeOct 3, 2024
  7. Jeff KingOct 3, 2024
  8. 0/3 line-log: protect inner strbuf from freeDerrick Stolee via GitGitGadget, Oct 3, 2024
  9. 1/3 line-log: protect inner strbuf from freeDerrick Stolee via GitGitGadget, Oct 3, 2024
  10. Patrick SteinhardtOct 4, 2024
  11. 2/3 line-log: remove output_prefix()Jeff King via GitGitGadget, Oct 3, 2024
  12. 3/3 diff: modify output_prefix function pointerJeff King via GitGitGadget, Oct 3, 2024
  13. Junio C HamanoOct 3, 2024
  14. 0/5 diff output_prefix cleanupsJeff King, Oct 3, 2024
  15. 1/5 line-log: use diff_line_prefix() instead of custom helperJeff King, Oct 3, 2024
  16. 2/5 diff: drop line_prefix_length fieldJeff King, Oct 3, 2024
  17. 3/5 diff: return const char from output_prefix callbackJeff King, Oct 3, 2024
  18. 4/5 diff: return line_prefix directly when possibleJeff King, Oct 3, 2024
  19. 5/5 diff: store graph prefix buf in git_graph structJeff King, Oct 3, 2024
  20. Junio C HamanoOct 3, 2024
  21. Patrick SteinhardtOct 4, 2024
  22. Derrick StoleeOct 4, 2024
  23. Junio C HamanoOct 4, 2024
  24. Derrick StoleeOct 4, 2024

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.