[PATCH net 0/2] net: Fix looping with vrf, xfrms and qdisc on VRF

[PATCH net 0/2] net: Fix looping with vrf, xfrms and qdisc on VRF

[David Ahern]

From: David Ahern <dsahern@gmail.com>

Trev reported that use of VRFs with xfrms is looping when a qdisc
is added to the VRF device. The combination of xfrm + qdisc is not
handled by the VRF driver which lost track that it has already
seen the packet.

The XFRM_TRANSFORMED flag is used by the netfilter code for a similar
purpose, so re-use for VRF. Patch 1 drops the #ifdef around setting
the flag in the xfrm output functions. Patch 2 adds a check to
the VRF driver for flag; if set the packet has already passed through
the VRF driver once and does not need to recirculated a second time.

This is a day 1 bug with VRFs; stable wise, I would only take this
back to 4.14. I have a set of test cases which I will submit to
net-next.

David Ahern (2):
  xfrm: Always set XFRM_TRANSFORMED in xfrm{4,6}_output_finish
  vrf: Check skb for XFRM_TRANSFORMED flag

 drivers/net/vrf.c       | 6 ++++--
 net/ipv4/xfrm4_output.c | 2 --
 net/ipv6/xfrm6_output.c | 2 --
 3 files changed, 4 insertions(+), 6 deletions(-)

-- 
2.20.1

[PATCH net 1/2] xfrm: Always set XFRM_TRANSFORMED in xfrm{4,6}_output_finish

[David Ahern]

From: David Ahern <dsahern@gmail.com>

IPSKB_XFRM_TRANSFORMED and IP6SKB_XFRM_TRANSFORMED are skb flags set by
xfrm code to tell other skb handlers that the packet has been passed
through the xfrm output functions. Simplify the code and just always
set them rather than conditionally based on netfilter enabled thus
making the flag available for other users.

Signed-off-by: David Ahern <dsahern@gmail.com>
---
 net/ipv4/xfrm4_output.c | 2 --
 net/ipv6/xfrm6_output.c | 2 --
 2 files changed, 4 deletions(-)

diff --git a/net/ipv4/xfrm4_output.c b/net/ipv4/xfrm4_output.c
index 89ba7c87de5d..30ddb9dc9398 100644
--- a/net/ipv4/xfrm4_output.c
+++ b/net/ipv4/xfrm4_output.c
@@ -58,9 +58,7 @@ int xfrm4_output_finish(struct sock *sk, struct sk_buff *skb)
 {
 	memset(IPCB(skb), 0, sizeof(*IPCB(skb)));
 
-#ifdef CONFIG_NETFILTER
 	IPCB(skb)->flags |= IPSKB_XFRM_TRANSFORMED;
-#endif
 
 	return xfrm_output(sk, skb);
 }
diff --git a/net/ipv6/xfrm6_output.c b/net/ipv6/xfrm6_output.c
index fbe51d40bd7e..e34167f790e6 100644
--- a/net/ipv6/xfrm6_output.c
+++ b/net/ipv6/xfrm6_output.c
@@ -111,9 +111,7 @@ int xfrm6_output_finish(struct sock *sk, struct sk_buff *skb)
 {
 	memset(IP6CB(skb), 0, sizeof(*IP6CB(skb)));
 
-#ifdef CONFIG_NETFILTER
 	IP6CB(skb)->flags |= IP6SKB_XFRM_TRANSFORMED;
-#endif
 
 	return xfrm_output(sk, skb);
 }
-- 
2.20.1

[PATCH net 2/2] vrf: Check skb for XFRM_TRANSFORMED flag

[David Ahern]

From: David Ahern <dsahern@gmail.com>

To avoid a loop with qdiscs and xfrms, check if the skb has already gone
through the qdisc attached to the VRF device and then to the xfrm layer.
If so, no need for a second redirect.

Fixes: 193125dbd8eb ("net: Introduce VRF device driver")
Reported-by: Trev Larock <trev@larock.ca>
Signed-off-by: David Ahern <dsahern@gmail.com>
---
 drivers/net/vrf.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/net/vrf.c b/drivers/net/vrf.c
index 66e00ddc0d42..6f5d03b7d9c0 100644
--- a/drivers/net/vrf.c
+++ b/drivers/net/vrf.c
@@ -474,7 +474,8 @@ static struct sk_buff *vrf_ip6_out(struct net_device *vrf_dev,
 	if (rt6_need_strict(&ipv6_hdr(skb)->daddr))
 		return skb;
 
-	if (qdisc_tx_is_default(vrf_dev))
+	if (qdisc_tx_is_default(vrf_dev) ||
+	    IP6CB(skb)->flags & IP6SKB_XFRM_TRANSFORMED)
 		return vrf_ip6_out_direct(vrf_dev, sk, skb);
 
 	return vrf_ip6_out_redirect(vrf_dev, skb);
@@ -686,7 +687,8 @@ static struct sk_buff *vrf_ip_out(struct net_device *vrf_dev,
 	    ipv4_is_lbcast(ip_hdr(skb)->daddr))
 		return skb;
 
-	if (qdisc_tx_is_default(vrf_dev))
+	if (qdisc_tx_is_default(vrf_dev) ||
+	    IPCB(skb)->flags & IPSKB_XFRM_TRANSFORMED)
 		return vrf_ip_out_direct(vrf_dev, sk, skb);
 
 	return vrf_ip_out_redirect(vrf_dev, skb);
-- 
2.20.1

Re: [PATCH net 0/2] net: Fix looping with vrf, xfrms and qdisc on VRF

[David Miller]

From: David Ahern <dsahern@kernel.org>
Date: Mon, 20 Apr 2020 17:13:50 -0600

> From: David Ahern <dsahern@gmail.com>
> 
> Trev reported that use of VRFs with xfrms is looping when a qdisc
> is added to the VRF device. The combination of xfrm + qdisc is not
> handled by the VRF driver which lost track that it has already
> seen the packet.
> 
> The XFRM_TRANSFORMED flag is used by the netfilter code for a similar
> purpose, so re-use for VRF. Patch 1 drops the #ifdef around setting
> the flag in the xfrm output functions. Patch 2 adds a check to
> the VRF driver for flag; if set the packet has already passed through
> the VRF driver once and does not need to recirculated a second time.
> 
> This is a day 1 bug with VRFs; stable wise, I would only take this
> back to 4.14. I have a set of test cases which I will submit to
> net-next.

Series applied and queued up for -stable, thanks David.