[patch -master] netfilter: xt_CT: checking for IS_ERR() instead of NULL

[patch -master] netfilter: xt_CT: checking for IS_ERR() instead of NULL

[Dan Carpenter]

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 <dan.carpenter@oracle.com>

diff --git a/net/netfilter/xt_CT.c b/net/netfilter/xt_CT.c
index c663003..43ddeee 100644
--- a/net/netfilter/xt_CT.c
+++ b/net/netfilter/xt_CT.c
@@ -202,9 +202,10 @@ static int xt_ct_tg_check(const struct xt_tgchk_param *par,
 		goto err1;
 
 	ct = nf_ct_tmpl_alloc(par->net, info->zone, GFP_KERNEL);
-	ret = PTR_ERR(ct);
-	if (IS_ERR(ct))
+	if (!ct) {
+		ret = -ENOMEM;
 		goto err2;
+	}
 
 	ret = 0;
 	if ((info->ct_events || info->exp_events) &&

Re: [patch -master] netfilter: xt_CT: checking for IS_ERR() instead of NULL

[Pablo Neira Ayuso]

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 <dan.carpenter@oracle.com>

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;
        }

> diff --git a/net/netfilter/xt_CT.c b/net/netfilter/xt_CT.c
> index c663003..43ddeee 100644
> --- a/net/netfilter/xt_CT.c
> +++ b/net/netfilter/xt_CT.c
> @@ -202,9 +202,10 @@ static int xt_ct_tg_check(const struct xt_tgchk_param *par,
>  		goto err1;
>  
>  	ct = nf_ct_tmpl_alloc(par->net, info->zone, GFP_KERNEL);
> -	ret = PTR_ERR(ct);
> -	if (IS_ERR(ct))
> +	if (!ct) {
> +		ret = -ENOMEM;
>  		goto err2;
> +	}
>  
>  	ret = 0;
>  	if ((info->ct_events || info->exp_events) &&

Re: [patch -master] netfilter: xt_CT: checking for IS_ERR() instead of NULL

[Dan Carpenter]

On Thu, Jul 30, 2015 at 01:57:43PM +0200, Pablo Neira Ayuso wrote:
> I have also appended this chunk, since synproxy is also affected:

Thanks.  I have rechecked my scripts and I *should* have caught that.
I'm not sure what went wrong...

regards,
dan carpenter

Re: [patch -master] netfilter: xt_CT: checking for IS_ERR() instead of NULL

[Pablo Neira Ayuso]

On Thu, Jul 30, 2015 at 03:01:08PM +0300, Dan Carpenter wrote:
> On Thu, Jul 30, 2015 at 01:57:43PM +0200, Pablo Neira Ayuso wrote:
> > I have also appended this chunk, since synproxy is also affected:
> 
> Thanks.  I have rechecked my scripts and I *should* have caught that.
> I'm not sure what went wrong...

Never trust robots :-P

Re: [patch -master] netfilter: xt_CT: checking for IS_ERR() instead of NULL

[Joe Stringer]

On 30 July 2015 at 04:57, Pablo Neira Ayuso <pablo@netfilter.org> 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 <dan.carpenter@oracle.com>
>
> 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.

Re: [patch -master] netfilter: xt_CT: checking for IS_ERR() instead of NULL

[Joe Stringer]

On 3 August 2015 at 11:29, Joe Stringer <joestringer@nicira.com> wrote:
> On 30 July 2015 at 04:57, Pablo Neira Ayuso <pablo@netfilter.org> 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 <dan.carpenter@oracle.com>
>>
>> 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.

Re: [patch -master] netfilter: xt_CT: checking for IS_ERR() instead of NULL

[Pablo Neira Ayuso]

On Mon, Aug 03, 2015 at 11:30:16AM -0700, Joe Stringer wrote:
> On 3 August 2015 at 11:29, Joe Stringer <joestringer@nicira.com> wrote:
> > On 30 July 2015 at 04:57, Pablo Neira Ayuso <pablo@netfilter.org> 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 <dan.carpenter@oracle.com>
> >>
> >> 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.

Re: [patch -master] netfilter: xt_CT: checking for IS_ERR() instead of NULL

[Joe Stringer]

On 3 August 2015 at 13:24, Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> On Mon, Aug 03, 2015 at 11:30:16AM -0700, Joe Stringer wrote:
>> On 3 August 2015 at 11:29, Joe Stringer <joestringer@nicira.com> wrote:
>> > On 30 July 2015 at 04:57, Pablo Neira Ayuso <pablo@netfilter.org> 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 <dan.carpenter@oracle.com>
>> >>
>> >> 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.