From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751964AbbCYHLT (ORCPT ); Wed, 25 Mar 2015 03:11:19 -0400 Received: from vps01.winsoft.pl ([5.133.9.51]:54532 "EHLO vps01.winsoft.pl" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750873AbbCYHLS (ORCPT ); Wed, 25 Mar 2015 03:11:18 -0400 X-Greylist: delayed 374 seconds by postgrey-1.27 at vger.kernel.org; Wed, 25 Mar 2015 03:11:18 EDT Subject: Re: lz4: fix system halted at boot kernel x86_64 compressed lz4 To: dsterba@suse.cz, tom.yeon@windriver.com, gregkh@linuxfoundation.org, linux-kernel@vger.kernel.org references: <55114A1D.7030508@winsoft.pl> <20150325004458.GA20767@suse.cz> From: Krzysztof Kolasa organization: P.H.U. "WINSOFT" Krzysztof Kolasa message-id: <55125E1B.3090506@winsoft.pl> Date: Wed, 25 Mar 2015 08:04:59 +0100 user-agent: Mozilla/5.0 (X11; Linux x86_64; rv:39.0) Gecko/20100101 Thunderbird/39.0a1 mime-version: 1.0 in-reply-to: <20150325004458.GA20767@suse.cz> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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 --- 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