From mboxrd@z Thu Jan 1 00:00:00 1970 From: Tomas Bortoli Subject: Re: [V9fs-developer] [PATCH] Integer underflow in pdu_read() Date: Tue, 10 Jul 2018 00:14:21 +0200 Message-ID: References: <20180709192651.28095-1-tomasbortoli@gmail.com> <20180709193151.GI30522@ZenIV.linux.org.uk> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Cc: ericvh@gmail.com, rminnich@sandia.gov, lucho@ionkov.net, davem@davemloft.net, v9fs-developer@lists.sourceforge.net, netdev@vger.kernel.org, linux-kernel@vger.kernel.org, syzkaller@googlegroups.com To: Al Viro Return-path: In-Reply-To: <20180709193151.GI30522@ZenIV.linux.org.uk> Content-Language: en-US Sender: linux-kernel-owner@vger.kernel.org List-Id: netdev.vger.kernel.org On 07/09/2018 09:31 PM, Al Viro wrote: > On Mon, Jul 09, 2018 at 09:26:51PM +0200, Tomas Bortoli wrote: >> The pdu_read() function suffers from an integer underflow. >> When pdu->offset is greater than pdu->size, the length calculation wil= l have >> a wrong result, resulting in an out-of-bound read. >> This patch modifies also pdu_write() in the same way to prevent the sa= me >> issue from happening there and for consistency. > What does cause the calls of pdu_read() in such conditions and shouldn'= t *that* > be dealt with? Mmh I think that's caused by p9_parse_header(). That function reads the first 7 bytes of a PDU regardless of the current offset. It then sets the PDU length to the one read and then it restores the original offset. Therefore, it's possible to set a size < offset here. (to be 100% sure I'd need more debugging) We can prevent it in p9_parse_header(), but if the invariant offset < size gets broken elsewhere we fall back to the underflow. Enforcing it in pdu_read() should be the safest thing to do as it would detect *any* bad read.