2015-04-03 11:34:04

by Krzysztof Kolasa

[permalink] [raw]
Subject: Re: lz4: fix system halted at boot kernel x86_64 compressed lz4

On 31.03.2015 17:22, Greg KH wrote:
> On Wed, Mar 25, 2015 at 08:04:59AM +0100, Krzysztof Kolasa wrote:
>> On 25.03.2015 01:44, David Sterba wrote:
>>> On Tue, Mar 24, 2015 at 12:27:25PM +0100, Krzysztof Kolasa wrote:
>>>> lz4: fix system halted at boot kernel x86_64 compressed lz4
>>>>
>>>> Decompression process ends with an error when loading kernel:
>>>>
>>>> Decoding failed
>>>> -- System halted
>>> Serious regression detected ...
>>>
>>>> This condition is probably not needed ( from the last commit d5e7caf) :
>>> The offending patch is on the way to stable trees, so it would be best
>>> to postpone it for now.
>>>
>>>> if( ... ||
>>>> (op + COPYLENGTH) > oend)
>>>> goto _output_error
>>>>
>>>> macro LZ4_SECURE_COPY() tests op and does not copy any data
>>>> when op exceeds the value, decompression process is continued.
>>>>
>>>> added by analogy security for the ref:
>>>>
>>>> if ((ref + COPYLENGTH) > oend...
>>>>
>>>> to lz4_uncompress_unknownoutputsize(...)
>>> I did only a quick check, your analysis seems correct. Reviewing the lz4
>>> patches is tedious as the kernel implementations do not match the
>>> upstream one line-by-line besides that I've missed the side effects of
>>> the macro.
>>>
>> Add patch source for review (send to LKML) :
>> ---------------------
>>
>> lz4: fix system halted at boot kernel x86_64 compressed lz4
>>
>> Decompression process ends with an error when loading kernel:
>>
>> Decoding failed
>> -- System halted
>>
>> This condition is probably not needed ( from the last commit d5e7caf) :
>>
>> if( ... ||
>> (op + COPYLENGTH) > oend)
>> goto _output_error
>>
>> macro LZ4_SECURE_COPY() tests op and does not copy any data
>> when op exceeds the value, decompression process is continued.
>>
>> added by analogy security for the ref:
>>
>> if ((ref + COPYLENGTH) > oend...
>>
>> to lz4_uncompress_unknownoutputsize(...)
>>
>> Signed-off-by: Krzysztof Kolasa <[email protected]>
>> ---
>> lib/lz4/lz4_decompress.c | 5 +++--
>> 1 file changed, 3 insertions(+), 2 deletions(-)
>>
>> diff --git a/lib/lz4/lz4_decompress.c b/lib/lz4/lz4_decompress.c
>> index f0f5c5c..e248c4e 100644
>> --- a/lib/lz4/lz4_decompress.c
>> +++ b/lib/lz4/lz4_decompress.c
>> @@ -139,8 +139,7 @@ static int lz4_uncompress(const char *source, char *dest, int osize)
>> /* Error: request to write beyond destination buffer */
>> if (cpy > oend)
>> goto _output_error;
>> - if ((ref + COPYLENGTH) > oend ||
>> - (op + COPYLENGTH) > oend)
>> + if ((ref + COPYLENGTH) > oend)
>> goto _output_error;
>> LZ4_SECURECOPY(ref, op, (oend - COPYLENGTH));
>> while (op < cpy)
>> @@ -270,6 +269,8 @@ static int lz4_uncompress_unknownoutputsize(const char *source, char *dest,
>> if (cpy > oend - COPYLENGTH) {
>> if (cpy > oend)
>> goto _output_error; /* write outside of buf */
>> + if ((ref + COPYLENGTH) > oend)
>> + goto _output_error;
>>
>> LZ4_SECURECOPY(ref, op, (oend - COPYLENGTH));
>> while (op < cpy)
>> -- 2.3.3.dirty
> I'm confused, what is the problem here? What went wrong with the
x86_64 lz4 kernel halted system...
> original patch that was posted, which is a mirror of what the lz4 code
> looks like "upstream"?
>
> Why make this change? Does it need to go into 4.0-final, or should I
> just revert the original patch?
>
> confused,
>
> greg k-h
>
OK, after further tests have modified the previous patch, please check and analyze:

[PATCH] lz4: fix system halted at boot kernel x86_64 compressed lz4

Decompression process ends with an error when loading 64bit lz4 kernel:

Decoding failed

-- System halted

This condition is not needed for 64bit kernel ( from the last commit d5e7caf )

if( ... ||
(op + COPYLENGTH) > oend)
goto _output_error

macro LZ4_SECURE_COPY() tests op and does not copy any data
when op exceeds the value, decompression process is continued.

added by analogy to lz4_uncompress_unknownoutputsize(...)

Signed-off-by: Krzysztof Kolasa <[email protected]>
---
lib/lz4/lz4_decompress.c | 12 +++++++++++-
1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/lib/lz4/lz4_decompress.c b/lib/lz4/lz4_decompress.c
index f0f5c5c..8a742b1 100644
--- a/lib/lz4/lz4_decompress.c
+++ b/lib/lz4/lz4_decompress.c
@@ -139,8 +139,12 @@ static int lz4_uncompress(const char *source, char *dest, int osize)
/* Error: request to write beyond destination buffer */
if (cpy > oend)
goto _output_error;
+#if LZ4_ARCH64
+ if ((ref + COPYLENGTH) > oend)
+#else
if ((ref + COPYLENGTH) > oend ||
(op + COPYLENGTH) > oend)
+#endif
goto _output_error;
LZ4_SECURECOPY(ref, op, (oend - COPYLENGTH));
while (op < cpy)
@@ -270,7 +274,13 @@ static int lz4_uncompress_unknownoutputsize(const char *source, char *dest,
if (cpy > oend - COPYLENGTH) {
if (cpy > oend)
goto _output_error; /* write outside of buf */
-
+#if LZ4_ARCH64
+ if ((ref + COPYLENGTH) > oend)
+#else
+ if ((ref + COPYLENGTH) > oend ||
+ (op + COPYLENGTH) > oend)
+#endif
+ goto _output_error;
LZ4_SECURECOPY(ref, op, (oend - COPYLENGTH));
while (op < cpy)
*op++ = *ref++;
--
2.4.0.rc0.dirty


2015-04-03 13:17:37

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: lz4: fix system halted at boot kernel x86_64 compressed lz4

On Fri, Apr 03, 2015 at 01:33:54PM +0200, Krzysztof Kolasa wrote:
> On 31.03.2015 17:22, Greg KH wrote:
> > On Wed, Mar 25, 2015 at 08:04:59AM +0100, Krzysztof Kolasa wrote:
> >> On 25.03.2015 01:44, David Sterba wrote:
> >>> On Tue, Mar 24, 2015 at 12:27:25PM +0100, Krzysztof Kolasa wrote:
> >>>> lz4: fix system halted at boot kernel x86_64 compressed lz4
> >>>>
> >>>> Decompression process ends with an error when loading kernel:
> >>>>
> >>>> Decoding failed
> >>>> -- System halted
> >>> Serious regression detected ...
> >>>
> >>>> This condition is probably not needed ( from the last commit d5e7caf) :
> >>> The offending patch is on the way to stable trees, so it would be best
> >>> to postpone it for now.
> >>>
> >>>> if( ... ||
> >>>> (op + COPYLENGTH) > oend)
> >>>> goto _output_error
> >>>>
> >>>> macro LZ4_SECURE_COPY() tests op and does not copy any data
> >>>> when op exceeds the value, decompression process is continued.
> >>>>
> >>>> added by analogy security for the ref:
> >>>>
> >>>> if ((ref + COPYLENGTH) > oend...
> >>>>
> >>>> to lz4_uncompress_unknownoutputsize(...)
> >>> I did only a quick check, your analysis seems correct. Reviewing the lz4
> >>> patches is tedious as the kernel implementations do not match the
> >>> upstream one line-by-line besides that I've missed the side effects of
> >>> the macro.
> >>>
> >> Add patch source for review (send to LKML) :
> >> ---------------------
> >>
> >> lz4: fix system halted at boot kernel x86_64 compressed lz4
> >>
> >> Decompression process ends with an error when loading kernel:
> >>
> >> Decoding failed
> >> -- System halted
> >>
> >> This condition is probably not needed ( from the last commit d5e7caf) :
> >>
> >> if( ... ||
> >> (op + COPYLENGTH) > oend)
> >> goto _output_error
> >>
> >> macro LZ4_SECURE_COPY() tests op and does not copy any data
> >> when op exceeds the value, decompression process is continued.
> >>
> >> added by analogy security for the ref:
> >>
> >> if ((ref + COPYLENGTH) > oend...
> >>
> >> to lz4_uncompress_unknownoutputsize(...)
> >>
> >> Signed-off-by: Krzysztof Kolasa <[email protected]>
> >> ---
> >> lib/lz4/lz4_decompress.c | 5 +++--
> >> 1 file changed, 3 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/lib/lz4/lz4_decompress.c b/lib/lz4/lz4_decompress.c
> >> index f0f5c5c..e248c4e 100644
> >> --- a/lib/lz4/lz4_decompress.c
> >> +++ b/lib/lz4/lz4_decompress.c
> >> @@ -139,8 +139,7 @@ static int lz4_uncompress(const char *source, char *dest, int osize)
> >> /* Error: request to write beyond destination buffer */
> >> if (cpy > oend)
> >> goto _output_error;
> >> - if ((ref + COPYLENGTH) > oend ||
> >> - (op + COPYLENGTH) > oend)
> >> + if ((ref + COPYLENGTH) > oend)
> >> goto _output_error;
> >> LZ4_SECURECOPY(ref, op, (oend - COPYLENGTH));
> >> while (op < cpy)
> >> @@ -270,6 +269,8 @@ static int lz4_uncompress_unknownoutputsize(const char *source, char *dest,
> >> if (cpy > oend - COPYLENGTH) {
> >> if (cpy > oend)
> >> goto _output_error; /* write outside of buf */
> >> + if ((ref + COPYLENGTH) > oend)
> >> + goto _output_error;
> >>
> >> LZ4_SECURECOPY(ref, op, (oend - COPYLENGTH));
> >> while (op < cpy)
> >> -- 2.3.3.dirty
> > I'm confused, what is the problem here? What went wrong with the
> x86_64 lz4 kernel halted system...
> > original patch that was posted, which is a mirror of what the lz4 code
> > looks like "upstream"?
> >
> > Why make this change? Does it need to go into 4.0-final, or should I
> > just revert the original patch?
> >
> > confused,
> >
> > greg k-h
> >
> OK, after further tests have modified the previous patch, please check and analyze:

What "previous patch"? None of my questions were answered here, so I
have no idea what is going on at all.

I'm going to have to just revert the original patch as obviously
something is wrong, but no one will tell me what, so I'll just go back
to the old behavior...

thanks,

greg k-h

2015-04-03 13:58:16

by Krzysztof Kolasa

[permalink] [raw]
Subject: Re: lz4: fix system halted at boot kernel x86_64 compressed lz4

On 03.04.2015 15:17, Greg KH wrote:
> What "previous patch"? None of my questions were answered here, so I have no idea what is going on at all. I'm going to have to just revert the original patch as obviously something is wrong, but no one will tell me what, so I'll just go back to the old behavior... thanks, greg k-h
again from the beginning:

commit:

https://github.com/torvalds/linux/commit/d5e7cafd69da24e6d6cc988fab6ea313a2577efc

halted my system on boot x86_64 kernel lz4 ( decompress error )

revert this commit is not required ( This patch improves security ), only slight changes I have proposed
in my first patch, modified this patch today and I send again to LKML and CC to overview.

This is a simple change to the analysis

I do not believe that only I have a problem with the 64-bit kernel compressed in lz4.

Krzysztof

2015-04-03 14:17:43

by Alexander Kuleshov

[permalink] [raw]
Subject: Re: lz4: fix system halted at boot kernel x86_64 compressed lz4

On 3 April 2015 at 19:58, Krzysztof Kolasa <[email protected]> wrote:
>
> I do not believe that only I have a problem with the 64-bit kernel compressed in lz4.

Hello all,

I can confirm that the same problem occurs for me. Tested current
mainline kernel with the Krzysztof's patch on qemu and real hardware,
it solves the problem.

2015-04-03 14:23:50

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: lz4: fix system halted at boot kernel x86_64 compressed lz4

On Fri, Apr 03, 2015 at 08:17:40PM +0600, Alexander Kuleshov wrote:
> On 3 April 2015 at 19:58, Krzysztof Kolasa <[email protected]> wrote:
> >
> > I do not believe that only I have a problem with the 64-bit kernel compressed in lz4.
>
> Hello all,
>
> I can confirm that the same problem occurs for me. Tested current
> mainline kernel with the Krzysztof's patch on qemu and real hardware,
> it solves the problem.

Ok, can someone send me the updated patch in a format that I can apply
it in? Please add your tested-by line to the patch as well.

thanks,

greg k-h

2015-04-03 14:30:47

by Krzysztof Kolasa

[permalink] [raw]
Subject: Re: lz4: fix system halted at boot kernel x86_64 compressed lz4

On 03.04.2015 16:23, Greg KH wrote:
> On Fri, Apr 03, 2015 at 08:17:40PM +0600, Alexander Kuleshov wrote:
>> On 3 April 2015 at 19:58, Krzysztof Kolasa <[email protected]> wrote:
>>> I do not believe that only I have a problem with the 64-bit kernel compressed in lz4.
>> Hello all,
>>
>> I can confirm that the same problem occurs for me. Tested current
>> mainline kernel with the Krzysztof's patch on qemu and real hardware,
>> it solves the problem.
> Ok, can someone send me the updated patch in a format that I can apply
> it in? Please add your tested-by line to the patch as well.
>
> thanks,
>
> greg k-h
>


[PATCHv2] lz4: fix system halted at boot kernel x86_64 compressed lz4

Decompression process ends with an error when loading 64bit lz4 kernel:

Decoding failed

-- System halted

This condition is not needed for 64bit kernel ( from the last commit d5e7caf )

if( ... ||
(op + COPYLENGTH) > oend)
goto _output_error

macro LZ4_SECURE_COPY() tests op and does not copy any data
when op exceeds the value, decompression process is continued.

added by analogy to lz4_uncompress_unknownoutputsize(...)

Signed-off-by: Krzysztof Kolasa <[email protected]>
---
lib/lz4/lz4_decompress.c | 12 +++++++++++-
1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/lib/lz4/lz4_decompress.c b/lib/lz4/lz4_decompress.c
index f0f5c5c..8a742b1 100644
--- a/lib/lz4/lz4_decompress.c
+++ b/lib/lz4/lz4_decompress.c
@@ -139,8 +139,12 @@ static int lz4_uncompress(const char *source, char *dest, int osize)
/* Error: request to write beyond destination buffer */
if (cpy > oend)
goto _output_error;
+#if LZ4_ARCH64
+ if ((ref + COPYLENGTH) > oend)
+#else
if ((ref + COPYLENGTH) > oend ||
(op + COPYLENGTH) > oend)
+#endif
goto _output_error;
LZ4_SECURECOPY(ref, op, (oend - COPYLENGTH));
while (op < cpy)
@@ -270,7 +274,13 @@ static int lz4_uncompress_unknownoutputsize(const char *source, char *dest,
if (cpy > oend - COPYLENGTH) {
if (cpy > oend)
goto _output_error; /* write outside of buf */
-
+#if LZ4_ARCH64
+ if ((ref + COPYLENGTH) > oend)
+#else
+ if ((ref + COPYLENGTH) > oend ||
+ (op + COPYLENGTH) > oend)
+#endif
+ goto _output_error;
LZ4_SECURECOPY(ref, op, (oend - COPYLENGTH));
while (op < cpy)
*op++ = *ref++;
-- 2.4.0.rc0.dirty

2015-04-03 14:44:34

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: lz4: fix system halted at boot kernel x86_64 compressed lz4

On Fri, Apr 03, 2015 at 04:30:40PM +0200, Krzysztof Kolasa wrote:
> On 03.04.2015 16:23, Greg KH wrote:
> > On Fri, Apr 03, 2015 at 08:17:40PM +0600, Alexander Kuleshov wrote:
> >> On 3 April 2015 at 19:58, Krzysztof Kolasa <[email protected]> wrote:
> >>> I do not believe that only I have a problem with the 64-bit kernel compressed in lz4.
> >> Hello all,
> >>
> >> I can confirm that the same problem occurs for me. Tested current
> >> mainline kernel with the Krzysztof's patch on qemu and real hardware,
> >> it solves the problem.
> > Ok, can someone send me the updated patch in a format that I can apply
> > it in? Please add your tested-by line to the patch as well.
> >
> > thanks,
> >
> > greg k-h
> >
>
>
> [PATCHv2] lz4: fix system halted at boot kernel x86_64 compressed lz4
>
> Decompression process ends with an error when loading 64bit lz4 kernel:

<snip>

I have to edit this by hand to remove the stuff above, please resend so
that I can just pass the email to 'git am' directly. I deal with
hundreds of patches a week, and can not waste time hand-editing them
all...

thanks,

greg k-h

2015-04-03 15:12:53

by Krzysztof Kolasa

[permalink] [raw]
Subject: [PATCHv2] lz4: fix system halted at boot kernel x86_64 compressed lz4

Decompression process ends with an error when loading 64bit lz4 kernel:

Decoding failed
-- System halted

This condition is not needed for 64bit kernel( from the last commit d5e7caf )

if( ... ||
(op + COPYLENGTH) > oend)
goto _output_error

macro LZ4_SECURE_COPY() tests op and does not copy any data
when op exceeds the value, decompression process is continued.

added by analogy to lz4_uncompress_unknownoutputsize(...)

Signed-off-by: Krzysztof Kolasa <[email protected]>
---
lib/lz4/lz4_decompress.c | 12 +++++++++++-
1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/lib/lz4/lz4_decompress.c b/lib/lz4/lz4_decompress.c
index f0f5c5c..8a742b1 100644
--- a/lib/lz4/lz4_decompress.c
+++ b/lib/lz4/lz4_decompress.c
@@ -139,8 +139,12 @@ static int lz4_uncompress(const char *source, char *dest, int osize)
/* Error: request to write beyond destination buffer */
if (cpy > oend)
goto _output_error;
+#if LZ4_ARCH64
+ if ((ref + COPYLENGTH) > oend)
+#else
if ((ref + COPYLENGTH) > oend ||
(op + COPYLENGTH) > oend)
+#endif
goto _output_error;
LZ4_SECURECOPY(ref, op, (oend - COPYLENGTH));
while (op < cpy)
@@ -270,7 +274,13 @@ static int lz4_uncompress_unknownoutputsize(const char *source, char *dest,
if (cpy > oend - COPYLENGTH) {
if (cpy > oend)
goto _output_error; /* write outside of buf */
-
+#if LZ4_ARCH64
+ if ((ref + COPYLENGTH) > oend)
+#else
+ if ((ref + COPYLENGTH) > oend ||
+ (op + COPYLENGTH) > oend)
+#endif
+ goto _output_error;
LZ4_SECURECOPY(ref, op, (oend - COPYLENGTH));
while (op < cpy)
*op++ = *ref++;
--
2.4.0.rc0.dirty

2015-04-03 17:36:19

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCHv2] lz4: fix system halted at boot kernel x86_64 compressed lz4

On Fri, Apr 03, 2015 at 05:12:47PM +0200, Krzysztof Kolasa wrote:
> Decompression process ends with an error when loading 64bit lz4 kernel:
>
> Decoding failed
> -- System halted
>
> This condition is not needed for 64bit kernel( from the last commit d5e7caf )
>
> if( ... ||
> (op + COPYLENGTH) > oend)
> goto _output_error
>
> macro LZ4_SECURE_COPY() tests op and does not copy any data
> when op exceeds the value, decompression process is continued.
>
> added by analogy to lz4_uncompress_unknownoutputsize(...)
>
> Signed-off-by: Krzysztof Kolasa <[email protected]>
> Tested-by: Alexander Kuleshov <[email protected]>
> ---
> lib/lz4/lz4_decompress.c | 12 +++++++++++-
> 1 file changed, 11 insertions(+), 1 deletion(-)
>
> diff --git a/lib/lz4/lz4_decompress.c b/lib/lz4/lz4_decompress.c
> index f0f5c5c..8a742b1 100644
> --- a/lib/lz4/lz4_decompress.c
> +++ b/lib/lz4/lz4_decompress.c
> @@ -139,8 +139,12 @@ static int lz4_uncompress(const char *source, char *dest, int osize)
> /* Error: request to write beyond destination buffer */
> if (cpy > oend)
> goto _output_error;
> +#if LZ4_ARCH64
> + if ((ref + COPYLENGTH) > oend)
> +#else
> if ((ref + COPYLENGTH) > oend ||
> (op + COPYLENGTH) > oend)
> +#endif
> goto _output_error;
> LZ4_SECURECOPY(ref, op, (oend - COPYLENGTH));
> while (op < cpy)
> @@ -270,7 +274,13 @@ static int lz4_uncompress_unknownoutputsize(const char *source, char *dest,
> if (cpy > oend - COPYLENGTH) {
> if (cpy > oend)
> goto _output_error; /* write outside of buf */
> -
> +#if LZ4_ARCH64
> + if ((ref + COPYLENGTH) > oend)
> +#else
> + if ((ref + COPYLENGTH) > oend ||
> + (op + COPYLENGTH) > oend)
> +#endif
> + goto _output_error;
> LZ4_SECURECOPY(ref, op, (oend - COPYLENGTH));
> while (op < cpy)
> *op++ = *ref++;

All whitespace is messed up, and this patch can not be applied :(

2015-04-03 18:03:40

by Krzysztof Kolasa

[permalink] [raw]
Subject: Re: [PATCHv2] lz4: fix system halted at boot kernel x86_64 compressed lz4

Decompression process ends with an error when loading 64bit lz4 kernel:

Decoding failed
-- System halted

This condition is not needed for 64bit kernel( from the last commit d5e7caf )

if( ... ||
(op + COPYLENGTH) > oend)
goto _output_error

macro LZ4_SECURE_COPY() tests op and does not copy any data
when op exceeds the value, decompression process is continued.

added by analogy to lz4_uncompress_unknownoutputsize(...)

Signed-off-by: Krzysztof Kolasa <[email protected]>
---
lib/lz4/lz4_decompress.c | 12 +++++++++++-
1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/lib/lz4/lz4_decompress.c b/lib/lz4/lz4_decompress.c
index f0f5c5c..8a742b1 100644
--- a/lib/lz4/lz4_decompress.c
+++ b/lib/lz4/lz4_decompress.c
@@ -139,8 +139,12 @@ static int lz4_uncompress(const char *source, char *dest, int osize)
/* Error: request to write beyond destination buffer */
if (cpy > oend)
goto _output_error;
+#if LZ4_ARCH64
+ if ((ref + COPYLENGTH) > oend)
+#else
if ((ref + COPYLENGTH) > oend ||
(op + COPYLENGTH) > oend)
+#endif
goto _output_error;
LZ4_SECURECOPY(ref, op, (oend - COPYLENGTH));
while (op < cpy)
@@ -270,7 +274,13 @@ static int lz4_uncompress_unknownoutputsize(const char *source, char *dest,
if (cpy > oend - COPYLENGTH) {
if (cpy > oend)
goto _output_error; /* write outside of buf */
-
+#if LZ4_ARCH64
+ if ((ref + COPYLENGTH) > oend)
+#else
+ if ((ref + COPYLENGTH) > oend ||
+ (op + COPYLENGTH) > oend)
+#endif
+ goto _output_error;
LZ4_SECURECOPY(ref, op, (oend - COPYLENGTH));
while (op < cpy)
*op++ = *ref++;
--
2.4.0.rc0.dirty

2015-04-03 18:06:18

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCHv2] lz4: fix system halted at boot kernel x86_64 compressed lz4

On Fri, Apr 03, 2015 at 08:03:33PM +0200, Krzysztof Kolasa wrote:
> Decompression process ends with an error when loading 64bit lz4 kernel:
>
> Decoding failed
> -- System halted
>
> This condition is not needed for 64bit kernel( from the last commit d5e7caf )
>
> if( ... ||
> (op + COPYLENGTH) > oend)
> goto _output_error
>
> macro LZ4_SECURE_COPY() tests op and does not copy any data
> when op exceeds the value, decompression process is continued.
>
> added by analogy to lz4_uncompress_unknownoutputsize(...)
>
> Signed-off-by: Krzysztof Kolasa <[email protected]>
> ---
> lib/lz4/lz4_decompress.c | 12 +++++++++++-
> 1 file changed, 11 insertions(+), 1 deletion(-)
>
> diff --git a/lib/lz4/lz4_decompress.c b/lib/lz4/lz4_decompress.c
> index f0f5c5c..8a742b1 100644
> --- a/lib/lz4/lz4_decompress.c
> +++ b/lib/lz4/lz4_decompress.c
> @@ -139,8 +139,12 @@ static int lz4_uncompress(const char *source, char *dest, int osize)
> /* Error: request to write beyond destination buffer */
> if (cpy > oend)
> goto _output_error;
> +#if LZ4_ARCH64
> + if ((ref + COPYLENGTH) > oend)
> +#else
> if ((ref + COPYLENGTH) > oend ||
> (op + COPYLENGTH) > oend)
> +#endif
> goto _output_error;
> LZ4_SECURECOPY(ref, op, (oend - COPYLENGTH));
> while (op < cpy)
> @@ -270,7 +274,13 @@ static int lz4_uncompress_unknownoutputsize(const char *source, char *dest,
> if (cpy > oend - COPYLENGTH) {
> if (cpy > oend)
> goto _output_error; /* write outside of buf */
> -
> +#if LZ4_ARCH64
> + if ((ref + COPYLENGTH) > oend)
> +#else
> + if ((ref + COPYLENGTH) > oend ||
> + (op + COPYLENGTH) > oend)
> +#endif
> + goto _output_error;
> LZ4_SECURECOPY(ref, op, (oend - COPYLENGTH));
> while (op < cpy)
> *op++ = *ref++;
> --
> 2.4.0.rc0.dirty

Still all tabs turned to spaces :(

2015-04-03 18:18:10

by Alexander Kuleshov

[permalink] [raw]
Subject: Re: [PATCHv2] lz4: fix system halted at boot kernel x86_64 compressed lz4

On 3 April 2015 at 23:36, Greg KH <[email protected]> wrote:
> On Fri, Apr 03, 2015 at 05:12:47PM +0200, Krzysztof Kolasa wrote:
>> added by analogy to lz4_uncompress_unknownoutputsize(...)
>>
>> Signed-off-by: Krzysztof Kolasa <[email protected]>
>> Tested-by: Alexander Kuleshov <[email protected]>

I'm not sure that my testing was important in this case, but if you'd
to add my name, please use another email as:

Tested-by: Alexander Kuleshov <[email protected]>

2015-04-03 19:01:14

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCHv2] lz4: fix system halted at boot kernel x86_64 compressed lz4

On Sat, Apr 04, 2015 at 12:18:07AM +0600, Alexander Kuleshov wrote:
> On 3 April 2015 at 23:36, Greg KH <[email protected]> wrote:
> > On Fri, Apr 03, 2015 at 05:12:47PM +0200, Krzysztof Kolasa wrote:
> >> added by analogy to lz4_uncompress_unknownoutputsize(...)
> >>
> >> Signed-off-by: Krzysztof Kolasa <[email protected]>
> >> Tested-by: Alexander Kuleshov <[email protected]>
>
> I'm not sure that my testing was important in this case, but if you'd
> to add my name, please use another email as:
>
> Tested-by: Alexander Kuleshov <[email protected]>

Thanks, will do so. If I ever get a patch that can be applied :)

2015-04-06 10:12:53

by Krzysztof Kolasa

[permalink] [raw]
Subject: Re: [PATCHv2] lz4: fix system halted at boot kernel x86_64 compressed lz4

Decompression process ends with an error when loading 64bit lz4 kernel:

Decoding failed
-- System halted

This condition is not needed for 64bit kernel from the last
commit d5e7cafd69da ("LZ4 : fix the data abort issue")

if( ... ||
(op + COPYLENGTH) > oend)
goto _output_error

macro LZ4_SECURE_COPY() tests op and does not copy any data
when op exceeds the value, decompression process is continued.

added by analogy to lz4_uncompress_unknownoutputsize(...)

Signed-off-by: Krzysztof Kolasa <[email protected]>
---
lib/lz4/lz4_decompress.c | 12 +++++++++++-
1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/lib/lz4/lz4_decompress.c b/lib/lz4/lz4_decompress.c
index f0f5c5c..8a742b1 100644
--- a/lib/lz4/lz4_decompress.c
+++ b/lib/lz4/lz4_decompress.c
@@ -139,8 +139,12 @@ static int lz4_uncompress(const char *source, char *dest, int osize)
/* Error: request to write beyond destination buffer */
if (cpy > oend)
goto _output_error;
+#if LZ4_ARCH64
+ if ((ref + COPYLENGTH) > oend)
+#else
if ((ref + COPYLENGTH) > oend ||
(op + COPYLENGTH) > oend)
+#endif
goto _output_error;
LZ4_SECURECOPY(ref, op, (oend - COPYLENGTH));
while (op < cpy)
@@ -270,7 +274,13 @@ static int lz4_uncompress_unknownoutputsize(const char *source, char *dest,
if (cpy > oend - COPYLENGTH) {
if (cpy > oend)
goto _output_error; /* write outside of buf */
-
+#if LZ4_ARCH64
+ if ((ref + COPYLENGTH) > oend)
+#else
+ if ((ref + COPYLENGTH) > oend ||
+ (op + COPYLENGTH) > oend)
+#endif
+ goto _output_error;
LZ4_SECURECOPY(ref, op, (oend - COPYLENGTH));
while (op < cpy)
*op++ = *ref++;
--
2.4.0.rc1