From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932523AbeCKWpv (ORCPT ); Sun, 11 Mar 2018 18:45:51 -0400 Received: from gateway22.websitewelcome.com ([192.185.47.79]:49921 "EHLO gateway22.websitewelcome.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932361AbeCKWpt (ORCPT ); Sun, 11 Mar 2018 18:45:49 -0400 Subject: Re: [RFC] netfilter: cttimeout: remove VLA in ctnl_timeout_parse_policy To: Pablo Neira Ayuso Cc: Jozsef Kadlecsik , Florian Westphal , "David S. Miller" , netfilter-devel@vger.kernel.org, coreteam@netfilter.org, netdev@vger.kernel.org, linux-kernel@vger.kernel.org, "Gustavo A. R. Silva" References: <20180306184755.GA7628@embeddedgus> <20180311220414.feda33aw72zw2rko@salvia> <20df6b07-b3e7-7733-ee05-b12589bd287f@embeddedor.com> <20180311222110.yk34o63ha5y34ssx@salvia> From: "Gustavo A. R. Silva" Message-ID: Date: Sun, 11 Mar 2018 17:45:46 -0500 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.6.0 MIME-Version: 1.0 In-Reply-To: <20180311222110.yk34o63ha5y34ssx@salvia> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit X-AntiAbuse: This header was added to track abuse, please include it with any abuse report X-AntiAbuse: Primary Hostname - gator4166.hostgator.com X-AntiAbuse: Original Domain - vger.kernel.org X-AntiAbuse: Originator/Caller UID/GID - [47 12] / [47 12] X-AntiAbuse: Sender Address Domain - embeddedor.com X-BWhitelist: no X-Source-IP: 189.175.117.58 X-Source-L: No X-Exim-ID: 1ev9it-0047Ks-Mv X-Source: X-Source-Args: X-Source-Dir: X-Source-Sender: ([192.168.1.66]) [189.175.117.58]:54000 X-Source-Auth: gustavo@embeddedor.com X-Email-Count: 16 X-Source-Cap: Z3V6aWRpbmU7Z3V6aWRpbmU7Z2F0b3I0MTY2Lmhvc3RnYXRvci5jb20= X-Local-Domain: yes Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 03/11/2018 05:21 PM, Pablo Neira Ayuso wrote: > On Sun, Mar 11, 2018 at 05:12:09PM -0500, Gustavo A. R. Silva wrote: >> Hi Pablo, >> >> On 03/11/2018 05:04 PM, Pablo Neira Ayuso wrote: >>> On Tue, Mar 06, 2018 at 12:47:55PM -0600, Gustavo A. R. Silva wrote: >>>> In preparation to enabling -Wvla, remove VLA and replace it >>>> with dynamic memory allocation. >>> >>> Looks good but... >>> >>>> Signed-off-by: Gustavo A. R. Silva >>>> --- >>>> net/netfilter/nfnetlink_cttimeout.c | 12 ++++++++++-- >>>> 1 file changed, 10 insertions(+), 2 deletions(-) >>>> >>>> diff --git a/net/netfilter/nfnetlink_cttimeout.c b/net/netfilter/nfnetlink_cttimeout.c >>>> index 95b0470..a2f7d92 100644 >>>> --- a/net/netfilter/nfnetlink_cttimeout.c >>>> +++ b/net/netfilter/nfnetlink_cttimeout.c >>>> @@ -52,18 +52,26 @@ ctnl_timeout_parse_policy(void *timeouts, >>>> struct net *net, const struct nlattr *attr) >>>> { >>>> int ret = 0; >>>> + struct nlattr **tb = NULL; >>> >>> I think we don't need to initialize this, right? >>> >> >> We actually do have to initialized it because in the unlikely case that the >> code block inside the 'if' below is not executed, then we will end up >> freeing an uninitialized pointer. > > I see, you're right indeed. > > We can probably simplify this code, but just doing: > > if (!l4proto->ctnl_timeout.nlattr_to_obj)) > return 0; > I wonder if it is better to code this instead: if (unlikely(!l4proto->ctnl_timeout.nlattr_to_obj))) return 0; > netlink attribute parsing here. > > You could even remove the likely() thing, which doesn't make much > sense for control plane code. > Why is that? > I understand this is a larger change, but I think this function will > look better while we're removing VLA. > > Would you mind having a look? I'd appreciate if so. > I can do that. No problem. Thanks -- Gustavo