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 10:27:32 +0200 Message-ID: <36523cc7-adec-9e61-d34c-dc00806c403a@gmail.com> References: <20180709192651.28095-1-tomasbortoli@gmail.com> <5B440B6A.9090000@huawei.com> Mime-Version: 1.0 Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: quoted-printable Cc: netdev@vger.kernel.org, linux-kernel@vger.kernel.org, syzkaller@googlegroups.com, v9fs-developer@lists.sourceforge.net, davem@davemloft.net To: piaojun , ericvh@gmail.com, rminnich@sandia.gov, lucho@ionkov.net Return-path: In-Reply-To: <5B440B6A.9090000@huawei.com> Content-Language: en-US Sender: linux-kernel-owner@vger.kernel.org List-Id: netdev.vger.kernel.org Hi Jun, Intuitively, if you have a packet of size x and you read at an offset y, when y>x you are off the packet. That's an out out bound read. In this specific code when offset > size, the available length estimation will fail as there will be an underflow resulting from offset-size (it'll give a big big number) that breaks the out-of-bound control put in place (if offset-size is a big big number, the asked size to read will be probably smaller and therefore allowed). These definitions might help: https://cwe.mitre.org/data/definitions/787.html https://cwe.mitre.org/data/definitions/125.html Tomas > Hi Tomas, > > It looks like pdu->size should always be greater than pdu->offset, righ= t? > My question may be very easy for you, please help explaining. > > Thanks, > Jun > > On 2018/7/10 3:26, 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. >> >> Signed-off-by: Tomas Bortoli >> Reported-by: syzbot+65c6b72f284a39d416b4@syzkaller.appspotmail.com >> --- >> net/9p/protocol.c | 12 ++++++++---- >> 1 file changed, 8 insertions(+), 4 deletions(-) >> >> diff --git a/net/9p/protocol.c b/net/9p/protocol.c >> index 931ea00c4fed..f1e2425f920b 100644 >> --- a/net/9p/protocol.c >> +++ b/net/9p/protocol.c >> @@ -55,16 +55,20 @@ EXPORT_SYMBOL(p9stat_free); >> =20 >> size_t pdu_read(struct p9_fcall *pdu, void *data, size_t size) >> { >> - size_t len =3D min(pdu->size - pdu->offset, size); >> - memcpy(data, &pdu->sdata[pdu->offset], len); >> + size_t len =3D pdu->offset > pdu->size ? 0 : >> + min(pdu->size - pdu->offset, size); >> + if (len !=3D 0) >> + memcpy(data, &pdu->sdata[pdu->offset], len); >> pdu->offset +=3D len; >> return size - len; >> } >> =20 >> static size_t pdu_write(struct p9_fcall *pdu, const void *data, size_= t size) >> { >> - size_t len =3D min(pdu->capacity - pdu->size, size); >> - memcpy(&pdu->sdata[pdu->size], data, len); >> + size_t len =3D pdu->size > pdu->capacity ? 0 : >> + min(pdu->capacity - pdu->size, size); >> + if (len !=3D 0) >> + memcpy(&pdu->sdata[pdu->size], data, len); >> pdu->size +=3D len; >> return size - len; >> } >>