Double free after calling git_diff_merge #6588

ehuss · 2023-07-08T20:10:18Z

Reproduction steps

#include <assert.h>
#include <git2/repository.h>
#include <git2/errors.h>
#include <git2/global.h>
#include <git2/config.h>
#include <git2/index.h>
#include <git2/diff.h>
#include <stdio.h>
#include <unistd.h>
#include <string.h>

int main() {
  assert(git_libgit2_init() == 1);
  git_diff *diff_a;
  git_diff *diff_b;

  const char *a = "diff --git a/README.md b/README.md\n\
index 18fb8328..ce60f40c 100644\n\
--- a/README.md\n\
+++ b/README.md\n\
@@ -4,1 +4,2 @@ componentwise\n\
 reusing\n\
+proverb\n\
";
  const char *b = "diff --git a/README.md b/README.md\n\
index 18fb8328..ce60f40c 100644\n\
--- a/README.md\n\
+++ b/README.md\n\
@@ -4,2 +4,3 @@ componentwise\n\
 reusing\n\
 proverb\n\
+offended\n\
";

  assert(git_diff_from_buffer(&diff_a, a, strlen(a)) == 0);
  assert(git_diff_from_buffer(&diff_b, b, strlen(b)) == 0);
  assert(git_diff_merge(diff_a, diff_b) == 0);
  git_diff_free(diff_b);
  git_diff_free(diff_a);  // ERROR here

  return 0;
}

Expected behavior

After calling git_diff_merge, I am assuming that both the original git_diff objects should be safe to free.

Actual behavior

There is a double-free when calling git_diff_free. From a small amount of debugging, I see that it is failing in patch_parsed__free when freeing patch->base.delta. The delta was previously freed in git_dif__merge when it calls git_vector_free_deep(&onto_new); removing the previous vector of deltas.

Version of libgit2 (release number or SHA1)

f041a94 or v1.6.4

Operating system(s) tested

Linux or macOS

The text was updated successfully, but these errors were encountered:

This commit addresses a double free issue that occurs when merging two diffs and free the diff objects. The problem was caused from copying the pointer using a vector swap git function and then make a deep free of the original vector. This operation made on a vector of pointers deallocated all the pointers inside the new vector where we swap the content to. As a result, an invalid object was returned to the user. The valgrid report. ``` ==256010== HEAP SUMMARY: ==256010== in use at exit: 696,247 bytes in 11,244 blocks ==256010== total heap usage: 50,123 allocs, 38,880 frees, 4,419,142 bytes allocated ==256010== ==256010== Searching for pointers to 11,244 not-freed blocks ==256010== Checked 850,720 bytes ==256010== ==256010== 112 bytes in 1 blocks are definitely lost in loss record 69 of 163 ==256010== at 0x4841848: malloc (vg_replace_malloc.c:431) ==256010== by 0x48964FD: stdalloc__malloc.lto_priv.0 (stdalloc.c:22) ==256010== by 0x48D4C34: git_diff__delta_dup (diff_tform.c:23) ==256010== by 0x48DAA22: git_diff__merge_like_cgit (diff_tform.c:81) ==256010== by 0x48D5C14: git_diff__merge (diff_tform.c:160) ==256010== by 0x1092A6: main (in /home/vincent/Github/libgit2/build/a.out) ==256010== ==256010== LEAK SUMMARY: ==256010== definitely lost: 112 bytes in 1 blocks ==256010== indirectly lost: 0 bytes in 0 blocks ==256010== possibly lost: 0 bytes in 0 blocks ==256010== still reachable: 696,135 bytes in 11,243 blocks ==256010== suppressed: 0 bytes in 0 blocks ==256010== Reachable blocks (those to which a pointer was found) are not shown. ==256010== To see them, rerun with: --leak-check=full --show-leak-kinds=all ==256010== ==256010== ERROR SUMMARY: 2 errors from 2 contexts (suppressed: 0 from 0) ==256010== ==256010== 1 errors in context 1 of 2: ==256010== Invalid free() / delete / delete[] / realloc() ==256010== at 0x484412F: free (vg_replace_malloc.c:974) ==256010== by 0x490D5B0: patch_parsed__free.part.0.lto_priv.0 (patch_parse.c:1150) ==256010== by 0x48CC454: diff_parsed_free (diff_parse.c:21) ==256010== by 0x1092EA: main (in /home/vincent/Github/libgit2/build/a.out) ==256010== Address 0x59e55b0 is 0 bytes inside a block of size 112 free'd ==256010== at 0x484412F: free (vg_replace_malloc.c:974) ==256010== by 0x48A17CD: git_vector_free_deep.part.0 (vector.c:99) ==256010== by 0x48D5A2B: UnknownInlinedFun (vector.c:95) ==256010== by 0x48D5A2B: git_diff__merge (diff_tform.c:193) ==256010== by 0x1092A6: main (in /home/vincent/Github/libgit2/build/a.out) ==256010== Block was alloc'd at ==256010== at 0x48469B3: calloc (vg_replace_malloc.c:1554) ==256010== by 0x489652D: stdalloc__calloc.lto_priv.0 (stdalloc.c:42) ==256010== by 0x48CCDE9: UnknownInlinedFun (patch_parse.c:1184) ==256010== by 0x48CCDE9: git_diff_from_buffer (diff_parse.c:86) ==256010== by 0x109219: main (in /home/vincent/Github/libgit2/build/a.out) ==256010== ==256010== ERROR SUMMARY: 2 errors from 2 contexts (suppressed: 0 from 0) ``` Link: libgit2#6588 Reported-by: Eric Huss Signed-off-by: Vincenzo Palazzo <vincenzopalazzodev@gmail.com>

vincenzopalazzo · 2023-07-10T14:43:33Z

Ok, a possible patch is live #6589

This commit addresses a double free issue that occurs when merging two diffs and free the diff objects. The problem was caused from copying the pointer using a vector swap git function and then make a deep free of the original vector. This operation made on a vector of pointers deallocated all the pointers inside the new vector where we swap the content to. As a result, an invalid object was returned to the user. The valgrid report. ``` ==256010== HEAP SUMMARY: ==256010== in use at exit: 696,247 bytes in 11,244 blocks ==256010== total heap usage: 50,123 allocs, 38,880 frees, 4,419,142 bytes allocated ==256010== ==256010== Searching for pointers to 11,244 not-freed blocks ==256010== Checked 850,720 bytes ==256010== ==256010== 112 bytes in 1 blocks are definitely lost in loss record 69 of 163 ==256010== at 0x4841848: malloc (vg_replace_malloc.c:431) ==256010== by 0x48964FD: stdalloc__malloc.lto_priv.0 (stdalloc.c:22) ==256010== by 0x48D4C34: git_diff__delta_dup (diff_tform.c:23) ==256010== by 0x48DAA22: git_diff__merge_like_cgit (diff_tform.c:81) ==256010== by 0x48D5C14: git_diff__merge (diff_tform.c:160) ==256010== by 0x1092A6: main (in /home/vincent/Github/libgit2/build/a.out) ==256010== ==256010== LEAK SUMMARY: ==256010== definitely lost: 112 bytes in 1 blocks ==256010== indirectly lost: 0 bytes in 0 blocks ==256010== possibly lost: 0 bytes in 0 blocks ==256010== still reachable: 696,135 bytes in 11,243 blocks ==256010== suppressed: 0 bytes in 0 blocks ==256010== Reachable blocks (those to which a pointer was found) are not shown. ==256010== To see them, rerun with: --leak-check=full --show-leak-kinds=all ==256010== ==256010== ERROR SUMMARY: 2 errors from 2 contexts (suppressed: 0 from 0) ==256010== ==256010== 1 errors in context 1 of 2: ==256010== Invalid free() / delete / delete[] / realloc() ==256010== at 0x484412F: free (vg_replace_malloc.c:974) ==256010== by 0x490D5B0: patch_parsed__free.part.0.lto_priv.0 (patch_parse.c:1150) ==256010== by 0x48CC454: diff_parsed_free (diff_parse.c:21) ==256010== by 0x1092EA: main (in /home/vincent/Github/libgit2/build/a.out) ==256010== Address 0x59e55b0 is 0 bytes inside a block of size 112 free'd ==256010== at 0x484412F: free (vg_replace_malloc.c:974) ==256010== by 0x48A17CD: git_vector_free_deep.part.0 (vector.c:99) ==256010== by 0x48D5A2B: UnknownInlinedFun (vector.c:95) ==256010== by 0x48D5A2B: git_diff__merge (diff_tform.c:193) ==256010== by 0x1092A6: main (in /home/vincent/Github/libgit2/build/a.out) ==256010== Block was alloc'd at ==256010== at 0x48469B3: calloc (vg_replace_malloc.c:1554) ==256010== by 0x489652D: stdalloc__calloc.lto_priv.0 (stdalloc.c:42) ==256010== by 0x48CCDE9: UnknownInlinedFun (patch_parse.c:1184) ==256010== by 0x48CCDE9: git_diff_from_buffer (diff_parse.c:86) ==256010== by 0x109219: main (in /home/vincent/Github/libgit2/build/a.out) ==256010== ==256010== ERROR SUMMARY: 2 errors from 2 contexts (suppressed: 0 from 0) ``` Link: libgit2#6588 Reported-by: Eric Huss Signed-off-by: Vincenzo Palazzo <vincenzopalazzodev@gmail.com>

ehuss mentioned this issue Jul 8, 2023

Diff::merge() causes double free rust-lang/git2-rs#964

Open

vincenzopalazzo linked a pull request Jul 9, 2023 that will close this issue

merge: Fix double free issue after merging the diffs #6589

Open

ethomson added bug v.next labels Mar 11, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Double free after calling git_diff_merge #6588

Double free after calling git_diff_merge #6588

ehuss commented Jul 8, 2023

vincenzopalazzo commented Jul 10, 2023

Double free after calling git_diff_merge #6588

Double free after calling git_diff_merge #6588

Comments

ehuss commented Jul 8, 2023

Reproduction steps

Expected behavior

Actual behavior

Version of libgit2 (release number or SHA1)

Operating system(s) tested

vincenzopalazzo commented Jul 10, 2023