Patchwork net: netfilter/xt_CT.c: fix uninitialized variable

login
register
mail settings
Submitter Cong Ding
Date Jan. 15, 2013, 6:58 p.m.
Message ID <1358276314-9536-1-git-send-email-dinggnu@gmail.com>
Download mbox | patch
Permalink /patch/212298/
State Not Applicable
Delegated to: David Miller
Headers show

Comments

Cong Ding - Jan. 15, 2013, 6:58 p.m.
If CONFIG_NF_CONNTRACK_ZONES is not defined, the variable ret might be
uninitialized when it goes to err1 through line 125 and 263 respectively.
So I change these goto err1 to return -EINVAL directly.

Signed-off-by: Cong Ding <dinggnu@gmail.com>
---
 net/netfilter/xt_CT.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
Waskiewicz Jr, Peter P - Jan. 15, 2013, 7:06 p.m.
On Tue, 2013-01-15 at 19:58 +0100, Cong Ding wrote:
> If CONFIG_NF_CONNTRACK_ZONES is not defined, the variable ret might be

> uninitialized when it goes to err1 through line 125 and 263 respectively.

> So I change these goto err1 to return -EINVAL directly.

> 

> Signed-off-by: Cong Ding <dinggnu@gmail.com>

> ---

>  net/netfilter/xt_CT.c |    4 ++--

>  1 file changed, 2 insertions(+), 2 deletions(-)

> 

> diff --git a/net/netfilter/xt_CT.c b/net/netfilter/xt_CT.c

> index 2a08430..941f600 100644

> --- a/net/netfilter/xt_CT.c

> +++ b/net/netfilter/xt_CT.c

> @@ -122,7 +122,7 @@ static int xt_ct_tg_check_v0(const struct xt_tgchk_param *par)

>  

>  #ifndef CONFIG_NF_CONNTRACK_ZONES

>  	if (info->zone)

> -		goto err1;

> +		return -EINVAL;

>  #endif

>  

>  	ret = nf_ct_l3proto_try_module_get(par->family);

> @@ -260,7 +260,7 @@ static int xt_ct_tg_check_v1(const struct xt_tgchk_param *par)

>  

>  #ifndef CONFIG_NF_CONNTRACK_ZONES

>  	if (info->zone)

> -		goto err1;

> +		return -EINVAL;

>  #endif


In dropping both goto's, you left the err1 label unused.  Wouldn't just
initializing ret to -EINVAL be easier and cleaner?  Then you wouldn't be
messing with the flow of the function.

-PJ
Cong Ding - Jan. 15, 2013, 7:16 p.m.
On Tue, Jan 15, 2013 at 07:06:51PM +0000, Waskiewicz Jr, Peter P wrote:
> On Tue, 2013-01-15 at 19:58 +0100, Cong Ding wrote:
> > If CONFIG_NF_CONNTRACK_ZONES is not defined, the variable ret might be
> > uninitialized when it goes to err1 through line 125 and 263 respectively.
> > So I change these goto err1 to return -EINVAL directly.
> > 
> > Signed-off-by: Cong Ding <dinggnu@gmail.com>
> > ---
> >  net/netfilter/xt_CT.c |    4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/net/netfilter/xt_CT.c b/net/netfilter/xt_CT.c
> > index 2a08430..941f600 100644
> > --- a/net/netfilter/xt_CT.c
> > +++ b/net/netfilter/xt_CT.c
> > @@ -122,7 +122,7 @@ static int xt_ct_tg_check_v0(const struct xt_tgchk_param *par)
> >  
> >  #ifndef CONFIG_NF_CONNTRACK_ZONES
> >  	if (info->zone)
> > -		goto err1;
> > +		return -EINVAL;
> >  #endif
> >  
> >  	ret = nf_ct_l3proto_try_module_get(par->family);
> > @@ -260,7 +260,7 @@ static int xt_ct_tg_check_v1(const struct xt_tgchk_param *par)
> >  
> >  #ifndef CONFIG_NF_CONNTRACK_ZONES
> >  	if (info->zone)
> > -		goto err1;
> > +		return -EINVAL;
> >  #endif
> 
> In dropping both goto's, you left the err1 label unused.  Wouldn't just
> initializing ret to -EINVAL be easier and cleaner?  Then you wouldn't be
> messing with the flow of the function.
The label err1 are also used in line 130 and 298. I change it to "return
-EINVAL" rather than initialize ret to -EINVAL is to keep it the same as line
115 and 253.  Otherwise, we should change line 115 and 253 to be goto err1,
too?
- cong
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Pablo Neira - Jan. 15, 2013, 7:18 p.m.
Hi Cong,

On Tue, Jan 15, 2013 at 07:58:34PM +0100, Cong Ding wrote:
> If CONFIG_NF_CONNTRACK_ZONES is not defined, the variable ret might be
> uninitialized when it goes to err1 through line 125 and 263 respectively.
> So I change these goto err1 to return -EINVAL directly.

This is already fixed in the net tree.

http://git.kernel.org/?p=linux/kernel/git/davem/net.git;a=commit;h=4610476d89d53714ca94aae081fa035908bc137a
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/net/netfilter/xt_CT.c b/net/netfilter/xt_CT.c
index 2a08430..941f600 100644
--- a/net/netfilter/xt_CT.c
+++ b/net/netfilter/xt_CT.c
@@ -122,7 +122,7 @@  static int xt_ct_tg_check_v0(const struct xt_tgchk_param *par)
 
 #ifndef CONFIG_NF_CONNTRACK_ZONES
 	if (info->zone)
-		goto err1;
+		return -EINVAL;
 #endif
 
 	ret = nf_ct_l3proto_try_module_get(par->family);
@@ -260,7 +260,7 @@  static int xt_ct_tg_check_v1(const struct xt_tgchk_param *par)
 
 #ifndef CONFIG_NF_CONNTRACK_ZONES
 	if (info->zone)
-		goto err1;
+		return -EINVAL;
 #endif
 
 	ret = nf_ct_l3proto_try_module_get(par->family);