2014-04-25 08:46:30

by Xufeng Zhang

[permalink] [raw]
Subject: [PATCH V2] sctp: reset flowi4_oif parameter on route lookup

commit 813b3b5db83 (ipv4: Use caller's on-stack flowi as-is
in output route lookups.) introduces another regression which
is very similar to the problem of commit e6b45241c (ipv4: reset
flowi parameters on route connect) wants to fix:
Before we call ip_route_output_key() in sctp_v4_get_dst() to
get a dst that matches a bind address as the source address,
we have already called this function previously and the flowi
parameters have been initialized including flowi4_oif, so when
we call this function again, the process in __ip_route_output_key()
will be different because of the setting of flowi4_oif, and we'll
get a networking device which corresponds to the inputted flowi4_oif
as the output device, this is wrong because we'll never hit this
place if the previously returned source address of dst match one
of the bound addresses.

To reproduce this problem, a vlan setting is enough:
# ifconfig eth0 up
# route del default
# vconfig add eth0 2
# vconfig add eth0 3
# ifconfig eth0.2 10.0.1.14 netmask 255.255.255.0
# route add default gw 10.0.1.254 dev eth0.2
# ifconfig eth0.3 10.0.0.14 netmask 255.255.255.0
# ip rule add from 10.0.0.14 table 4
# ip route add table 4 default via 10.0.0.254 src 10.0.0.14 dev eth0.3
# sctp_darn -H 10.0.0.14 -P 36422 -h 10.1.4.134 -p 36422 -s -I
You'll detect that all the flow are routed to eth0.2(10.0.1.254).

Signed-off-by: Xufeng Zhang <[email protected]>
Signed-off-by: Julian Anastasov <[email protected]>
---
Changelog:

v1->v2:
Suggested by Julian Anastasov, reset flowi parameters by flowi4_update_output().

net/sctp/protocol.c | 7 ++++++-
1 files changed, 6 insertions(+), 1 deletions(-)

diff --git a/net/sctp/protocol.c b/net/sctp/protocol.c
index c09757f..44cbb54 100644
--- a/net/sctp/protocol.c
+++ b/net/sctp/protocol.c
@@ -491,8 +491,13 @@ static void sctp_v4_get_dst(struct sctp_transport *t, union sctp_addr *saddr,
continue;
if ((laddr->state == SCTP_ADDR_SRC) &&
(AF_INET == laddr->a.sa.sa_family)) {
- fl4->saddr = laddr->a.v4.sin_addr.s_addr;
fl4->fl4_sport = laddr->a.v4.sin_port;
+ flowi4_update_output(fl4,
+ asoc->base.sk->sk_bound_dev_if,
+ RT_CONN_FLAGS(asoc->base.sk),
+ daddr->v4.sin_addr.s_addr,
+ laddr->a.v4.sin_addr.s_addr);
+
rt = ip_route_output_key(sock_net(sk), fl4);
if (!IS_ERR(rt)) {
dst = &rt->dst;
--
1.7.0.2


2014-04-25 13:37:50

by Greg KH

[permalink] [raw]
Subject: Re: [PATCH V2] sctp: reset flowi4_oif parameter on route lookup

On Fri, Apr 25, 2014 at 04:55:41PM +0800, Xufeng Zhang wrote:
> commit 813b3b5db83 (ipv4: Use caller's on-stack flowi as-is
> in output route lookups.) introduces another regression which
> is very similar to the problem of commit e6b45241c (ipv4: reset
> flowi parameters on route connect) wants to fix:
> Before we call ip_route_output_key() in sctp_v4_get_dst() to
> get a dst that matches a bind address as the source address,
> we have already called this function previously and the flowi
> parameters have been initialized including flowi4_oif, so when
> we call this function again, the process in __ip_route_output_key()
> will be different because of the setting of flowi4_oif, and we'll
> get a networking device which corresponds to the inputted flowi4_oif
> as the output device, this is wrong because we'll never hit this
> place if the previously returned source address of dst match one
> of the bound addresses.
>
> To reproduce this problem, a vlan setting is enough:
> # ifconfig eth0 up
> # route del default
> # vconfig add eth0 2
> # vconfig add eth0 3
> # ifconfig eth0.2 10.0.1.14 netmask 255.255.255.0
> # route add default gw 10.0.1.254 dev eth0.2
> # ifconfig eth0.3 10.0.0.14 netmask 255.255.255.0
> # ip rule add from 10.0.0.14 table 4
> # ip route add table 4 default via 10.0.0.254 src 10.0.0.14 dev eth0.3
> # sctp_darn -H 10.0.0.14 -P 36422 -h 10.1.4.134 -p 36422 -s -I
> You'll detect that all the flow are routed to eth0.2(10.0.1.254).
>
> Signed-off-by: Xufeng Zhang <[email protected]>
> Signed-off-by: Julian Anastasov <[email protected]>
> ---
> Changelog:
>
> v1->v2:
> Suggested by Julian Anastasov, reset flowi parameters by flowi4_update_output().
>
> net/sctp/protocol.c | 7 ++++++-
> 1 files changed, 6 insertions(+), 1 deletions(-)

<formletter>

This is not the correct way to submit patches for inclusion in the
stable kernel tree. Please read Documentation/stable_kernel_rules.txt
for how to do this properly.

</formletter>

2014-04-25 13:45:10

by Vlad Yasevich

[permalink] [raw]
Subject: Re: [PATCH V2] sctp: reset flowi4_oif parameter on route lookup

On 04/25/2014 04:55 AM, Xufeng Zhang wrote:
> commit 813b3b5db83 (ipv4: Use caller's on-stack flowi as-is
> in output route lookups.) introduces another regression which
> is very similar to the problem of commit e6b45241c (ipv4: reset
> flowi parameters on route connect) wants to fix:
> Before we call ip_route_output_key() in sctp_v4_get_dst() to
> get a dst that matches a bind address as the source address,
> we have already called this function previously and the flowi
> parameters have been initialized including flowi4_oif, so when
> we call this function again, the process in __ip_route_output_key()
> will be different because of the setting of flowi4_oif, and we'll
> get a networking device which corresponds to the inputted flowi4_oif
> as the output device, this is wrong because we'll never hit this
> place if the previously returned source address of dst match one
> of the bound addresses.
>
> To reproduce this problem, a vlan setting is enough:
> # ifconfig eth0 up
> # route del default
> # vconfig add eth0 2
> # vconfig add eth0 3
> # ifconfig eth0.2 10.0.1.14 netmask 255.255.255.0
> # route add default gw 10.0.1.254 dev eth0.2
> # ifconfig eth0.3 10.0.0.14 netmask 255.255.255.0
> # ip rule add from 10.0.0.14 table 4
> # ip route add table 4 default via 10.0.0.254 src 10.0.0.14 dev eth0.3
> # sctp_darn -H 10.0.0.14 -P 36422 -h 10.1.4.134 -p 36422 -s -I
> You'll detect that all the flow are routed to eth0.2(10.0.1.254).
>
> Signed-off-by: Xufeng Zhang <[email protected]>
> Signed-off-by: Julian Anastasov <[email protected]>

Acked-by: Vlad Yasevich <[email protected]>

-vlad

2014-04-27 23:54:54

by David Miller

[permalink] [raw]
Subject: Re: [PATCH V2] sctp: reset flowi4_oif parameter on route lookup

From: Xufeng Zhang <[email protected]>
Date: Fri, 25 Apr 2014 16:55:41 +0800

> commit 813b3b5db83 (ipv4: Use caller's on-stack flowi as-is
> in output route lookups.) introduces another regression which
> is very similar to the problem of commit e6b45241c (ipv4: reset
> flowi parameters on route connect) wants to fix:
> Before we call ip_route_output_key() in sctp_v4_get_dst() to
> get a dst that matches a bind address as the source address,
> we have already called this function previously and the flowi
> parameters have been initialized including flowi4_oif, so when
> we call this function again, the process in __ip_route_output_key()
> will be different because of the setting of flowi4_oif, and we'll
> get a networking device which corresponds to the inputted flowi4_oif
> as the output device, this is wrong because we'll never hit this
> place if the previously returned source address of dst match one
> of the bound addresses.
>
> To reproduce this problem, a vlan setting is enough:
> # ifconfig eth0 up
> # route del default
> # vconfig add eth0 2
> # vconfig add eth0 3
> # ifconfig eth0.2 10.0.1.14 netmask 255.255.255.0
> # route add default gw 10.0.1.254 dev eth0.2
> # ifconfig eth0.3 10.0.0.14 netmask 255.255.255.0
> # ip rule add from 10.0.0.14 table 4
> # ip route add table 4 default via 10.0.0.254 src 10.0.0.14 dev eth0.3
> # sctp_darn -H 10.0.0.14 -P 36422 -h 10.1.4.134 -p 36422 -s -I
> You'll detect that all the flow are routed to eth0.2(10.0.1.254).
>
> Signed-off-by: Xufeng Zhang <[email protected]>
> Signed-off-by: Julian Anastasov <[email protected]>

Applied and queued up for -stable.