[git commit] xz: fix incorrect XZ_BUF_ERROR

Denys Vlasenko vda.linux at googlemail.com
Wed Feb 27 15:39:56 UTC 2013


commit: http://git.busybox.net/busybox/commit/?id=c3045edec221876596fe910901191720d8f55986
branch: http://git.busybox.net/busybox/commit/?id=refs/heads/master

xz_dec_run() could incorrectly return XZ_BUF_ERROR if
all of the following was true:

  - The caller knows how many bytes of output to expect
    and only provides that much output space.

  - When the last output bytes are decoded, the
    caller-provided input buffer ends right before
    the LZMA2 end of payload marker. So LZMA2 won't
    provide more output anymore, but it won't know it
    yet and thus won't return XZ_STREAM_END yet.

  - A BCJ filter is in use and it hasn't left any
    unfiltered bytes in the temp buffer. This can happen
    with any BCJ filter, but in practice it's more likely
    with filters other than the x86 BCJ.

This fixes <https://bugzilla.redhat.com/show_bug.cgi?id=735408>
where Squashfs thinks that a valid file system is corrupt.
Thanks to Jindrich Novy for telling me that such a bug report
exists, Phillip Lougher for providing excellent debug info,
and other people on #fedora-ppc.

This also fixes a similar bug in single-call mode where the
uncompressed size of a XZ Block using BCJ + LZMA2 was 0 bytes
and caller provided no output space. Many empty .xz files
don't contain any Blocks and thus don't trigger this bug.

This also tweaks a closely related detail: xz_dec_bcj_run()
could call xz_dec_lzma2_run() to decode into temp buffer when
it was known to be useless. This was harmless although it
wasted a minuscule number of CPU cycles.

Signed-off-by: Lasse Collin <lasse.collin at tukaani.org>
Signed-off-by: Denys Vlasenko <vda.linux at googlemail.com>
---
 archival/libarchive/unxz/xz_dec_bcj.c |   27 ++++++++++++++++++++-------
 1 files changed, 20 insertions(+), 7 deletions(-)

diff --git a/archival/libarchive/unxz/xz_dec_bcj.c b/archival/libarchive/unxz/xz_dec_bcj.c
index a01a4cd..e0f913a 100644
--- a/archival/libarchive/unxz/xz_dec_bcj.c
+++ b/archival/libarchive/unxz/xz_dec_bcj.c
@@ -446,8 +446,12 @@ XZ_EXTERN enum xz_ret XZ_FUNC xz_dec_bcj_run(struct xz_dec_bcj *s,
 	 * next filter in the chain. Apply the BCJ filter on the new data
 	 * in the output buffer. If everything cannot be filtered, copy it
 	 * to temp and rewind the output buffer position accordingly.
+	 *
+	 * This needs to be always run when temp.size == 0 to handle a special
+	 * case where the output buffer is full and the next filter has no
+	 * more output coming but hasn't returned XZ_STREAM_END yet.
 	 */
-	if (s->temp.size < b->out_size - b->out_pos) {
+	if (s->temp.size < b->out_size - b->out_pos || s->temp.size == 0) {
 		out_start = b->out_pos;
 		memcpy(b->out + b->out_pos, s->temp.buf, s->temp.size);
 		b->out_pos += s->temp.size;
@@ -470,16 +474,25 @@ XZ_EXTERN enum xz_ret XZ_FUNC xz_dec_bcj_run(struct xz_dec_bcj *s,
 		s->temp.size = b->out_pos - out_start;
 		b->out_pos -= s->temp.size;
 		memcpy(s->temp.buf, b->out + b->out_pos, s->temp.size);
+
+		/*
+		 * If there wasn't enough input to the next filter to fill
+		 * the output buffer with unfiltered data, there's no point
+		 * to try decoding more data to temp.
+		 */
+		if (b->out_pos + s->temp.size < b->out_size)
+			return XZ_OK;
 	}
 
 	/*
-	 * If we have unfiltered data in temp, try to fill by decoding more
-	 * data from the next filter. Apply the BCJ filter on temp. Then we
-	 * hopefully can fill the actual output buffer by copying filtered
-	 * data from temp. A mix of filtered and unfiltered data may be left
-	 * in temp; it will be taken care on the next call to this function.
+	 * We have unfiltered data in temp. If the output buffer isn't full
+	 * yet, try to fill the temp buffer by decoding more data from the
+	 * next filter. Apply the BCJ filter on temp. Then we hopefully can
+	 * fill the actual output buffer by copying filtered data from temp.
+	 * A mix of filtered and unfiltered data may be left in temp; it will
+	 * be taken care on the next call to this function.
 	 */
-	if (s->temp.size > 0) {
+	if (b->out_pos < b->out_size) {
 		/* Make b->out{,_pos,_size} temporarily point to s->temp. */
 		s->out = b->out;
 		s->out_pos = b->out_pos;


More information about the busybox-cvs mailing list