git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Derrick Stolee via GitGitGadget" <gitgitgadget@gmail.com>
To: git@vger.kernel.org
Cc: gitster@pobox.com, ps@pks.im, Jeff King <peff@peff.net>,
	Derrick Stolee <stolee@gmail.com>
Subject: [PATCH v2 0/3] line-log: protect inner strbuf from free
Date: Thu, 03 Oct 2024 11:58:41 +0000	[thread overview]
Message-ID: <pull.1806.v2.git.1727956724.gitgitgadget@gmail.com> (raw)
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

  parent reply	other threads:[~2024-10-03 11:58 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-10-02 16:07 [PATCH] line-log: protect inner strbuf from free Derrick Stolee via GitGitGadget
2024-10-02 23:56 ` Jeff King
2024-10-03  0:19   ` Jeff King
2024-10-03  2:36   ` Derrick Stolee
2024-10-03  6:11     ` Jeff King
2024-10-03 12:23       ` Derrick Stolee
2024-10-03 21:02         ` Jeff King
2024-10-03 11:58 ` Derrick Stolee via GitGitGadget [this message]
2024-10-03 11:58   ` [PATCH v2 1/3] " Derrick Stolee via GitGitGadget
2024-10-04  4:32     ` Patrick Steinhardt
2024-10-03 11:58   ` [PATCH v2 2/3] line-log: remove output_prefix() Jeff King via GitGitGadget
2024-10-03 11:58   ` [PATCH v2 3/3] diff: modify output_prefix function pointer Jeff King via GitGitGadget
2024-10-03 16:26     ` Junio C Hamano
2024-10-03 21:05   ` [PATCH 0/5] diff output_prefix cleanups Jeff King
2024-10-03 21:06     ` [PATCH 1/5] line-log: use diff_line_prefix() instead of custom helper Jeff King
2024-10-03 21:06     ` [PATCH 2/5] diff: drop line_prefix_length field Jeff King
2024-10-03 21:09     ` [PATCH 3/5] diff: return const char from output_prefix callback Jeff King
2024-10-03 21:11     ` [PATCH 4/5] diff: return line_prefix directly when possible Jeff King
2024-10-03 21:13     ` [PATCH 5/5] diff: store graph prefix buf in git_graph struct Jeff King
2024-10-03 23:43       ` Junio C Hamano
2024-10-04  4:32         ` Patrick Steinhardt
2024-10-04 19:27     ` [PATCH 0/5] diff output_prefix cleanups Derrick Stolee
2024-10-04 19:31       ` Junio C Hamano
2024-10-04 19:33     ` Derrick Stolee

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=pull.1806.v2.git.1727956724.gitgitgadget@gmail.com \
    --to=gitgitgadget@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=peff@peff.net \
    --cc=ps@pks.im \
    --cc=stolee@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).