Received: by 2002:a05:7412:2a8c:b0:e2:908c:2ebd with SMTP id u12csp3760684rdh; Fri, 29 Sep 2023 01:11:04 -0700 (PDT) X-Google-Smtp-Source: AGHT+IHYsoCRlBetC28DvltQjfcOluyJ+kBJYRZnK4AgOW/kh+Mjoj7nRASSztY6sDFAhx17s5w+ X-Received: by 2002:a05:6808:9a7:b0:3ab:83fe:e18f with SMTP id e7-20020a05680809a700b003ab83fee18fmr3404143oig.35.1695975064442; Fri, 29 Sep 2023 01:11:04 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1695975064; cv=none; d=google.com; s=arc-20160816; b=EKYu7zfMmv4Ls7dBn5y5efghpxNXgx6mgGyG2gtxxMplXCPzdElZ0tcb/chur/tW2J dap9I8iwwdS5ulqhelCawR/Vx6IOOuyLa+gokD2l8qJHoLkAoHZDrib+ngfnZQAdLyMu UkwLMn8+2Dq5U/Pai9dyiT6l58X/4x/adjqw50ox6XDljpVDuEOY27nZdWVBOpVlQbF0 2GaeOuUfckOcAki5tZPmyX7YMeSDzPPV1RrADEbknSg/udMwjszpcV5J/8XVIpxshhfk XNsvI5cB1XlxISmOezkZKOF4RAJewBPZ0Z7a0h0fLjMVDam7/wQ8eqxIi+Q8Ynl0Ym4k 8J0w== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:in-reply-to:content-disposition:mime-version :references:message-id:subject:cc:to:from:date; bh=7HWFfhWLhYZo3e8pkblffQWcL5vWSI7hbBfWU0LAI2w=; fh=lOnKzTvIuQ1MRgQrzS1tuzHcEbejIIjsMe7wr3UD3+A=; b=JrldNOx6nBH/EOCJ1ays7tlJUn1p9T4z8jEx5k5+7SrZIYHh5ebNqUhDebnmGpDdkz 8K+Zh5zrJGz/+PnYpGRJ9ywmGUYLx00yd7Eg6KfYsFGfe3Bz7Xq1qyvJ8b+5yRs9ReJG ODdmEzMDIlv/InkHijZ9M7EMwrUcd1CoJjk2/qGtj/Zs6alJjXBoW8HU+2ZD1dthjsto 2/h3MeH7PqBAZjJuQESERUrQLQDLJmJio3vR8+2CqGxRsaCvr5t7qdRndYYA0gHk8h2Y uxYlRn0qVSt/yZ6pfISTm/Cuhg5g4S6b59V5z9vjmLEH4FJRery0HT4/g9goWAHfJTai 8Nfg== ARC-Authentication-Results: i=1; mx.google.com; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.35 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Return-Path: Received: from groat.vger.email (groat.vger.email. [23.128.96.35]) by mx.google.com with ESMTPS id v135-20020a63618d000000b0056401aef836si20306681pgb.822.2023.09.29.01.11.04 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 29 Sep 2023 01:11:04 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.35 as permitted sender) client-ip=23.128.96.35; Authentication-Results: mx.google.com; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.35 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: from out1.vger.email (depot.vger.email [IPv6:2620:137:e000::3:0]) by groat.vger.email (Postfix) with ESMTP id AF62A80267E8; Fri, 29 Sep 2023 01:06:41 -0700 (PDT) X-Virus-Status: Clean X-Virus-Scanned: clamav-milter 0.103.10 at groat.vger.email Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232806AbjI2IGL (ORCPT + 99 others); Fri, 29 Sep 2023 04:06:11 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:50350 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S232674AbjI2IGK (ORCPT ); Fri, 29 Sep 2023 04:06:10 -0400 Received: from ganesha.gnumonks.org (ganesha.gnumonks.org [IPv6:2001:780:45:1d:225:90ff:fe52:c662]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id AE5051A5; Fri, 29 Sep 2023 01:06:07 -0700 (PDT) Received: from [78.30.34.192] (port=36492 helo=gnumonks.org) by ganesha.gnumonks.org with esmtpsa (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.94.2) (envelope-from ) id 1qm8Vc-007w9u-C9; Fri, 29 Sep 2023 10:06:02 +0200 Date: Fri, 29 Sep 2023 10:05:59 +0200 From: Pablo Neira Ayuso To: Joao Moreira Cc: netfilter-devel@vger.kernel.org, coreteam@netfilter.org, netdev@vger.kernel.org, linux-kernel@vger.kernel.org, kadlec@netfilter.org, fw@strlen.de, davem@davemloft.net, edumazet@google.com, kuba@kernel.org, pabeni@redhat.com, rkannoth@marvell.com, wojciech.drewek@intel.com, steen.hegenlund@microhip.com, keescook@chromium.org, Joao Moreira Subject: Re: [PATCH v3 1/2] Make loop indexes unsigned Message-ID: References: <20230927164715.76744-1-joao@overdrivepizza.com> <20230927164715.76744-2-joao@overdrivepizza.com> <77df92a5627411471f1f374d41ae500c@overdrivepizza.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <77df92a5627411471f1f374d41ae500c@overdrivepizza.com> X-Spam-Score: -1.9 (-) X-Spam-Status: No, score=-0.8 required=5.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS autolearn=unavailable autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on groat.vger.email Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org X-Greylist: Sender passed SPF test, not delayed by milter-greylist-4.6.4 (groat.vger.email [0.0.0.0]); Fri, 29 Sep 2023 01:06:41 -0700 (PDT) On Thu, Sep 28, 2023 at 07:53:14PM -0700, Joao Moreira wrote: > On 2023-09-28 06:40, Pablo Neira Ayuso wrote: > > On Wed, Sep 27, 2023 at 09:47:14AM -0700, joao@overdrivepizza.com wrote: > > > From: Joao Moreira > > > > > > Both flow_rule_alloc and offload_action_alloc functions received an > > > unsigned num_actions parameters which are then operated within a loop. > > > The index of this loop is declared as a signed int. If it was possible > > > to pass a large enough num_actions to these functions, it would lead > > > to > > > an out of bounds write. > > > > > > After checking with maintainers, it was mentioned that front-end will > > > cap the num_actions value and that it is not possible to reach this > > > function with such a large number. Yet, for correctness, it is still > > > better to fix this. > > > > > > This issue was observed by the commit author while reviewing a > > > write-up > > > regarding a CVE within the same subsystem [1]. > > > > > > 1 - https://nickgregory.me/post/2022/03/12/cve-2022-25636/ > > > > > > Signed-off-by: Joao Moreira > > > --- > > > net/core/flow_offload.c | 4 ++-- > > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > > > diff --git a/net/core/flow_offload.c b/net/core/flow_offload.c > > > index bc5169482710..bc3f53a09d8f 100644 > > > --- a/net/core/flow_offload.c > > > +++ b/net/core/flow_offload.c > > > @@ -10,7 +10,7 @@ > > > struct flow_rule *flow_rule_alloc(unsigned int num_actions) > > > { > > > struct flow_rule *rule; > > > - int i; > > > + unsigned int i; > > > > With the 2^8 cap, I don't think this patch is required anymore. > > Hm. While I understand that there is not a significant menace haunting > this... would it be good for (1) type correctness and (2) prevent that > things blow up if something changes and someone misses this spot? Nothing is going to change, please remove unnecesary updates. Capping to 2^8 for all hardware offload subsystems is sufficient by now. If someone needs more than that, it will have to justify it.