[net] vxlan: fix the issue that neigh proxy blocks all icmpv6 packets

Message ID 4044ad2be78d1905117fba5e464ce67b22f48633.1510401530.git.lucien.xin@gmail.com
State Accepted
Delegated to: David Miller
Headers show
Series
  • [net] vxlan: fix the issue that neigh proxy blocks all icmpv6 packets
Related show

Commit Message

Xin Long Nov. 11, 2017, 11:58 a.m.
Commit f1fb08f6337c ("vxlan: fix ND proxy when skb doesn't have transport
header offset") removed icmp6_code and icmp6_type check before calling
neigh_reduce when doing neigh proxy.

It means all icmpv6 packets would be blocked by this, not only ns packet.
In Jianlin's env, even ping6 couldn't work through it.

This patch is to bring the icmp6_code and icmp6_type check back and also
removed the same check from neigh_reduce().

Fixes: f1fb08f6337c ("vxlan: fix ND proxy when skb doesn't have transport header offset")
Reported-by: Jianlin Shi <jishi@redhat.com>
Signed-off-by: Xin Long <lucien.xin@gmail.com>
---
 drivers/net/vxlan.c | 31 +++++++++++++------------------
 1 file changed, 13 insertions(+), 18 deletions(-)

Comments

Vincent Bernat Nov. 11, 2017, 12:44 p.m. | #1
❦ 11 novembre 2017 19:58 +0800, Xin Long <lucien.xin@gmail.com> :

> Commit f1fb08f6337c ("vxlan: fix ND proxy when skb doesn't have transport
> header offset") removed icmp6_code and icmp6_type check before calling
> neigh_reduce when doing neigh proxy.
>
> It means all icmpv6 packets would be blocked by this, not only ns packet.
> In Jianlin's env, even ping6 couldn't work through it.
>
> This patch is to bring the icmp6_code and icmp6_type check back and also
> removed the same check from neigh_reduce().

I am very sorry for not having spotted this bug earlier. I have tested
your fix and I can confirm it works as expected.

> Fixes: f1fb08f6337c ("vxlan: fix ND proxy when skb doesn't have transport header offset")
> Reported-by: Jianlin Shi <jishi@redhat.com>
> Signed-off-by: Xin Long <lucien.xin@gmail.com>
Reviewed-by: Vincent Bernat <vincent@bernat.im>
David Miller Nov. 14, 2017, 7:13 a.m. | #2
From: Xin Long <lucien.xin@gmail.com>
Date: Sat, 11 Nov 2017 19:58:50 +0800

> Commit f1fb08f6337c ("vxlan: fix ND proxy when skb doesn't have transport
> header offset") removed icmp6_code and icmp6_type check before calling
> neigh_reduce when doing neigh proxy.
> 
> It means all icmpv6 packets would be blocked by this, not only ns packet.
> In Jianlin's env, even ping6 couldn't work through it.
> 
> This patch is to bring the icmp6_code and icmp6_type check back and also
> removed the same check from neigh_reduce().
> 
> Fixes: f1fb08f6337c ("vxlan: fix ND proxy when skb doesn't have transport header offset")
> Reported-by: Jianlin Shi <jishi@redhat.com>
> Signed-off-by: Xin Long <lucien.xin@gmail.com>

Applied and queued up for -stable, thanks.

Patch

diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c
index d7c49cf..a2f4e52 100644
--- a/drivers/net/vxlan.c
+++ b/drivers/net/vxlan.c
@@ -1623,26 +1623,19 @@  static struct sk_buff *vxlan_na_create(struct sk_buff *request,
 static int neigh_reduce(struct net_device *dev, struct sk_buff *skb, __be32 vni)
 {
 	struct vxlan_dev *vxlan = netdev_priv(dev);
-	struct nd_msg *msg;
-	const struct ipv6hdr *iphdr;
 	const struct in6_addr *daddr;
-	struct neighbour *n;
+	const struct ipv6hdr *iphdr;
 	struct inet6_dev *in6_dev;
+	struct neighbour *n;
+	struct nd_msg *msg;
 
 	in6_dev = __in6_dev_get(dev);
 	if (!in6_dev)
 		goto out;
 
-	if (!pskb_may_pull(skb, sizeof(struct ipv6hdr) + sizeof(struct nd_msg)))
-		goto out;
-
 	iphdr = ipv6_hdr(skb);
 	daddr = &iphdr->daddr;
-
 	msg = (struct nd_msg *)(iphdr + 1);
-	if (msg->icmph.icmp6_code != 0 ||
-	    msg->icmph.icmp6_type != NDISC_NEIGHBOUR_SOLICITATION)
-		goto out;
 
 	if (ipv6_addr_loopback(daddr) ||
 	    ipv6_addr_is_multicast(&msg->target))
@@ -2240,11 +2233,11 @@  static void vxlan_xmit_one(struct sk_buff *skb, struct net_device *dev,
 static netdev_tx_t vxlan_xmit(struct sk_buff *skb, struct net_device *dev)
 {
 	struct vxlan_dev *vxlan = netdev_priv(dev);
+	struct vxlan_rdst *rdst, *fdst = NULL;
 	const struct ip_tunnel_info *info;
-	struct ethhdr *eth;
 	bool did_rsc = false;
-	struct vxlan_rdst *rdst, *fdst = NULL;
 	struct vxlan_fdb *f;
+	struct ethhdr *eth;
 	__be32 vni = 0;
 
 	info = skb_tunnel_info(skb);
@@ -2269,12 +2262,14 @@  static netdev_tx_t vxlan_xmit(struct sk_buff *skb, struct net_device *dev)
 		if (ntohs(eth->h_proto) == ETH_P_ARP)
 			return arp_reduce(dev, skb, vni);
 #if IS_ENABLED(CONFIG_IPV6)
-		else if (ntohs(eth->h_proto) == ETH_P_IPV6) {
-			struct ipv6hdr *hdr, _hdr;
-			if ((hdr = skb_header_pointer(skb,
-						      skb_network_offset(skb),
-						      sizeof(_hdr), &_hdr)) &&
-			    hdr->nexthdr == IPPROTO_ICMPV6)
+		else if (ntohs(eth->h_proto) == ETH_P_IPV6 &&
+			 pskb_may_pull(skb, sizeof(struct ipv6hdr) +
+					    sizeof(struct nd_msg)) &&
+			 ipv6_hdr(skb)->nexthdr == IPPROTO_ICMPV6) {
+			struct nd_msg *m = (struct nd_msg *)(ipv6_hdr(skb) + 1);
+
+			if (m->icmph.icmp6_code == 0 &&
+			    m->icmph.icmp6_type == NDISC_NEIGHBOUR_SOLICITATION)
 				return neigh_reduce(dev, skb, vni);
 		}
 #endif