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 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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
