[PATCH] ipv4: reset flowi parameters on route connect

[PATCH] ipv4: reset flowi parameters on route connect

[Julian Anastasov]

	Eric Dumazet found that commit 813b3b5db83
(ipv4: Use caller's on-stack flowi as-is in output
route lookups.) that comes in 3.0 added a regression.
The problem appears to be that resulting flowi4_oif is
used incorrectly as input parameter to some routing lookups.
The result is that when connecting to local port without
listener if the IP address that is used is not on a loopback
interface we incorrectly assign RTN_UNICAST to the output
route because no route is matched by oif=lo. The RST packet
can not be sent immediately by tcp_v4_send_reset because
it expects RTN_LOCAL.

	So, change ip_route_connect and ip_route_newports to
update the flowi4 fields that are input parameters because
we do not want unnecessary binding to oif.

	To make it clear what are the input parameters that
can be modified during lookup and to show which fields of
floiw4 are reused add a new function to update the flowi4
structure: flowi4_update_output.

Thanks to Yurij M. Plotnikov for providing a bug report including a
program to reproduce the problem.

Thanks to Eric Dumazet for tracking the problem down to
tcp_v4_send_reset and providing initial fix.

Reported-by: Yurij M. Plotnikov <Yurij.Plotnikov@oktetlabs.ru>
Signed-off-by: Julian Anastasov <ja@ssi.bg>
---

	Eric, please comment and ack this change by signing it.

 include/net/flow.h  |   10 ++++++++++
 include/net/route.h |    4 ++++
 2 files changed, 14 insertions(+), 0 deletions(-)

diff --git a/include/net/flow.h b/include/net/flow.h
index 9b58243..6c469db 100644
--- a/include/net/flow.h
+++ b/include/net/flow.h
@@ -93,6 +93,16 @@ static inline void flowi4_init_output(struct flowi4 *fl4, int oif,
 	fl4->fl4_dport = dport;
 	fl4->fl4_sport = sport;
 }
+
+/* Reset some input parameters after previous lookup */
+static inline void flowi4_update_output(struct flowi4 *fl4, int oif, __u8 tos,
+					__be32 daddr, __be32 saddr)
+{
+	fl4->flowi4_oif = oif;
+	fl4->flowi4_tos = tos;
+	fl4->daddr = daddr;
+	fl4->saddr = saddr;
+}
 				      
 
 struct flowi6 {
diff --git a/include/net/route.h b/include/net/route.h
index 91855d1..b1c0d5b 100644
--- a/include/net/route.h
+++ b/include/net/route.h
@@ -270,6 +270,7 @@ static inline struct rtable *ip_route_connect(struct flowi4 *fl4,
 		if (IS_ERR(rt))
 			return rt;
 		ip_rt_put(rt);
+		flowi4_update_output(fl4, oif, tos, fl4->daddr, fl4->saddr);
 	}
 	security_sk_classify_flow(sk, flowi4_to_flowi(fl4));
 	return ip_route_output_flow(net, fl4, sk);
@@ -284,6 +285,9 @@ static inline struct rtable *ip_route_newports(struct flowi4 *fl4, struct rtable
 		fl4->fl4_dport = dport;
 		fl4->fl4_sport = sport;
 		ip_rt_put(rt);
+		flowi4_update_output(fl4, sk->sk_bound_dev_if,
+				     RT_CONN_FLAGS(sk), fl4->daddr,
+				     fl4->saddr);
 		security_sk_classify_flow(sk, flowi4_to_flowi(fl4));
 		return ip_route_output_flow(sock_net(sk), fl4, sk);
 	}
-- 
1.7.3.4

Re: [PATCH] ipv4: reset flowi parameters on route connect

[Eric Dumazet]

Le dimanche 05 février 2012 à 01:04 +0200, Julian Anastasov a écrit :
> 	Eric Dumazet found that commit 813b3b5db83
> (ipv4: Use caller's on-stack flowi as-is in output
> route lookups.) that comes in 3.0 added a regression.
> The problem appears to be that resulting flowi4_oif is
> used incorrectly as input parameter to some routing lookups.
> The result is that when connecting to local port without
> listener if the IP address that is used is not on a loopback
> interface we incorrectly assign RTN_UNICAST to the output
> route because no route is matched by oif=lo. The RST packet
> can not be sent immediately by tcp_v4_send_reset because
> it expects RTN_LOCAL.
> 
> 	So, change ip_route_connect and ip_route_newports to
> update the flowi4 fields that are input parameters because
> we do not want unnecessary binding to oif.
> 
> 	To make it clear what are the input parameters that
> can be modified during lookup and to show which fields of
> floiw4 are reused add a new function to update the flowi4
> structure: flowi4_update_output.
> 
> Thanks to Yurij M. Plotnikov for providing a bug report including a
> program to reproduce the problem.
> 
> Thanks to Eric Dumazet for tracking the problem down to
> tcp_v4_send_reset and providing initial fix.
> 
> Reported-by: Yurij M. Plotnikov <Yurij.Plotnikov@oktetlabs.ru>
> Signed-off-by: Julian Anastasov <ja@ssi.bg>
> ---
> 
> 	Eric, please comment and ack this change by signing it.
> 
>  include/net/flow.h  |   10 ++++++++++
>  include/net/route.h |    4 ++++
>  2 files changed, 14 insertions(+), 0 deletions(-)
> 
> diff --git a/include/net/flow.h b/include/net/flow.h
> index 9b58243..6c469db 100644
> --- a/include/net/flow.h
> +++ b/include/net/flow.h
> @@ -93,6 +93,16 @@ static inline void flowi4_init_output(struct flowi4 *fl4, int oif,
>  	fl4->fl4_dport = dport;
>  	fl4->fl4_sport = sport;
>  }
> +
> +/* Reset some input parameters after previous lookup */
> +static inline void flowi4_update_output(struct flowi4 *fl4, int oif, __u8 tos,
> +					__be32 daddr, __be32 saddr)
> +{
> +	fl4->flowi4_oif = oif;
> +	fl4->flowi4_tos = tos;
> +	fl4->daddr = daddr;
> +	fl4->saddr = saddr;
> +}
>  				      
>  
>  struct flowi6 {
> diff --git a/include/net/route.h b/include/net/route.h
> index 91855d1..b1c0d5b 100644
> --- a/include/net/route.h
> +++ b/include/net/route.h
> @@ -270,6 +270,7 @@ static inline struct rtable *ip_route_connect(struct flowi4 *fl4,
>  		if (IS_ERR(rt))
>  			return rt;
>  		ip_rt_put(rt);
> +		flowi4_update_output(fl4, oif, tos, fl4->daddr, fl4->saddr);
>  	}
>  	security_sk_classify_flow(sk, flowi4_to_flowi(fl4));
>  	return ip_route_output_flow(net, fl4, sk);
> @@ -284,6 +285,9 @@ static inline struct rtable *ip_route_newports(struct flowi4 *fl4, struct rtable
>  		fl4->fl4_dport = dport;
>  		fl4->fl4_sport = sport;
>  		ip_rt_put(rt);
> +		flowi4_update_output(fl4, sk->sk_bound_dev_if,
> +				     RT_CONN_FLAGS(sk), fl4->daddr,
> +				     fl4->saddr);
>  		security_sk_classify_flow(sk, flowi4_to_flowi(fl4));
>  		return ip_route_output_flow(sock_net(sk), fl4, sk);
>  	}

This seems very good to me, thanks !

Acked-by: Eric Dumazet <eric.dumazet@gmail.com>

Re: [PATCH] ipv4: reset flowi parameters on route connect

[David Miller]

From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Sun, 05 Feb 2012 00:33:36 +0100

> Le dimanche 05 février 2012 à 01:04 +0200, Julian Anastasov a écrit :
>> 	Eric Dumazet found that commit 813b3b5db83
>> (ipv4: Use caller's on-stack flowi as-is in output
>> route lookups.) that comes in 3.0 added a regression.
>> The problem appears to be that resulting flowi4_oif is
>> used incorrectly as input parameter to some routing lookups.
>> The result is that when connecting to local port without
>> listener if the IP address that is used is not on a loopback
>> interface we incorrectly assign RTN_UNICAST to the output
>> route because no route is matched by oif=lo. The RST packet
>> can not be sent immediately by tcp_v4_send_reset because
>> it expects RTN_LOCAL.
>> 
>> 	So, change ip_route_connect and ip_route_newports to
>> update the flowi4 fields that are input parameters because
>> we do not want unnecessary binding to oif.
>> 
>> 	To make it clear what are the input parameters that
>> can be modified during lookup and to show which fields of
>> floiw4 are reused add a new function to update the flowi4
>> structure: flowi4_update_output.
>> 
>> Thanks to Yurij M. Plotnikov for providing a bug report including a
>> program to reproduce the problem.
>> 
>> Thanks to Eric Dumazet for tracking the problem down to
>> tcp_v4_send_reset and providing initial fix.
>> 
>> Reported-by: Yurij M. Plotnikov <Yurij.Plotnikov@oktetlabs.ru>
>> Signed-off-by: Julian Anastasov <ja@ssi.bg>
...
> Acked-by: Eric Dumazet <eric.dumazet@gmail.com>

Applied, thanks everyone.