From mboxrd@z Thu Jan 1 00:00:00 1970 From: Joe Stringer Subject: Re: [patch -master] netfilter: xt_CT: checking for IS_ERR() instead of NULL Date: Mon, 3 Aug 2015 13:34:58 -0700 Message-ID: References: <20150727224228.GA10367@mwanda> <20150730115743.GA7077@salvia> <20150803202406.GA3339@salvia> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Cc: Dan Carpenter , Patrick McHardy , Jozsef Kadlecsik , "David S. Miller" , netfilter-devel@vger.kernel.org, coreteam@netfilter.org, Linux Netdev List , kernel-janitors@vger.kernel.org To: Pablo Neira Ayuso Return-path: In-Reply-To: <20150803202406.GA3339@salvia> Sender: netfilter-devel-owner@vger.kernel.org List-Id: netdev.vger.kernel.org On 3 August 2015 at 13:24, Pablo Neira Ayuso wrote: > On Mon, Aug 03, 2015 at 11:30:16AM -0700, Joe Stringer wrote: >> On 3 August 2015 at 11:29, Joe Stringer wrote: >> > On 30 July 2015 at 04:57, Pablo Neira Ayuso wrote: >> >> On Tue, Jul 28, 2015 at 01:42:28AM +0300, Dan Carpenter wrote: >> >>> We recently changed this from nf_conntrack_alloc() to nf_ct_tmpl_alloc() >> >>> so the error handling needs to changed to check for NULL instead of >> >>> IS_ERR(). >> >>> >> >>> Fixes: 0838aa7fcfcd ('netfilter: fix netns dependencies with conntrack templates') >> >>> Signed-off-by: Dan Carpenter >> >> >> >> Applied, thanks. >> >> >> >> I have also appended this chunk, since synproxy is also affected: >> >> >> >> --- a/net/netfilter/nf_synproxy_core.c >> >> +++ b/net/netfilter/nf_synproxy_core.c >> >> @@ -353,7 +353,7 @@ static int __net_init synproxy_net_init(struct net *net) >> >> int err = -ENOMEM; >> >> >> >> ct = nf_ct_tmpl_alloc(net, 0, GFP_KERNEL); >> >> - if (IS_ERR(ct)) { >> >> + if (!ct) { >> >> err = PTR_ERR(ct); >> >> goto err1; >> >> } >> > >> > Does PTR_ERR() implicitly interpret NULL as -ENOMEM? Seems like the >> > fix applied here is a little different from the xt_CT fix. >> >> Just saw the initialization of err now, but this would be overridden >> within the error checking statement. > > Right, I noticed before pushing out this change, the final applied > patch is here: > > http://git.kernel.org/cgit/linux/kernel/git/pablo/nf.git/commit/?id=1a727c63612fc582370cf3dc01239d3d239743b5 > > Let me know if you still have any concern, thanks Joe. Looks fine. Apologies for the noise.