ipvs: uninitialized data with IP_VS_IPV6

PATCH Superseded Dan Carpenter 3 participants 8 messages Dec 3, 2014 – Dec 10, 2014

Subsystems (5)

Networking Network drivers Kernel janitors netfilter +1 more subsystems

lore

Share & tools

lore

Dan Carpenter

author

2014-12-03 10:12 UTC #0 lore raw

PATCH 1/1 ipvs: uninitialized data with IP_VS_IPV6

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 <[email protected]>

---
This bug is very old.

net/netfilter/ipvs/ip_vs_ftp.c +2 -2

index 1d5341f..f93f974 100644
@@ -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;

Julian Anastasov

maintainer

2014-12-04 21:19 UTC #1 lore raw

Hello,

Quoted 6 lines · 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 <[email protected]>
---
This bug is very old.

I guess ip_vs_ftp_in() needs the same fix?

Regards

--
Julian Anastasov <[email protected]>

Dan Carpenter

author

2014-12-05 07:25 UTC #2 lore raw

Quoted 2 lines · Julian Anastasov

I guess ip_vs_ftp_in() needs the same fix?

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

regards,
dan carpenter

Dan Carpenter

author

2014-12-06 13:49 UTC #3 lore raw

PATCH v2 1/1 ipvs: uninitialized data with IP_VS_IPV6

In reply to Julian Anastasov

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 <[email protected]>

---
v2: fix app_tcp_pkt_in() as well. This is an old bug.

net/netfilter/ipvs/ip_vs_ftp.c +5 -5

index 1d5341f..5d3daae 100644
@@ -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;

Julian Anastasov

maintainer Acked-by on v2 1/1

2014-12-07 18:39 UTC #4 lore raw

Hello,

Quoted 9 lines · 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 <[email protected]>
---
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 <[email protected]>

Quoted 33 lines

net/netfilter/ipvs/ip_vs_ftp.c +5 -2

index 1d5341f..5d3daae 100644
@@ -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

net/netfilter/ipvs/ip_vs_ftp.c static int ip_vs_ftp_in(struct ip_vs_app *app, struct ip_vs_conn *cp, · line 338

Quoted 9 lines

net/netfilter/ipvs/ip_vs_ftp.c +0 -3

@@ -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 <[email protected]>

Simon Horman

maintainer

2014-12-09 00:52 UTC #5 lore raw

Quoted 11 lines · Julian Anastasov

Hello,
Quoted 6 lines · 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.

Quoted 8 lines

Quoted 3 lines
Signed-off-by Dan Carpenter <[email protected]>
---
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.

Quoted 52 lines

Simon, please apply.

Acked-by Julian Anastasov <[email protected]>

Quoted 33 lines

net/netfilter/ipvs/ip_vs_ftp.c +5 -2

index 1d5341f..5d3daae 100644
@@ -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

net/netfilter/ipvs/ip_vs_ftp.c static int ip_vs_ftp_in(struct ip_vs_app *app, struct ip_vs_conn *cp, · line 338

Quoted 9 lines

net/netfilter/ipvs/ip_vs_ftp.c +0 -3

@@ -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 <[email protected]>

Dan Carpenter

author

2014-12-09 06:49 UTC #6 lore raw

Quoted 16 lines · Simon Horman

Quoted 15 lines · Julian Anastasov
Hello,
Quoted 9 lines · 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 <[email protected]>
---
v2: fix app_tcp_pkt_in() as well. This is an old bug.

Quoted 8 lines

Quoted 4 lines
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

Simon Horman

maintainer

2014-12-10 08:47 UTC #7 lore raw

Quoted 22 lines · Dan Carpenter

Quoted 21 lines · Simon Horman
Quoted 19 lines · Julian Anastasov
Hello,
Quoted 9 lines · 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 <[email protected]>
---
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?

Quoted 5 lines

Quoted 3 lines
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.