[patch] ipvs: uninitialized data with IP_VS_IPV6

[patch] ipvs: uninitialized data with IP_VS_IPV6

[Dan Carpenter]

The app_tcp_pkt_out() function expects "*diff" to be set and ends up
using uninitialized data if CONFIG_IP_VS_IPV6 is turned on.

Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
---
This bug is very old.

diff --git a/net/netfilter/ipvs/ip_vs_ftp.c b/net/netfilter/ipvs/ip_vs_ftp.c
index 1d5341f..f93f974 100644
--- a/net/netfilter/ipvs/ip_vs_ftp.c
+++ b/net/netfilter/ipvs/ip_vs_ftp.c
@@ -183,6 +183,8 @@ static int ip_vs_ftp_out(struct ip_vs_app *app, struct ip_vs_conn *cp,
 	struct nf_conn *ct;
 	struct net *net;
 
+	*diff = 0;
+
 #ifdef CONFIG_IP_VS_IPV6
 	/* This application helper doesn't work with IPv6 yet,
 	 * so turn this into a no-op for IPv6 packets
@@ -191,8 +193,6 @@ static int ip_vs_ftp_out(struct ip_vs_app *app, struct ip_vs_conn *cp,
 		return 1;
 #endif
 
-	*diff = 0;
-
 	/* Only useful for established sessions */
 	if (cp->state != IP_VS_TCP_S_ESTABLISHED)
 		return 1;

Re: [patch] ipvs: uninitialized data with IP_VS_IPV6

[Julian Anastasov]

	Hello,

On Wed, 3 Dec 2014, Dan Carpenter wrote:

> The app_tcp_pkt_out() function expects "*diff" to be set and ends up
> using uninitialized data if CONFIG_IP_VS_IPV6 is turned on.
> 
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> ---
> This bug is very old.

	I guess ip_vs_ftp_in() needs the same fix?

Regards

--
Julian Anastasov <ja@ssi.bg>

Re: [patch] ipvs: uninitialized data with IP_VS_IPV6

[Dan Carpenter]

On Thu, Dec 04, 2014 at 11:19:34PM +0200, Julian Anastasov wrote:
> 
> 	I guess ip_vs_ftp_in() needs the same fix?

Good catch.  Thanks for noticing that.  I'll send a v2.

regards,
dan carpenter

[patch v2] ipvs: uninitialized data with IP_VS_IPV6

[Dan Carpenter]

The app_tcp_pkt_out() function expects "*diff" to be set and ends up
using uninitialized data if CONFIG_IP_VS_IPV6 is turned on.

The same issue is there in app_tcp_pkt_in().  Thanks to Julian Anastasov
for noticing that.

Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
---
v2: fix app_tcp_pkt_in() as well.  This is an old bug.

diff --git a/net/netfilter/ipvs/ip_vs_ftp.c b/net/netfilter/ipvs/ip_vs_ftp.c
index 1d5341f..5d3daae 100644
--- a/net/netfilter/ipvs/ip_vs_ftp.c
+++ b/net/netfilter/ipvs/ip_vs_ftp.c
@@ -183,6 +183,8 @@ static int ip_vs_ftp_out(struct ip_vs_app *app, struct ip_vs_conn *cp,
 	struct nf_conn *ct;
 	struct net *net;
 
+	*diff = 0;
+
 #ifdef CONFIG_IP_VS_IPV6
 	/* This application helper doesn't work with IPv6 yet,
 	 * so turn this into a no-op for IPv6 packets
@@ -191,8 +193,6 @@ static int ip_vs_ftp_out(struct ip_vs_app *app, struct ip_vs_conn *cp,
 		return 1;
 #endif
 
-	*diff = 0;
-
 	/* Only useful for established sessions */
 	if (cp->state != IP_VS_TCP_S_ESTABLISHED)
 		return 1;
@@ -322,6 +322,9 @@ static int ip_vs_ftp_in(struct ip_vs_app *app, struct ip_vs_conn *cp,
 	struct ip_vs_conn *n_cp;
 	struct net *net;
 
+	/* no diff required for incoming packets */
+	*diff = 0;
+
 #ifdef CONFIG_IP_VS_IPV6
 	/* This application helper doesn't work with IPv6 yet,
 	 * so turn this into a no-op for IPv6 packets
@@ -330,9 +333,6 @@ static int ip_vs_ftp_in(struct ip_vs_app *app, struct ip_vs_conn *cp,
 		return 1;
 #endif
 
-	/* no diff required for incoming packets */
-	*diff = 0;

Re: [patch v2] ipvs: uninitialized data with IP_VS_IPV6

[Julian Anastasov]

	Hello,

On Sat, 6 Dec 2014, Dan Carpenter wrote:

> The app_tcp_pkt_out() function expects "*diff" to be set and ends up
> using uninitialized data if CONFIG_IP_VS_IPV6 is turned on.
> 
> The same issue is there in app_tcp_pkt_in().  Thanks to Julian Anastasov
> for noticing that.
> 
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> ---
> v2: fix app_tcp_pkt_in() as well.  This is an old bug.

	Thanks! It will not break seqs for IPv6 control
connection, only that we do not support FTP yet :( I have
the doubt whether this should be classified as bugfix :)
I guess, it is a net-next material, right?

	Simon, please apply.

Acked-by: Julian Anastasov <ja@ssi.bg>

> diff --git a/net/netfilter/ipvs/ip_vs_ftp.c b/net/netfilter/ipvs/ip_vs_ftp.c
> index 1d5341f..5d3daae 100644
> --- a/net/netfilter/ipvs/ip_vs_ftp.c
> +++ b/net/netfilter/ipvs/ip_vs_ftp.c
> @@ -183,6 +183,8 @@ static int ip_vs_ftp_out(struct ip_vs_app *app, struct ip_vs_conn *cp,
>  	struct nf_conn *ct;
>  	struct net *net;
>  
> +	*diff = 0;
> +
>  #ifdef CONFIG_IP_VS_IPV6
>  	/* This application helper doesn't work with IPv6 yet,
>  	 * so turn this into a no-op for IPv6 packets
> @@ -191,8 +193,6 @@ static int ip_vs_ftp_out(struct ip_vs_app *app, struct ip_vs_conn *cp,
>  		return 1;
>  #endif
>  
> -	*diff = 0;
> -
>  	/* Only useful for established sessions */
>  	if (cp->state != IP_VS_TCP_S_ESTABLISHED)
>  		return 1;
> @@ -322,6 +322,9 @@ static int ip_vs_ftp_in(struct ip_vs_app *app, struct ip_vs_conn *cp,
>  	struct ip_vs_conn *n_cp;
>  	struct net *net;
>  
> +	/* no diff required for incoming packets */
> +	*diff = 0;
> +
>  #ifdef CONFIG_IP_VS_IPV6
>  	/* This application helper doesn't work with IPv6 yet,
>  	 * so turn this into a no-op for IPv6 packets
> @@ -330,9 +333,6 @@ static int ip_vs_ftp_in(struct ip_vs_app *app, struct ip_vs_conn *cp,
>  		return 1;
>  #endif
>  
> -	/* no diff required for incoming packets */
> -	*diff = 0;
> -
>  	/* Only useful for established sessions */
>  	if (cp->state != IP_VS_TCP_S_ESTABLISHED)
>  		return 1;

Regards

--
Julian Anastasov <ja@ssi.bg>

Re: [patch v2] ipvs: uninitialized data with IP_VS_IPV6

[Simon Horman]

On Sun, Dec 07, 2014 at 08:39:35PM +0200, Julian Anastasov wrote:
> 
> 	Hello,
> 
> On Sat, 6 Dec 2014, Dan Carpenter wrote:
> 
> > The app_tcp_pkt_out() function expects "*diff" to be set and ends up
> > using uninitialized data if CONFIG_IP_VS_IPV6 is turned on.
> > 
> > The same issue is there in app_tcp_pkt_in().  Thanks to Julian Anastasov
> > for noticing that.
> > 
> > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> > ---
> > v2: fix app_tcp_pkt_in() as well.  This is an old bug.
> 
> 	Thanks! It will not break seqs for IPv6 control
> connection, only that we do not support FTP yet :( I have
> the doubt whether this should be classified as bugfix :)
> I guess, it is a net-next material, right?

Agreed, I have queued it up in ipvs-next.
I'll send a pull request to Pablo if Dan doesn't object
to it going there.

> 	Simon, please apply.
> 
> Acked-by: Julian Anastasov <ja@ssi.bg>
> 
> > diff --git a/net/netfilter/ipvs/ip_vs_ftp.c b/net/netfilter/ipvs/ip_vs_ftp.c
> > index 1d5341f..5d3daae 100644
> > --- a/net/netfilter/ipvs/ip_vs_ftp.c
> > +++ b/net/netfilter/ipvs/ip_vs_ftp.c
> > @@ -183,6 +183,8 @@ static int ip_vs_ftp_out(struct ip_vs_app *app, struct ip_vs_conn *cp,
> >  	struct nf_conn *ct;
> >  	struct net *net;
> >  
> > +	*diff = 0;
> > +
> >  #ifdef CONFIG_IP_VS_IPV6
> >  	/* This application helper doesn't work with IPv6 yet,
> >  	 * so turn this into a no-op for IPv6 packets
> > @@ -191,8 +193,6 @@ static int ip_vs_ftp_out(struct ip_vs_app *app, struct ip_vs_conn *cp,
> >  		return 1;
> >  #endif
> >  
> > -	*diff = 0;
> > -
> >  	/* Only useful for established sessions */
> >  	if (cp->state != IP_VS_TCP_S_ESTABLISHED)
> >  		return 1;
> > @@ -322,6 +322,9 @@ static int ip_vs_ftp_in(struct ip_vs_app *app, struct ip_vs_conn *cp,
> >  	struct ip_vs_conn *n_cp;
> >  	struct net *net;
> >  
> > +	/* no diff required for incoming packets */
> > +	*diff = 0;
> > +
> >  #ifdef CONFIG_IP_VS_IPV6
> >  	/* This application helper doesn't work with IPv6 yet,
> >  	 * so turn this into a no-op for IPv6 packets
> > @@ -330,9 +333,6 @@ static int ip_vs_ftp_in(struct ip_vs_app *app, struct ip_vs_conn *cp,
> >  		return 1;
> >  #endif
> >  
> > -	/* no diff required for incoming packets */
> > -	*diff = 0;
> > -
> >  	/* Only useful for established sessions */
> >  	if (cp->state != IP_VS_TCP_S_ESTABLISHED)
> >  		return 1;
> 
> Regards
> 
> --
> Julian Anastasov <ja@ssi.bg>
> 

Re: [patch v2] ipvs: uninitialized data with IP_VS_IPV6

[Dan Carpenter]

On Tue, Dec 09, 2014 at 09:52:15AM +0900, Simon Horman wrote:
> On Sun, Dec 07, 2014 at 08:39:35PM +0200, Julian Anastasov wrote:
> > 
> > 	Hello,
> > 
> > On Sat, 6 Dec 2014, Dan Carpenter wrote:
> > 
> > > The app_tcp_pkt_out() function expects "*diff" to be set and ends up
> > > using uninitialized data if CONFIG_IP_VS_IPV6 is turned on.
> > > 
> > > The same issue is there in app_tcp_pkt_in().  Thanks to Julian Anastasov
> > > for noticing that.
> > > 
> > > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> > > ---
> > > v2: fix app_tcp_pkt_in() as well.  This is an old bug.
> > 
> > 	Thanks! It will not break seqs for IPv6 control
> > connection, only that we do not support FTP yet :( I have
> > the doubt whether this should be classified as bugfix :)
> > I guess, it is a net-next material, right?
> 
> Agreed, I have queued it up in ipvs-next.
> I'll send a pull request to Pablo if Dan doesn't object
> to it going there.

No objections from me.

regards,
dan carpenter

Re: [patch v2] ipvs: uninitialized data with IP_VS_IPV6

[Simon Horman]

On Tue, Dec 09, 2014 at 09:49:47AM +0300, Dan Carpenter wrote:
> On Tue, Dec 09, 2014 at 09:52:15AM +0900, Simon Horman wrote:
> > On Sun, Dec 07, 2014 at 08:39:35PM +0200, Julian Anastasov wrote:
> > > 
> > > 	Hello,
> > > 
> > > On Sat, 6 Dec 2014, Dan Carpenter wrote:
> > > 
> > > > The app_tcp_pkt_out() function expects "*diff" to be set and ends up
> > > > using uninitialized data if CONFIG_IP_VS_IPV6 is turned on.
> > > > 
> > > > The same issue is there in app_tcp_pkt_in().  Thanks to Julian Anastasov
> > > > for noticing that.
> > > > 
> > > > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> > > > ---
> > > > v2: fix app_tcp_pkt_in() as well.  This is an old bug.
> > > 
> > > 	Thanks! It will not break seqs for IPv6 control
> > > connection, only that we do not support FTP yet :( I have
> > > the doubt whether this should be classified as bugfix :)
> > > I guess, it is a net-next material, right?
> > 
> > Agreed, I have queued it up in ipvs-next.
> > I'll send a pull request to Pablo if Dan doesn't object
> > to it going there.
> 
> No objections from me.

Thanks, I have sent a pull-request.