[PATCH] apply: don't segfault on binary files with missing data

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

 



Usually when applying a binary diff generated without
--binary, it will be rejected early, as we don't even have
the full sha1 of the pre- and post-images.

However, if the diff is generated with --full-index (but not
--binary), then we will actually try to apply it. If we have
the postimage blob, then we can take a shortcut and never
even look at the binary diff at all (e.g., this can happen
when rebasing changes within a repository).

If we don't have the postimage blob, though, we try to look
at the actual fragments, of which there are none, and get a
segfault. This patch checks explicitly for that case and
complains to the user instead of segfaulting. We need to
keep the check at a low level so that the "shortcut" case
above continues to work.

We also add a test that demonstrates the segfault. While
we're at it, let's also explicitly test the shortcut case.

Reported-by: RafaÃl Carrà <rafael.carre@xxxxxxxxx>
Signed-off-by: Jeff King <peff@xxxxxxxx>
---
 builtin/apply.c         |    6 ++++++
 t/t4103-apply-binary.sh |   27 ++++++++++++++++++++++++++-
 2 files changed, 32 insertions(+), 1 deletions(-)

diff --git a/builtin/apply.c b/builtin/apply.c
index 23c18c5..f051e66 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -2645,6 +2645,12 @@ static int apply_binary_fragment(struct image *img, struct patch *patch)
 	unsigned long len;
 	void *dst;
 
+	if (!fragment)
+		return error("missing binary patch data for '%s'",
+			     patch->new_name ?
+			     patch->new_name :
+			     patch->old_name);
+
 	/* Binary patch is irreversible without the optional second hunk */
 	if (apply_in_reverse) {
 		if (!fragment->next)
diff --git a/t/t4103-apply-binary.sh b/t/t4103-apply-binary.sh
index 9692f16..08ad6d8 100755
--- a/t/t4103-apply-binary.sh
+++ b/t/t4103-apply-binary.sh
@@ -37,7 +37,16 @@ test_expect_success 'setup' "
 	git diff-tree -p -C master binary >C.diff &&
 
 	git diff-tree -p --binary master binary >BF.diff &&
-	git diff-tree -p --binary -C master binary >CF.diff
+	git diff-tree -p --binary -C master binary >CF.diff &&
+
+	git diff-tree -p --full-index master binary >B-index.diff &&
+	git diff-tree -p -C --full-index master binary >C-index.diff &&
+
+	git init other-repo &&
+	(cd other-repo &&
+	 git fetch .. master &&
+	 git reset --hard FETCH_HEAD
+	)
 "
 
 test_expect_success 'stat binary diff -- should not fail.' \
@@ -100,6 +109,22 @@ test_expect_success 'apply binary diff (copy) -- should fail.' \
 	'do_reset &&
 	 test_must_fail git apply --index C.diff'
 
+test_expect_success 'apply binary diff with full-index' '
+	do_reset &&
+	git apply B-index.diff
+'
+
+test_expect_success 'apply binary diff with full-index (copy)' '
+	do_reset &&
+	git apply C-index.diff
+'
+
+test_expect_success 'apply full-index binary diff in new repo' '
+	(cd other-repo &&
+	 do_reset &&
+	 test_must_fail git apply ../B-index.diff)
+'
+
 test_expect_success 'apply binary diff without replacement.' \
 	'do_reset &&
 	 git apply BF.diff'
-- 
1.7.3.1.227.ge6319.dirty
--
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]