From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-0.8 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,SPF_PASS,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id D0175C3279B for ; Tue, 10 Jul 2018 11:06:49 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 8CA7920881 for ; Tue, 10 Jul 2018 11:06:49 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 8CA7920881 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=huawei.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S933285AbeGJLGq (ORCPT ); Tue, 10 Jul 2018 07:06:46 -0400 Received: from szxga04-in.huawei.com ([45.249.212.190]:9168 "EHLO huawei.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1751432AbeGJLGo (ORCPT ); Tue, 10 Jul 2018 07:06:44 -0400 Received: from DGGEMS405-HUB.china.huawei.com (unknown [172.30.72.58]) by Forcepoint Email with ESMTP id 42F7C6B4A7162; Tue, 10 Jul 2018 19:06:29 +0800 (CST) Received: from [10.177.253.249] (10.177.253.249) by smtp.huawei.com (10.3.19.205) with Microsoft SMTP Server id 14.3.382.0; Tue, 10 Jul 2018 19:06:28 +0800 Subject: Re: [V9fs-developer] [PATCH] Integer underflow in pdu_read() To: Tomas Bortoli , , , References: <20180709192651.28095-1-tomasbortoli@gmail.com> <5B440B6A.9090000@huawei.com> <36523cc7-adec-9e61-d34c-dc00806c403a@gmail.com> CC: , , , , From: piaojun Message-ID: <5B449329.1050507@huawei.com> Date: Tue, 10 Jul 2018 19:06:17 +0800 User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:38.0) Gecko/20100101 Thunderbird/38.2.0 MIME-Version: 1.0 In-Reply-To: <36523cc7-adec-9e61-d34c-dc00806c403a@gmail.com> Content-Type: text/plain; charset="windows-1252" Content-Transfer-Encoding: 7bit X-Originating-IP: [10.177.253.249] X-CFilter-Loop: Reflected Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Tomas, Thanks for your explaination, and I get your point. On 2018/7/10 16:27, Tomas Bortoli wrote: > 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, right? >> 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 will have >>> a wrong result, resulting in an out-of-bound read. >>> This patch modifies also pdu_write() in the same way to prevent the same >>> 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); >>> >>> size_t pdu_read(struct p9_fcall *pdu, void *data, size_t size) >>> { >>> - size_t len = min(pdu->size - pdu->offset, size); >>> - memcpy(data, &pdu->sdata[pdu->offset], len); >>> + size_t len = pdu->offset > pdu->size ? 0 : >>> + min(pdu->size - pdu->offset, size); >>> + if (len != 0) >>> + memcpy(data, &pdu->sdata[pdu->offset], len); >>> pdu->offset += len; >>> return size - len; >>> } >>> >>> static size_t pdu_write(struct p9_fcall *pdu, const void *data, size_t size) >>> { >>> - size_t len = min(pdu->capacity - pdu->size, size); >>> - memcpy(&pdu->sdata[pdu->size], data, len); >>> + size_t len = pdu->size > pdu->capacity ? 0 : >>> + min(pdu->capacity - pdu->size, size); >>> + if (len != 0) >>> + memcpy(&pdu->sdata[pdu->size], data, len); >>> pdu->size += len; >>> return size - len; >>> } >>> > > >