diff mbox

busybox: add another upstream patch to fix unzip

Message ID 1449001829-26507-1-git-send-email-arnout@mind.be
State Accepted
Commit 63fdab6b4fcc860abd8dbc8342c7b01643a37e1a
Headers show

Commit Message

Arnout Vandecappelle Dec. 1, 2015, 8:30 p.m. UTC 
0002-unzip.patch which was added in 69516e0 to fix a segmentation
fault in the gunzip applet. However, it introduced a new issue that
made the unzipping of some files fail.

Add an upstream patch that fixes this new issue.

Fixes #8501.

Signed-off-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
Cc: Jason Rush <rush0033@hotmail.com>
---
Jason, could you apply this patch to your buildroot tree, rebuild
busybox, and add your Tested-by if it solves the issue?
---
 .../busybox/0003-g-unzip-fix-recent-breakage.patch | 134 +++++++++++++++++++++
 1 file changed, 134 insertions(+)
 create mode 100644 package/busybox/0003-g-unzip-fix-recent-breakage.patch

Comments

Thomas Petazzoni Dec. 1, 2015, 9:30 p.m. UTC | #1 
Arnout, Peter,

On Tue, 1 Dec 2015 21:30:29 +0100, Arnout Vandecappelle
(Essensium/Mind) wrote:
> 0002-unzip.patch which was added in 69516e0 to fix a segmentation
> fault in the gunzip applet. However, it introduced a new issue that
> made the unzipping of some files fail.
> 
> Add an upstream patch that fixes this new issue.
> 
> Fixes #8501.
> 
> Signed-off-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
> Cc: Jason Rush <rush0033@hotmail.com>

It is really nasty to have this problem affecting 2015.11 in such a
fundamental package IMO, so I would like to suggest to merge this and
release 2015.11.1 with the fix.

Sad to have to do this just one day after 2015.11 release, but it's
really not great to have such a major issue left in 2015.11.

Best regards,

Thomas
Peter Korsgaard Dec. 1, 2015, 9:55 p.m. UTC | #2 
>>>>> "Thomas" == Thomas Petazzoni <thomas.petazzoni@free-electrons.com> writes:

 > Arnout, Peter,
 > On Tue, 1 Dec 2015 21:30:29 +0100, Arnout Vandecappelle
 > (Essensium/Mind) wrote:
 >> 0002-unzip.patch which was added in 69516e0 to fix a segmentation
 >> fault in the gunzip applet. However, it introduced a new issue that
 >> made the unzipping of some files fail.
 >> 
 >> Add an upstream patch that fixes this new issue.
 >> 
 >> Fixes #8501.
 >> 
 >> Signed-off-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
 >> Cc: Jason Rush <rush0033@hotmail.com>

 > It is really nasty to have this problem affecting 2015.11 in such a
 > fundamental package IMO, so I would like to suggest to merge this and
 > release 2015.11.1 with the fix.

 > Sad to have to do this just one day after 2015.11 release, but it's
 > really not great to have such a major issue left in 2015.11.

No, indeed not :/ And the patch was even an upstream fix. Where does
this new patch come from, I don't see it on
http://busybox.net/downloads/fixes-1.24.1/?

While this is an important thing to fix, I don't think it is urgent
enough to release a 2015.11.1 right away. It sounds like we might need a
fix for the host rpath checks as well, so I'll wait a few days to see
what shows up.
Arnout Vandecappelle Dec. 1, 2015, 10:08 p.m. UTC | #3 
On 01-12-15 22:55, Peter Korsgaard wrote:
>>>>>> "Thomas" == Thomas Petazzoni <thomas.petazzoni@free-electrons.com> writes:
> 
>  > Arnout, Peter,
>  > On Tue, 1 Dec 2015 21:30:29 +0100, Arnout Vandecappelle
>  > (Essensium/Mind) wrote:
>  >> 0002-unzip.patch which was added in 69516e0 to fix a segmentation
>  >> fault in the gunzip applet. However, it introduced a new issue that
>  >> made the unzipping of some files fail.
>  >> 
>  >> Add an upstream patch that fixes this new issue.
>  >> 
>  >> Fixes #8501.
>  >> 
>  >> Signed-off-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
>  >> Cc: Jason Rush <rush0033@hotmail.com>
> 
>  > It is really nasty to have this problem affecting 2015.11 in such a
>  > fundamental package IMO, so I would like to suggest to merge this and
>  > release 2015.11.1 with the fix.
> 
>  > Sad to have to do this just one day after 2015.11 release, but it's
>  > really not great to have such a major issue left in 2015.11.
> 
> No, indeed not :/ And the patch was even an upstream fix. Where does
> this new patch come from, I don't see it on
> http://busybox.net/downloads/fixes-1.24.1/?

 I took it straight from git.

 Regards,
 Arnout

> 
> While this is an important thing to fix, I don't think it is urgent
> enough to release a 2015.11.1 right away. It sounds like we might need a
> fix for the host rpath checks as well, so I'll wait a few days to see
> what shows up.
>
Peter Korsgaard Dec. 1, 2015, 10:33 p.m. UTC | #4 
>>>>> "Arnout" == Arnout Vandecappelle <arnout@mind.be> writes:

Hi,

 >> > Sad to have to do this just one day after 2015.11 release, but it's
 >> > really not great to have such a major issue left in 2015.11.
 >> 
 >> No, indeed not :/ And the patch was even an upstream fix. Where does
 >> this new patch come from, I don't see it on
 >> http://busybox.net/downloads/fixes-1.24.1/?

 >  I took it straight from git.

Ahh, saw it now. It would be good to get Denys to add it to the fixes
directory as well.

How easy is it to trigger? I did a quick test in qemu and wasn't able to
trigger it.
Mike Frysinger Dec. 2, 2015, 5:20 a.m. UTC | #5 
On 01 Dec 2015 23:33, Peter Korsgaard wrote:
> >>>>> "Arnout" == Arnout Vandecappelle <arnout@mind.be> writes:
>  >> > Sad to have to do this just one day after 2015.11 release, but it's
>  >> > really not great to have such a major issue left in 2015.11.
>  >> 
>  >> No, indeed not :/ And the patch was even an upstream fix. Where does
>  >> this new patch come from, I don't see it on
>  >> http://busybox.net/downloads/fixes-1.24.1/?
> 
>  >  I took it straight from git.
> 
> Ahh, saw it now. It would be good to get Denys to add it to the fixes
> directory as well.

done
-mike
Peter Korsgaard Dec. 2, 2015, 6:24 a.m. UTC | #6 
>>>>> "Mike" == Mike Frysinger <vapier@gentoo.org> writes:

Hi,

 >> Ahh, saw it now. It would be good to get Denys to add it to the fixes
 >> directory as well.

 > done

Thanks, mike!
Peter Korsgaard Dec. 2, 2015, 10:48 p.m. UTC | #7 
>>>>> "Arnout" == Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be> writes:

 > 0002-unzip.patch which was added in 69516e0 to fix a segmentation
 > fault in the gunzip applet. However, it introduced a new issue that
 > made the unzipping of some files fail.

 > Add an upstream patch that fixes this new issue.

 > Fixes #8501.

 > Signed-off-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
 > Cc: Jason Rush <rush0033@hotmail.com>
 > ---
 > Jason, could you apply this patch to your buildroot tree, rebuild
 > busybox, and add your Tested-by if it solves the issue?

Committed after changing the subject to use (g)unzip, thanks.

I'll also cherry pick it for the 2015.11.x branch.
diff mbox

Patch

diff --git a/package/busybox/0003-g-unzip-fix-recent-breakage.patch b/package/busybox/0003-g-unzip-fix-recent-breakage.patch
new file mode 100644
index 0000000..061e2c4
--- /dev/null
+++ b/package/busybox/0003-g-unzip-fix-recent-breakage.patch
@@ -0,0 +1,134 @@ 
+From 6bd3fff51aa74e2ee2d87887b12182a3b09792ef Mon Sep 17 00:00:00 2001
+From: Denys Vlasenko <vda.linux@googlemail.com>
+Date: Fri, 30 Oct 2015 23:41:53 +0100
+Subject: [PATCH] [g]unzip: fix recent breakage.
+
+Also, do emit error message we so painstakingly pass from gzip internals
+
+Signed-off-by: Denys Vlasenko <vda.linux@googlemail.com>
+Signed-off-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
+---
+ archival/libarchive/decompress_gunzip.c | 33 +++++++++++++++++++++------------
+ testsuite/unzip.tests                   |  1 +
+ 2 files changed, 22 insertions(+), 12 deletions(-)
+
+diff --git a/archival/libarchive/decompress_gunzip.c b/archival/libarchive/decompress_gunzip.c
+index 30bf451..20e4d9a 100644
+--- a/archival/libarchive/decompress_gunzip.c
++++ b/archival/libarchive/decompress_gunzip.c
+@@ -309,8 +309,7 @@ static int huft_build(const unsigned *b, const unsigned n,
+ 	huft_t *q;              /* points to current table */
+ 	huft_t r;               /* table entry for structure assignment */
+ 	huft_t *u[BMAX];        /* table stack */
+-	unsigned v[N_MAX];      /* values in order of bit length */
+-	unsigned v_end;
++	unsigned v[N_MAX + 1];  /* values in order of bit length. last v[] is never used */
+ 	int ws[BMAX + 1];       /* bits decoded stack */
+ 	int w;                  /* bits decoded */
+ 	unsigned x[BMAX + 1];   /* bit offsets, then code stack */
+@@ -365,15 +364,17 @@ static int huft_build(const unsigned *b, const unsigned n,
+ 		*xp++ = j;
+ 	}
+ 
+-	/* Make a table of values in order of bit lengths */
++	/* Make a table of values in order of bit lengths.
++	 * To detect bad input, unused v[i]'s are set to invalid value UINT_MAX.
++	 * In particular, last v[i] is never filled and must not be accessed.
++	 */
++	memset(v, 0xff, sizeof(v));
+ 	p = b;
+ 	i = 0;
+-	v_end = 0;
+ 	do {
+ 		j = *p++;
+ 		if (j != 0) {
+ 			v[x[j]++] = i;
+-			v_end = x[j];
+ 		}
+ 	} while (++i < n);
+ 
+@@ -435,7 +436,9 @@ static int huft_build(const unsigned *b, const unsigned n,
+ 
+ 			/* set up table entry in r */
+ 			r.b = (unsigned char) (k - w);
+-			if (p >= v + v_end) { // Was "if (p >= v + n)" but v[] can be shorter!
++			if (/*p >= v + n || -- redundant, caught by the second check: */
++			    *p == UINT_MAX /* do we access uninited v[i]? (see memset(v))*/
++			) {
+ 				r.e = 99; /* out of values--invalid code */
+ 			} else if (*p < s) {
+ 				r.e = (unsigned char) (*p < 256 ? 16 : 15);	/* 256 is EOB code */
+@@ -520,8 +523,9 @@ static NOINLINE int inflate_codes(STATE_PARAM_ONLY)
+ 		e = t->e;
+ 		if (e > 16)
+ 			do {
+-				if (e == 99)
+-					abort_unzip(PASS_STATE_ONLY);;
++				if (e == 99) {
++					abort_unzip(PASS_STATE_ONLY);
++				}
+ 				bb >>= t->b;
+ 				k -= t->b;
+ 				e -= 16;
+@@ -557,8 +561,9 @@ static NOINLINE int inflate_codes(STATE_PARAM_ONLY)
+ 			e = t->e;
+ 			if (e > 16)
+ 				do {
+-					if (e == 99)
++					if (e == 99) {
+ 						abort_unzip(PASS_STATE_ONLY);
++					}
+ 					bb >>= t->b;
+ 					k -= t->b;
+ 					e -= 16;
+@@ -824,8 +829,9 @@ static int inflate_block(STATE_PARAM smallint *e)
+ 
+ 		b_dynamic >>= 4;
+ 		k_dynamic -= 4;
+-		if (nl > 286 || nd > 30)
++		if (nl > 286 || nd > 30) {
+ 			abort_unzip(PASS_STATE_ONLY);	/* bad lengths */
++		}
+ 
+ 		/* read in bit-length-code lengths */
+ 		for (j = 0; j < nb; j++) {
+@@ -906,12 +912,14 @@ static int inflate_block(STATE_PARAM smallint *e)
+ 		bl = lbits;
+ 
+ 		i = huft_build(ll, nl, 257, cplens, cplext, &inflate_codes_tl, &bl);
+-		if (i != 0)
++		if (i != 0) {
+ 			abort_unzip(PASS_STATE_ONLY);
++		}
+ 		bd = dbits;
+ 		i = huft_build(ll + nl, nd, 0, cpdist, cpdext, &inflate_codes_td, &bd);
+-		if (i != 0)
++		if (i != 0) {
+ 			abort_unzip(PASS_STATE_ONLY);
++		}
+ 
+ 		/* set up data for inflate_codes() */
+ 		inflate_codes_setup(PASS_STATE bl, bd);
+@@ -999,6 +1007,7 @@ inflate_unzip_internal(STATE_PARAM transformer_state_t *xstate)
+ 	error_msg = "corrupted data";
+ 	if (setjmp(error_jmp)) {
+ 		/* Error from deep inside zip machinery */
++		bb_error_msg(error_msg);
+ 		n = -1;
+ 		goto ret;
+ 	}
+diff --git a/testsuite/unzip.tests b/testsuite/unzip.tests
+index ca0a458..d8738a3 100755
+--- a/testsuite/unzip.tests
++++ b/testsuite/unzip.tests
+@@ -34,6 +34,7 @@ rm foo.zip
+ testing "unzip (bad archive)" "uudecode; unzip bad.zip 2>&1; echo \$?" \
+ "Archive:  bad.zip
+   inflating: ]3j½r«IK-%Ix
++unzip: corrupted data
+ unzip: inflate error
+ 1
+ " \
+-- 
+2.6.2
+