Re: [RFC] LZO1X de/compression support

[Richard Purdie <richard@openedhand.com>]

Hi,

On Fri, 2007-05-18 at 15:28 +0530, Nitin Gupta wrote:
> This is kernel port of LZO1X de/compression algo stripped down to just ~500 LOC!
> It is derived from original LZO 2.02 code found at:
> http://www.oberhumer.com/opensource/lzo/download/
> The code has also been reformatted to match general kernel style.
> 
> Facts for LZO (at least for original code. Should hold true for this
> port also - hence the RFC!):
> - The compressor can never overrun buffer.
> - The "non-safe" version of decompressor can never overrun buffer if
> compressed data is unmodified. I am not sure about this if compressed
> data is malicious (to be confirmed from the author).

If the data is malicious, it *can* overrun the buffer.

> - The "safe" version can never crash (buffer overrun etc.) - confirmed
> from the author.
> This patch, as of yet, only gives 'non-safe' version of decompressor.
> The 'safe' version  will be included soon.

How are you planning to add that back?

> Since 'non-safe' version has no problems if compressed data is
> unmodified, it is useful in cases we have such guarantees on
> compressed data and hence don't want additional overhead of 'safe'
> version. For e.g. Compressed Caching project
> (http://linuxcompressed.sourceforge.net) has been using the 'non-safe'
> version of LZO1X since long time without any problems w.r.t.
> de/compression itself.
> 
> For now, I have tested this on x86 only.

The LZO author had some concerns about this code. The major issue he
highlighted was that it was 64-bit unsafe. Have you addressed that
problem? Has it been tested on 64bit?

I'm worried that in converting this code the way you have, you've
possibly introduced potential security holes. You've removed all bounds
checking and are going to have to add that back to create the "safe"
version of the decompression function. Until I mentioned it, you seemed
unaware of the potential problem and the comments above suggest you
don't understand this code as fully as I'd like with regard to
overflows.

The version I submitted has at least been subject to userspace scrutiny
over a period of time and is basically unchanged with regard to
security. It is much uglier though.

Richard