@matheusbsbs / matheus-bagliano / commits / fc5589d6c1

line-log: protect inner strbuf from free

The output_prefix() method in line-log.c may call a function pointer via the diff_options struct. This function pointer returns a strbuf struct and then its buffer is passed back. However, that implies that the consumer is responsible to free the string. This is especially true because the default behavior is to duplicate the empty string. The existing functions used in the output_prefix pointer include: 1. idiff_prefix_cb() in diff-lib.c. This returns the data pointer, so the value exists across multiple calls. 2. diff_output_prefix_callback() in graph.c. This uses a static strbuf struct, so it reuses buffers across calls. These should not be freed. 3. output_prefix_cb() in range-diff.c. This is similar to the diff-lib.c case. In each case, we should not be freeing this buffer. We can convert the output_prefix() function to return a const char pointer and stop freeing the result. This choice is essentially the opposite of what was done in 394affd46d (line-log: always allocate the output prefix, 2024-06-07). This was discovered via 'valgrind' while investigating a public report of a bug in 'git log --graph -L' [1]. [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> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Derrick Stolee committed Oct 3, 2024 at 11:58 UTC fc5589d6c1200f87689ff95067f18caa2a826382

2 files changed +32 -6

line-log.c

+4 -6

@@ -897,13 +897,13 @@ static void print_line(const char *prefix, char first,

897 fputs("\\ No newline at end of file\n", file);

898 }

899

900 -static char *output_prefix(struct diff_options *opt)

900 +static const char *output_prefix(struct diff_options *opt)

901 {

902 if (opt->output_prefix) {

903 struct strbuf *sb = opt->output_prefix(opt, opt->output_prefix_data);

904 return sb->buf;

905 } else {

906 - return xstrdup("");

906 + return "";

907 }

908 }

909

@@ -916,7 +916,7 @@ static void dump_diff_hacky_one(struct rev_info *rev, struct line_log_data *rang

916 struct diff_ranges *diff = &range->diff;

917

918 struct diff_options *opt = &rev->diffopt;

919 - char *prefix = output_prefix(opt);

919 + const char *prefix = output_prefix(opt);

920 const char *c_reset = diff_get_color(opt->use_color, DIFF_RESET);

921 const char *c_frag = diff_get_color(opt->use_color, DIFF_FRAGINFO);

922 const char *c_meta = diff_get_color(opt->use_color, DIFF_METAINFO);

@@ -1003,7 +1003,6 @@ static void dump_diff_hacky_one(struct rev_info *rev, struct line_log_data *rang

1003 out:

1004 free(p_ends);

1005 free(t_ends);

1006 - free(prefix);

1006 }

1007

1008 /*

@@ -1012,10 +1011,9 @@ out:

1011 */

1012 static void dump_diff_hacky(struct rev_info *rev, struct line_log_data *range)

1013 {

1015 - char *prefix = output_prefix(&rev->diffopt);

1014 + const char *prefix = output_prefix(&rev->diffopt);

1015

1016 fprintf(rev->diffopt.file, "%s\n", prefix);

1018 - free(prefix);

1017

1018 while (range) {

1019 dump_diff_hacky_one(rev, range);

t/t4211-line-log.sh

+28

@@ -337,4 +337,32 @@ test_expect_success 'zero-width regex .* matches any function name' '

337 test_cmp expect actual

338 '

339

340 +test_expect_success 'show line-log with graph' '

341 + qz_to_tab_space >expect <<-EOF &&

342 + * $head_oid Modify func2() in file.c

343 + |Z

344 + | diff --git a/file.c b/file.c

345 + | --- a/file.c

346 + | +++ b/file.c

347 + | @@ -6,4 +6,4 @@

348 + | int func2()

349 + | {

350 + | - return F2;

351 + | + return F2 + 2;

352 + | }

353 + * $root_oid Add func1() and func2() in file.c

354 + ZZ

355 + diff --git a/file.c b/file.c

356 + --- /dev/null

357 + +++ b/file.c

358 + @@ -0,0 +6,4 @@

359 + +int func2()

360 + +{

361 + + return F2;

362 + +}

363 + EOF

364 + git log --graph --oneline -L:func2:file.c >actual &&

365 + test_cmp expect actual

366 +'

367 +

368 test_done