Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Date:   Wed, 28 Nov 2018 14:52:42 +0100
From:   David Sterba <dsterba@suse.cz>
To:     Yueyi Li <liyueyi@live.com>
Cc:     "gregkh@linuxfoundation.org" <gregkh@linuxfoundation.org>,
        "w@1wt.eu" <w@1wt.eu>,
        "donb@securitymouse.com" <donb@securitymouse.com>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        markus@oberhumer.com
Subject: Re: [PATCH v2] lzo: fix ip overrun during compress.
Message-ID: <20181128135242.gy3avmbp2pdmjaka@twin.jikos.cz>
Reply-To: dsterba@suse.cz
Mail-Followup-To: dsterba@suse.cz, Yueyi Li <liyueyi@live.com>,
        "gregkh@linuxfoundation.org" <gregkh@linuxfoundation.org>,
        "w@1wt.eu" <w@1wt.eu>,
        "donb@securitymouse.com" <donb@securitymouse.com>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        markus@oberhumer.com
References: <BLUPR13MB0289EFEB79833470EE62F48EDFD10@BLUPR13MB0289.namprd13.prod.outlook.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <BLUPR13MB0289EFEB79833470EE62F48EDFD10@BLUPR13MB0289.namprd13.prod.outlook.com>
User-Agent: NeoMutt/20180716
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

Adding Markus to to CC

On Wed, Nov 28, 2018 at 07:31:26AM +0000, Yueyi Li wrote:
> It`s possible ip overrun in lzo1x_1_do_compress() when compressed page is
> point to the end of memory and which virtual address is 0xfffffffffffff000.
> Leading to a NULL pointer access during the get_unaligned_le32(ip).

So this could happen in practice in zram, but unlikely for other users
of lzo (like btrfs). I'm not sure but expect that the last page would
not be returned by allocator.

The fix is adding a few branches to code that's supposed to be as fast
as possible. The branches would be evaluated all the time while
protecting against one signle bad page address. This does not look like
a good performance tradeoff.

> +#define OVERFLOW_ADD_CHECK(a, b)  \
> +		(((a) + (b)) < (a))

I'm not sure if this is generally safe overflow check (never not
optimized out). Here it depends on the types of 'a' and 'b' that are
pointer (ip) and size_t (m_len). GCC has __builtin_add_overflow_p so
that one should be used where possible.