{"thread":{"id":"6124","subject":"[PATCH] Fix random segfaults in pack-objects.","startedAt":"2006-12-27T07:46:23Z","lastAt":"2006-12-27T07:46:23Z","messageCount":1,"participants":["Shawn O. Pearce"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"30333","messageId":"20061227074623.GA23165@spearce.org","threadId":"6124","inReplyTo":null,"subject":"[PATCH] Fix random segfaults in pack-objects.","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2006-12-27T07:46:23Z","receivedAt":"2006-12-27T07:46:23Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Junio noticed that 'non-trivial' pushes were failing if executed\nusing the sliding window mmap changes.  This was somewhat difficult\nto track down as the failure was appearing randomly.\n\nIt turns out this was a failure caused by the delta base reference\n(either ref or offset format) spanning over the end of a mmap window.\n\nThe error in pack-objects was we were not recalling use_pack\nafter the object header was unpacked, and therefore we did not\nget the promise of at least 20 bytes in the buffer for the delta\nbase parsing.  This would case later memcmp() calls to walk into\nunassigned address space at the end of the window.\n\nThe reason Junio and I had hard time tracking this down in current\nGit repositories is we were both probably packing with offset deltas,\nwhich minimized the odds of the delta base reference spanning over\nthe end of the mmap window.  Stepping back and repacking with\nversion 1.3.3 (which only supported reference deltas) increased\nthe likelyhood of seeing the bug.\n\nThe correct technique (as used in sha1_file.c) is to invoke\nuse_pack() after unpack_object_header_gently to ensure we have\nenough data available for the delta base decoding.\n\nSigned-off-by: Shawn O. Pearce <spearce@spearce.org>\n---\n\n Applies to the top of my sp/mmap topic.\n\n builtin-pack-objects.c |    7 ++++---\n 1 files changed, 4 insertions(+), 3 deletions(-)\n\ndiff --git a/builtin-pack-objects.c b/builtin-pack-objects.c\nindex 93eefc4..31ebf5c 100644\n--- a/builtin-pack-objects.c\n+++ b/builtin-pack-objects.c\n@@ -995,8 +995,6 @@ static void check_object(struct object_entry *entry)\n \t\t */\n \t\tused = unpack_object_header_gently(buf, left,\n \t\t\t\t\t\t   &entry->in_pack_type, &size);\n-\t\tif (!used || left - used <= 20)\n-\t\t\tdie(\"corrupt pack for %s\", sha1_to_hex(entry->sha1));\n \n \t\t/* Check if it is delta, and the base is also an object\n \t\t * we are going to pack.  If so we will reuse the existing\n@@ -1008,10 +1006,13 @@ static void check_object(struct object_entry *entry)\n \t\t\t/* there is at least 20 bytes left in the pack */\n \t\t\tswitch (entry->in_pack_type) {\n \t\t\tcase OBJ_REF_DELTA:\n-\t\t\t\tbase_name = buf + used;\n+\t\t\t\tbase_name = use_pack(p, &w_curs,\n+\t\t\t\t\tentry->in_pack_offset + used, NULL);\n \t\t\t\tused += 20;\n \t\t\t\tbreak;\n \t\t\tcase OBJ_OFS_DELTA:\n+\t\t\t\tbuf = use_pack(p, &w_curs,\n+\t\t\t\t\tentry->in_pack_offset + used, NULL);\n \t\t\t\tc = buf[used++];\n \t\t\t\tofs = c & 127;\n \t\t\t\twhile (c & 128) {\n-- \n1.4.4.3.gd2e4\n"}]}