line-log: protect inner strbuf from free
Get series with b4
b4 am [email protected]Versions newest first · linked by author and title
- v2 Latest3 patches 2 repliesIn review
- v11 patch 6 repliesSuperseded
Patches in v2 No applies recorded yet
| # | Subject and files | Size | Replies | Applied |
|---|---|---|---|---|
| 1/3 | line-log: protect inner strbuf from freeline-log.c t/ | +32 | 1 reply | |
| 2/3 | line-log: remove output_prefix()line-log.c | +2 | 0 replies | |
| 3/3 | diff: modify output_prefix function pointerdiff-lib.c diff.c diff.h +3 more | +12 | 1 reply |
Reviews trailers per patch
None of the 3 patches has a Reviewed-by or Acked-by.
| Patch | Jeff King |
|---|---|
| 1/3 | Co-authored-by |
| 2/3 | |
| 3/3 |
Trailers given in replies are filled; those carried in the patches from earlier versions are outlined with dashes.
Cover letter [PATCH v2 0/3] line-log: protect inner strbuf from free
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 <[email protected]>
+ Signed-off-by: Jeff King <[email protected]>
Signed-off-by: Derrick Stolee <[email protected]>
## 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 pointerSignature · 1 line
gitgitgadget