{"thread":{"id":"25471","subject":"[CRASH] git segfaults on invalid binary diff","startedAt":"2010-10-18T16:06:32Z","lastAt":"2010-10-18T18:39:17Z","messageCount":3,"participants":["Rafaël Carré","Jeff King"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"153720","messageId":"20101018180632.7eb40628@wai","threadId":"25471","inReplyTo":null,"subject":"[CRASH] git segfaults on invalid binary diff","fromName":"Rafaël Carré","fromEmail":"rafael.carre@gmail.com","sentAt":"2010-10-18T16:06:32Z","receivedAt":"2010-10-18T16:06:32Z","isPatch":false,"sender":{"key":"rafael.carre@gmail.com","avatar":null},"body":"Hi,\n\nI noticed git crashed on a patch generated with:\ngit show --full-index --pretty=email \n\n(I had forgotten --binary)\n\n% git --version\ngit version 1.7.3.1.120.g38a18\n% mkdir test\n% cd test\n% git init\nInitialized empty Git repository in /home/fun/test/.git/\n% git am ~/bad.diff\nApplying: foo bar\nSegmentation fault (core dumped)\nPatch failed at 0001 foo bar\nWhen you have resolved this problem run \"git am --resolved\".\nIf you would prefer to skip this patch, instead run \"git am --skip\".\nTo restore the original branch and stop patching run \"git am --abort\".\n% cat ~/bad.diff\nFrom 2e888fde246ecb0d346ecc02e7df0c82724f02b9 Mon Sep 17 00:00:00 2001\nFrom: John Doe \nDate: Mon, 4 Feb 2008 01:17:33 +0000\nSubject: [PATCH] foo\n bar\n\n\ndiff --git a/axvlc.tlb b/axvlc.tlb\nnew file mode 100644\nindex 0000000000000000000000000000000000000000..a61182de2f3eb2dc2fc13a08b448b79ae56f1967\nBinary files /dev/null and b/axvlc.tlb differ\n% gdb git core\nGNU gdb (GDB) 7.2-ubuntu\nCopyright (C) 2010 Free Software Foundation, Inc.\nLicense GPLv3+: GNU GPL version 3 or later \nThis is free software: you are free to change and redistribute it.\nThere is NO WARRANTY, to the extent permitted by law. Type \"show copying\"\nand \"show warranty\" for details.\nThis GDB was configured as \"x86_64-linux-gnu\".\nFor bug reporting instructions, please see:\n...\nReading symbols from /home/fun/.local/git/bin/git...done.\n[New Thread 24102]\n\nwarning: Can't read pathname for load map: Erreur d'entrée/sortie.\nReading symbols from /lib/libz.so.1...(no debugging symbols found)...done.\nLoaded symbols for /lib/libz.so.1\nReading symbols from /lib/libpthread.so.0...Reading symbols from /usr/lib/debug/lib/libpthread-2.12.1.so...done.\ndone.\nLoaded symbols for /lib/libpthread.so.0\nReading symbols from /lib/libc.so.6...Reading symbols from /usr/lib/debug/lib/libc-2.12.1.so...done.\ndone.\nLoaded symbols for /lib/libc.so.6\nReading symbols from /lib64/ld-linux-x86-64.so.2...Reading symbols from /usr/lib/debug/lib/ld-2.12.1.so...done.\ndone.\nLoaded symbols for /lib64/ld-linux-x86-64.so.2\nCore was generated by `git apply --index /home/fun/test/.git/rebase-apply/patch'.\nProgram terminated with signal 11, Segmentation fault.\n#0 0x000000000040c49e in apply_binary_fragment (img=0x7fff47152cd0, \n patch=0x16ef2a0) at builtin/apply.c:2657\n2657\t\tswitch (fragment->binary_patch_method) {\n(gdb) bt\n#0 0x000000000040c49e in apply_binary_fragment (img=0x7fff47152cd0, \n patch=0x16ef2a0) at builtin/apply.c:2657\n#1 0x000000000040c7ef in apply_binary (img=0x7fff47152cd0, patch=0x16ef2a0)\n at builtin/apply.c:2739\n#2 0x000000000040c92f in apply_fragments (img=0x7fff47152cd0, patch=0x16ef2a0)\n at builtin/apply.c:2761\n#3 0x000000000040cdc5 in apply_data (patch=0x16ef2a0, st=0x7fff47152d60, \n ce=0x0) at builtin/apply.c:2908\n#4 0x000000000040d64a in check_patch (patch=0x16ef2a0) at builtin/apply.c:3098\n#5 0x000000000040d6d7 in check_patch_list (patch=0x16ef2a0)\n at builtin/apply.c:3113\n#6 0x000000000040ef6d in apply_patch (fd=3, \n filename=0x7fff471549e7 \"/home/fun/test/.git/rebase-apply/patch\", \n options=0) at builtin/apply.c:3683\n#7 0x000000000040faee in cmd_apply (argc=1, argv=0x7fff471538a0, prefix_=0x0)\n at builtin/apply.c:3897\n#8 0x00000000004048c5 in run_builtin (p=0x766688, argc=3, argv=0x7fff471538a0)\n at git.c:275\n#9 0x0000000000404a60 in handle_internal_command (argc=3, argv=0x7fff471538a0)\n at git.c:431\n#10 0x0000000000404b5a in run_argv (argcp=0x7fff4715378c, argv=0x7fff47153780)\n at git.c:475\n#11 0x0000000000404ce2 in main (argc=3, argv=0x7fff471538a0) at git.c:548\n(gdb) frame 0\n#0 0x000000000040c49e in apply_binary_fragment (img=0x7fff47152cd0, \n patch=0x16ef2a0) at builtin/apply.c:2657\n2657\t\tswitch (fragment->binary_patch_method) {\n(gdb) print *patch\n$1 = {new_name = 0x16ec0e0 \"axvlc.tlb\", old_name = 0x0, \n def_name = 0x16ec0e0 \"axvlc.tlb\", old_mode = 0, new_mode = 33188, \n is_new = 1, is_delete = 0, rejected = 1, ws_rule = 19, deflate_origlen = 0, \n lines_added = 0, lines_deleted = 0, score = 0, is_toplevel_relative = 1, \n inaccurate_eof = 0, is_binary = 1, is_copy = 0, is_rename = 0, recount = 0, \n fragments = 0x0, result = 0x0, resultsize = 0, \n old_sha1_prefix = '0' I noticed git crashed on a patch generated with:\n> git show --full-index --pretty=email \n> \n> (I had forgotten --binary)\n> \n> % git --version\n> git version 1.7.3.1.120.g38a18\n> % mkdir test\n> % cd test\n> % git init\n> Initialized empty Git repository in /home/fun/test/.git/\n> % git am ~/bad.diff\n> Applying: foo bar\n> Segmentation fault (core dumped)\n> Patch failed at 0001 foo bar\n> When you have resolved this problem run \"git am --resolved\".\n> If you would prefer to skip this patch, instead run \"git am --skip\".\n> To restore the original branch and stop patching run \"git am --abort\".\n> % cat ~/bad.diff\n\nHmm, the key seems to be applying it in a new repo without the actual\nobjects. Git will fall back to trying to apply based on the index line,\nand that is presumably where the bug is. I'll try to work up a patch.\n\n-Peff\n"},{"id":"153740","messageId":"20101018183916.GA21988@sigill.intra.peff.net","threadId":"25471","inReplyTo":"20101018174031.GA9511@sigill.intra.peff.net","subject":"[PATCH] apply: don't segfault on binary files with missing data","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2010-10-18T18:39:17Z","receivedAt":"2010-10-18T18:39:17Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"Usually when applying a binary diff generated without\n--binary, it will be rejected early, as we don't even have\nthe full sha1 of the pre- and post-images.\n\nHowever, if the diff is generated with --full-index (but not\n--binary), then we will actually try to apply it. If we have\nthe postimage blob, then we can take a shortcut and never\neven look at the binary diff at all (e.g., this can happen\nwhen rebasing changes within a repository).\n\nIf we don't have the postimage blob, though, we try to look\nat the actual fragments, of which there are none, and get a\nsegfault. This patch checks explicitly for that case and\ncomplains to the user instead of segfaulting. We need to\nkeep the check at a low level so that the \"shortcut\" case\nabove continues to work.\n\nWe also add a test that demonstrates the segfault. While\nwe're at it, let's also explicitly test the shortcut case.\n\nReported-by: Rafaël Carré \nSigned-off-by: Jeff King \n---\n builtin/apply.c | 6 ++++++\n t/t4103-apply-binary.sh | 27 ++++++++++++++++++++++++++-\n 2 files changed, 32 insertions(+), 1 deletions(-)\n\ndiff --git a/builtin/apply.c b/builtin/apply.c\nindex 23c18c5..f051e66 100644\n--- a/builtin/apply.c\n+++ b/builtin/apply.c\n@@ -2645,6 +2645,12 @@ static int apply_binary_fragment(struct image *img, struct patch *patch)\n \tunsigned long len;\n \tvoid *dst;\n \n+\tif (!fragment)\n+\t\treturn error(\"missing binary patch data for '%s'\",\n+\t\t\t patch->new_name ?\n+\t\t\t patch->new_name :\n+\t\t\t patch->old_name);\n+\n \t/* Binary patch is irreversible without the optional second hunk */\n \tif (apply_in_reverse) {\n \t\tif (!fragment->next)\ndiff --git a/t/t4103-apply-binary.sh b/t/t4103-apply-binary.sh\nindex 9692f16..08ad6d8 100755\n--- a/t/t4103-apply-binary.sh\n+++ b/t/t4103-apply-binary.sh\n@@ -37,7 +37,16 @@ test_expect_success 'setup' \"\n \tgit diff-tree -p -C master binary >C.diff &&\n \n \tgit diff-tree -p --binary master binary >BF.diff &&\n-\tgit diff-tree -p --binary -C master binary >CF.diff\n+\tgit diff-tree -p --binary -C master binary >CF.diff &&\n+\n+\tgit diff-tree -p --full-index master binary >B-index.diff &&\n+\tgit diff-tree -p -C --full-index master binary >C-index.diff &&\n+\n+\tgit init other-repo &&\n+\t(cd other-repo &&\n+\t git fetch .. master &&\n+\t git reset --hard FETCH_HEAD\n+\t)\n \"\n \n test_expect_success 'stat binary diff -- should not fail.' \\\n@@ -100,6 +109,22 @@ test_expect_success 'apply binary diff (copy) -- should fail.' \\\n \t'do_reset &&\n \t test_must_fail git apply --index C.diff'\n \n+test_expect_success 'apply binary diff with full-index' '\n+\tdo_reset &&\n+\tgit apply B-index.diff\n+'\n+\n+test_expect_success 'apply binary diff with full-index (copy)' '\n+\tdo_reset &&\n+\tgit apply C-index.diff\n+'\n+\n+test_expect_success 'apply full-index binary diff in new repo' '\n+\t(cd other-repo &&\n+\t do_reset &&\n+\t test_must_fail git apply ../B-index.diff)\n+'\n+\n test_expect_success 'apply binary diff without replacement.' \\\n \t'do_reset &&\n \t git apply BF.diff'\n-- \n1.7.3.1.227.ge6319.dirty\n"}]}