[PATCH 1/2] xen: always set an error return code on lz4 decompression failures

[PATCH 1/2] xen: always set an error return code on lz4 decompression failures

[Matthew Daley]

Signed-off-by: Matthew Daley <mattjd@gmail.com>
---
 xen/common/unlz4.c |    6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/xen/common/unlz4.c b/xen/common/unlz4.c
index 195d829..ae87f4c 100644
--- a/xen/common/unlz4.c
+++ b/xen/common/unlz4.c
@@ -112,6 +112,7 @@ STATIC int INIT unlz4(unsigned char *input, unsigned int in_len,
 		if (fill) {
 			if (chunksize > lz4_compressbound(uncomp_chunksize)) {
 				error("chunk length is longer than allocated");
+				ret = -1;
 				goto exit_2;
 			}
 			fill(inp, chunksize);
@@ -133,8 +134,10 @@ STATIC int INIT unlz4(unsigned char *input, unsigned int in_len,
 			goto exit_2;
 		}
 
-		if (flush && flush(outp, dest_len) != dest_len)
+		if (flush && flush(outp, dest_len) != dest_len) {
+			ret = -1;
 			goto exit_2;
+		}
 		if (output)
 			outp += dest_len;
 		if (posp)
@@ -146,6 +149,7 @@ STATIC int INIT unlz4(unsigned char *input, unsigned int in_len,
 			break;
 		else if (size < 0) {
 			error("data corrupted");
+			ret = -1;
 			goto exit_2;
 		}
 
-- 
1.7.10.4

[PATCH 2/2] libxc: always set a error return code on lz4 decompression failures

[Matthew Daley]

While at it, rename the exit_2 label to exit_1; there is no exit_1
currently.

Signed-off-by: Matthew Daley <mattjd@gmail.com>
---
 tools/libxc/xc_dom_decompress_lz4.c |   15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/tools/libxc/xc_dom_decompress_lz4.c b/tools/libxc/xc_dom_decompress_lz4.c
index 4787535..b980186 100644
--- a/tools/libxc/xc_dom_decompress_lz4.c
+++ b/tools/libxc/xc_dom_decompress_lz4.c
@@ -69,13 +69,14 @@ int xc_try_lz4_decode(
 		size -= 4;
 	} else {
 		msg = "invalid header";
-		goto exit_2;
+		goto exit_1;
 	}
 
 	for (;;) {
 		if (size < 4) {
 			msg = "missing data";
-			goto exit_2;
+			ret = -1;
+			goto exit_1;
 		}
 		chunksize = get_unaligned_le32(inp);
 		if (chunksize == ARCHIVE_MAGICNUMBER) {
@@ -87,7 +88,8 @@ int xc_try_lz4_decode(
 		size -= 4;
 		if (chunksize > size) {
 			msg = "insufficient input data";
-			goto exit_2;
+			ret = -1;
+			goto exit_1;
 		}
 
 		dest_len = out_len - (outp - output);
@@ -95,7 +97,7 @@ int xc_try_lz4_decode(
 				&dest_len);
 		if (ret < 0) {
 			msg = "decoding failed";
-			goto exit_2;
+			goto exit_1;
 		}
 
 		outp += dest_len;
@@ -110,13 +112,14 @@ int xc_try_lz4_decode(
 
 		if (size < 0) {
 			msg = "data corrupted";
-			goto exit_2;
+			ret = -1;
+			goto exit_1;
 		}
 
 		inp += chunksize;
 	}
 
-exit_2:
+exit_1:
 	free(output);
 exit_0:
 	DOMPRINTF("LZ4 decompression error: %s\n", msg);
-- 
1.7.10.4

Re: [PATCH 1/2] xen: always set an error return code on lz4 decompression failures

[Jan Beulich]

>>> On 08.11.13 at 01:26, Matthew Daley <mattjd@gmail.com> wrote:

As just sent to LKML, I'd like to do this with a one line change
(reproduced below), and by addressing it in the original first (and
us just inheriting the fix). I'd recommend the same for patch 2, as
that also touches a clone of the original code.

Jan

unlz4: always set an error return code on failures

"ret", being set to -1 early on, gets cleared by the first invocation
of lz4_decompress()/lz4_decompress_unknownoutputsize(), and hence
subsequent failures wouldn't be noticed by the caller without setting
it back to -1 right after those calls.

Reported-by: Matthew Daley <mattjd@gmail.com>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Cc: Kyungsik Lee <kyungsik.lee@lge.com>
Cc: Andrew Morton <akpm@linux-foundation.org>

--- a/lib/decompress_unlz4.c
+++ b/lib/decompress_unlz4.c
@@ -141,6 +141,7 @@ STATIC inline int INIT unlz4(u8 *input, 
 			goto exit_2;
 		}
 
+		ret = -1;
 		if (flush && flush(outp, dest_len) != dest_len)
 			goto exit_2;
 		if (output)

[PATCH] xen/unlz4: always set an error return code on failures

[Jan Beulich]

[-- Attachment #1: Type: text/plain, Size: 653 bytes --]

"ret", being set to -1 early on, gets cleared by the first invocation
of lz4_decompress()/lz4_decompress_unknownoutputsize(), and hence
subsequent failures wouldn't be noticed by the caller without setting
it back to -1 right after those calls.

Linux commit: 2a1d689c9ba42a6066540fb221b6ecbd6298b728

Reported-by: Matthew Daley <mattjd@gmail.com>
Signed-off-by: Jan Beulich <jbeulich@suse.com>

--- a/xen/common/unlz4.c
+++ b/xen/common/unlz4.c
@@ -133,6 +133,7 @@ STATIC int INIT unlz4(unsigned char *inp
 			goto exit_2;
 		}
 
+		ret = -1;
 		if (flush && flush(outp, dest_len) != dest_len)
 			goto exit_2;
 		if (output)




[-- Attachment #2: unlz4-errors-dom0.patch --]
[-- Type: text/plain, Size: 705 bytes --]

xen/unlz4: always set an error return code on failures

"ret", being set to -1 early on, gets cleared by the first invocation
of lz4_decompress()/lz4_decompress_unknownoutputsize(), and hence
subsequent failures wouldn't be noticed by the caller without setting
it back to -1 right after those calls.

Linux commit: 2a1d689c9ba42a6066540fb221b6ecbd6298b728

Reported-by: Matthew Daley <mattjd@gmail.com>
Signed-off-by: Jan Beulich <jbeulich@suse.com>

--- a/xen/common/unlz4.c
+++ b/xen/common/unlz4.c
@@ -133,6 +133,7 @@ STATIC int INIT unlz4(unsigned char *inp
 			goto exit_2;
 		}
 
+		ret = -1;
 		if (flush && flush(outp, dest_len) != dest_len)
 			goto exit_2;
 		if (output)

[-- Attachment #3: Type: text/plain, Size: 126 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

[PATCH] libxc/unlz4: always set an error return code on failures

[Jan Beulich]

[-- Attachment #1: Type: text/plain, Size: 629 bytes --]

"ret", being set to -1 early on, gets cleared by the first invocation
of lz4_decompress()/lz4_decompress_unknownoutputsize(), and hence
subsequent failures wouldn't be noticed by the caller without setting
it back to -1 right after those calls.

Linux commit: 2a1d689c9ba42a6066540fb221b6ecbd6298b728

Reported-by: Matthew Daley <mattjd@gmail.com>
Signed-off-by: Jan Beulich <jbeulich@suse.com>

--- a/tools/libxc/xc_dom_decompress_lz4.c
+++ b/tools/libxc/xc_dom_decompress_lz4.c
@@ -98,6 +98,7 @@ int xc_try_lz4_decode(
 			goto exit_2;
 		}
 
+		ret = -1;
 		outp += dest_len;
 		size -= chunksize;
 




[-- Attachment #2: unlz4-errors-domU.patch --]
[-- Type: text/plain, Size: 683 bytes --]

libxc/unlz4: always set an error return code on failures

"ret", being set to -1 early on, gets cleared by the first invocation
of lz4_decompress()/lz4_decompress_unknownoutputsize(), and hence
subsequent failures wouldn't be noticed by the caller without setting
it back to -1 right after those calls.

Linux commit: 2a1d689c9ba42a6066540fb221b6ecbd6298b728

Reported-by: Matthew Daley <mattjd@gmail.com>
Signed-off-by: Jan Beulich <jbeulich@suse.com>

--- a/tools/libxc/xc_dom_decompress_lz4.c
+++ b/tools/libxc/xc_dom_decompress_lz4.c
@@ -98,6 +98,7 @@ int xc_try_lz4_decode(
 			goto exit_2;
 		}
 
+		ret = -1;
 		outp += dest_len;
 		size -= chunksize;
 

[-- Attachment #3: Type: text/plain, Size: 126 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [PATCH] libxc/unlz4: always set an error return code on failures

[Ian Campbell]

On Fri, 2014-01-24 at 08:01 +0000, Jan Beulich wrote:
> "ret", being set to -1 early on, gets cleared by the first invocation
> of lz4_decompress()/lz4_decompress_unknownoutputsize(), and hence
> subsequent failures wouldn't be noticed by the caller without setting
> it back to -1 right after those calls.
> 
> Linux commit: 2a1d689c9ba42a6066540fb221b6ecbd6298b728
> 
> Reported-by: Matthew Daley <mattjd@gmail.com>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

Acked-by: Ian Campbell <ian.campbell@citrix.com>

Re: [PATCH] libxc/unlz4: always set an error return code on failures

[Ian Campbell]

On Fri, 2014-01-24 at 09:22 +0000, Ian Campbell wrote:
> On Fri, 2014-01-24 at 08:01 +0000, Jan Beulich wrote:
> > "ret", being set to -1 early on, gets cleared by the first invocation
> > of lz4_decompress()/lz4_decompress_unknownoutputsize(), and hence
> > subsequent failures wouldn't be noticed by the caller without setting
> > it back to -1 right after those calls.
> > 
> > Linux commit: 2a1d689c9ba42a6066540fb221b6ecbd6298b728
> > 
> > Reported-by: Matthew Daley <mattjd@gmail.com>
> > Signed-off-by: Jan Beulich <jbeulich@suse.com>
> 
> Acked-by: Ian Campbell <ian.campbell@citrix.com>

This is a pretty obvious bug fix, which is already in Linux (where this
code originates), so I've applied it.

I guess you will do the Xen side one yourself.

Ian.

Re: [PATCH] libxc/unlz4: always set an error return code on failures

[Jan Beulich]

>>> On 28.01.14 at 12:49, Ian Campbell <Ian.Campbell@citrix.com> wrote:
> On Fri, 2014-01-24 at 09:22 +0000, Ian Campbell wrote:
>> On Fri, 2014-01-24 at 08:01 +0000, Jan Beulich wrote:
>> > "ret", being set to -1 early on, gets cleared by the first invocation
>> > of lz4_decompress()/lz4_decompress_unknownoutputsize(), and hence
>> > subsequent failures wouldn't be noticed by the caller without setting
>> > it back to -1 right after those calls.
>> > 
>> > Linux commit: 2a1d689c9ba42a6066540fb221b6ecbd6298b728
>> > 
>> > Reported-by: Matthew Daley <mattjd@gmail.com>
>> > Signed-off-by: Jan Beulich <jbeulich@suse.com>
>> 
>> Acked-by: Ian Campbell <ian.campbell@citrix.com>
> 
> This is a pretty obvious bug fix, which is already in Linux (where this
> code originates), so I've applied it.
> 
> I guess you will do the Xen side one yourself.

I was sort of hoping to get an ack from Keir, and wanted to apply
both as a pair. Now that you did the tools side, I guess I'll call it
trivial enough and apply the hypervisor side as is.

Jan

Re: [PATCH] libxc/unlz4: always set an error return code on failures

[Ian Campbell]

On Tue, 2014-01-28 at 12:30 +0000, Jan Beulich wrote:
> >>> On 28.01.14 at 12:49, Ian Campbell <Ian.Campbell@citrix.com> wrote:
> > On Fri, 2014-01-24 at 09:22 +0000, Ian Campbell wrote:
> >> On Fri, 2014-01-24 at 08:01 +0000, Jan Beulich wrote:
> >> > "ret", being set to -1 early on, gets cleared by the first invocation
> >> > of lz4_decompress()/lz4_decompress_unknownoutputsize(), and hence
> >> > subsequent failures wouldn't be noticed by the caller without setting
> >> > it back to -1 right after those calls.
> >> > 
> >> > Linux commit: 2a1d689c9ba42a6066540fb221b6ecbd6298b728
> >> > 
> >> > Reported-by: Matthew Daley <mattjd@gmail.com>
> >> > Signed-off-by: Jan Beulich <jbeulich@suse.com>
> >> 
> >> Acked-by: Ian Campbell <ian.campbell@citrix.com>
> > 
> > This is a pretty obvious bug fix, which is already in Linux (where this
> > code originates), so I've applied it.
> > 
> > I guess you will do the Xen side one yourself.
> 
> I was sort of hoping to get an ack from Keir, and wanted to apply
> both as a pair.

Oops, sorry!

>  Now that you did the tools side, I guess I'll call it
> trivial enough and apply the hypervisor side as is.

FWIW: Acked-By: Ian Campbell <ian.campbell@citrix.com> on that...