[PATCH] Fix random segfaults in pack-objects.

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Junio noticed that 'non-trivial' pushes were failing if executed
using the sliding window mmap changes.  This was somewhat difficult
to track down as the failure was appearing randomly.

It turns out this was a failure caused by the delta base reference
(either ref or offset format) spanning over the end of a mmap window.

The error in pack-objects was we were not recalling use_pack
after the object header was unpacked, and therefore we did not
get the promise of at least 20 bytes in the buffer for the delta
base parsing.  This would case later memcmp() calls to walk into
unassigned address space at the end of the window.

The reason Junio and I had hard time tracking this down in current
Git repositories is we were both probably packing with offset deltas,
which minimized the odds of the delta base reference spanning over
the end of the mmap window.  Stepping back and repacking with
version 1.3.3 (which only supported reference deltas) increased
the likelyhood of seeing the bug.

The correct technique (as used in sha1_file.c) is to invoke
use_pack() after unpack_object_header_gently to ensure we have
enough data available for the delta base decoding.

Signed-off-by: Shawn O. Pearce <spearce@xxxxxxxxxxx>
---

 Applies to the top of my sp/mmap topic.

 builtin-pack-objects.c |    7 ++++---
 1 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/builtin-pack-objects.c b/builtin-pack-objects.c
index 93eefc4..31ebf5c 100644
--- a/builtin-pack-objects.c
+++ b/builtin-pack-objects.c
@@ -995,8 +995,6 @@ static void check_object(struct object_entry *entry)
 		 */
 		used = unpack_object_header_gently(buf, left,
 						   &entry->in_pack_type, &size);
-		if (!used || left - used <= 20)
-			die("corrupt pack for %s", sha1_to_hex(entry->sha1));
 
 		/* Check if it is delta, and the base is also an object
 		 * we are going to pack.  If so we will reuse the existing
@@ -1008,10 +1006,13 @@ static void check_object(struct object_entry *entry)
 			/* there is at least 20 bytes left in the pack */
 			switch (entry->in_pack_type) {
 			case OBJ_REF_DELTA:
-				base_name = buf + used;
+				base_name = use_pack(p, &w_curs,
+					entry->in_pack_offset + used, NULL);
 				used += 20;
 				break;
 			case OBJ_OFS_DELTA:
+				buf = use_pack(p, &w_curs,
+					entry->in_pack_offset + used, NULL);
 				c = buf[used++];
 				ofs = c & 127;
 				while (c & 128) {
-- 
1.4.4.3.gd2e4
-
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]