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 Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 0241FC433FE for ; Fri, 18 Nov 2022 10:18:22 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S241472AbiKRKSU (ORCPT ); Fri, 18 Nov 2022 05:18:20 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:46622 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S239887AbiKRKST (ORCPT ); Fri, 18 Nov 2022 05:18:19 -0500 Received: from szxga08-in.huawei.com (szxga08-in.huawei.com [45.249.212.255]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 6B15511174 for ; Fri, 18 Nov 2022 02:18:18 -0800 (PST) Received: from dggpemm500024.china.huawei.com (unknown [172.30.72.54]) by szxga08-in.huawei.com (SkyGuard) with ESMTP id 4NDCQm4tfDz15Mj6; Fri, 18 Nov 2022 18:17:52 +0800 (CST) Received: from [10.67.110.173] (10.67.110.173) by dggpemm500024.china.huawei.com (7.185.36.203) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2375.31; Fri, 18 Nov 2022 18:18:16 +0800 Message-ID: Date: Fri, 18 Nov 2022 18:18:16 +0800 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101 Thunderbird/102.3.2 Subject: Re: [PATCH 0/3 v2] 9p: Fix write overflow in p9_read_work Content-Language: en-US To: , Christian Schoenebeck CC: , , , , , , , References: <20221117091159.31533-1-guozihua@huawei.com> <3918617.6eBe0Ihrjo@silver> From: "Guozihua (Scott)" In-Reply-To: Content-Type: text/plain; charset="UTF-8"; format=flowed Content-Transfer-Encoding: 8bit X-Originating-IP: [10.67.110.173] X-ClientProxiedBy: dggems705-chm.china.huawei.com (10.3.19.182) To dggpemm500024.china.huawei.com (7.185.36.203) X-CFilter-Loop: Reflected Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org On 2022/11/18 12:59, asmadeus@codewreck.org wrote: > Christian Schoenebeck wrote on Thu, Nov 17, 2022 at 02:33:28PM +0100: >>> GUO Zihua (3): >>> 9p: Fix write overflow in p9_read_work >>> 9p: Remove redundent checks for message size against msize. >>> 9p: Use P9_HDRSZ for header size >> >> For entire series: >> >> Reviewed-by: Christian Schoenebeck >> >> I agree with Dominique that patch 1 and 2 should be merged. > > Thank you both! > > I've just pushed the patches to my next branch: > https://github.com/martinetd/linux/commits/9p-next > > ... with a twist, as the original patch fails on any normal workload: > --- > / # ls /mnt > 9pnet: -- p9_read_work (19): requested packet size too big: 9 for tag 0 with capacity 11 > --- > (so much for having two pairs of eyes :-D > By the way we -really- need to replace P9_DEBUG_ERROR by pr_error or > something, these should be displayed without having to specify > debug=1...) > > > capacity is only set in a handful of places (alloc time, hardcoded 7 in > trans_fd, after receiving packet) so I've added logs and our alloc > really passed '11' for alloc_rsize (logging tsize/rsize) > > 9pnet: (00000087) >>> TWALK fids 1,2 nwname 0d wname[0] (null) > 9pnet: -- p9_tag_alloc (87): allocating capacity to 17/11 for tag 0 > 9pnet: -- p9_read_work (19): requested packet size too big: 9 for tag 0 with capacity 11 > > ... So this is RWALK, right: > size[4] Rwalk tag[2] nwqid[2] nwqid*(wqid[13]) > 4 ..... 5.... 7..... 9....... packet end at 9 as 0 nwqid. > We have capacity 11 to allow rlerror_size which is bigger; everything is > good. > > Long story short, the size header includes the header size, when I > misread https://9fans.github.io/plan9port/man/man9/version.html to > say it didn't (it just says it doesn't include the enveloping transport > protocol, it starts from size alright and I just misread that) > Thanksfully the code caught it. > > So I've just removed the - offset part and things appear to work again. > > Guo Zihua, can you check this still fixes your syz repro, or was that > substraction needed? If it's still needed we have an off by 1 somewhere > to look for. > Hi Dominique, I retried the repro on your branch, the issue does not reproduce. What a good pair of eyes :)！ -- Best GUO Zihua