All of lore.kernel.org
 help / color / mirror / Atom feed
* [net-next v2] vxlan: fix ND proxy when skb doesn't have transport header offset
@ 2017-03-29 20:47 Vincent Bernat
  2017-03-30  6:41 ` Vincent Bernat
  0 siblings, 1 reply; 8+ messages in thread
From: Vincent Bernat @ 2017-03-29 20:47 UTC (permalink / raw)
  To: Eric Dumazet, Roopa Prabhu, David S. Miller, Jiri Benc, netdev
  Cc: Vincent Bernat

When an incoming frame is tagged or when GRO is disabled, the skb
handled to vxlan_xmit() doesn't contain a valid transport header
offset. This makes ND proxying fail.

Do not rely on skb_transport_offset(). Instead, use offsets from
skb_network_offset() with skb_header_pointer() to extract appropriate
information to detect an ICMPv6 neighbor solicitation and to extract the
information needed to build the answer.

Duplicate checks at the beginning of neigh_reduce() are removed.

Parsing of neighbor discovery options is done earlier to ignore the
whole packet in case of a malformed option. Moreover, the assumption the
skb was linear is removed and options are extracted with
skb_header_pointer() as well. The check on the source link-layer address
option is also more strict (for Ethernet, we expect the length field to
be 1).

After this change, the vxlan module is correctly able to answer to
VLAN-encapsulated frames:

22:02:46.332573 50:54:33:00:00:02 > 33:33:00:00:00:02, ethertype 802.1Q (0x8100), length 74: vlan 100, p 0,ethertype IPv6, (hlim 255, next-header ICMPv6 (58) payload length: 16) fe80::5254:33ff:fe00:2 > ff02::2: [icmp6 sum ok] ICMP6, router solicitation, length 16
          source link-address option (1), length 8 (1): 50:54:33:00:00:02
            0x0000:  5054 3300 0002
22:02:47.846631 50:54:33:00:00:08 > 33:33:00:00:00:02, ethertype 802.1Q (0x8100), length 74: vlan 100, p 0,ethertype IPv6, (hlim 255, next-header ICMPv6 (58) payload length: 16) fe80::5254:33ff:fe00:8 > ff02::2: [icmp6 sum ok] ICMP6, router solicitation, length 16
          source link-address option (1), length 8 (1): 50:54:33:00:00:08
            0x0000:  5054 3300 0008

Signed-off-by: Vincent Bernat <vincent@bernat.im>
---
 drivers/net/vxlan.c | 85 ++++++++++++++++++++++++++++++-----------------------
 1 file changed, 48 insertions(+), 37 deletions(-)

diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c
index 1e54fb5c883a..f40272785aa2 100644
--- a/drivers/net/vxlan.c
+++ b/drivers/net/vxlan.c
@@ -1504,20 +1504,43 @@ static int arp_reduce(struct net_device *dev, struct sk_buff *skb, __be32 vni)
 
 #if IS_ENABLED(CONFIG_IPV6)
 static struct sk_buff *vxlan_na_create(struct sk_buff *request,
-	struct neighbour *n, bool isrouter)
+				       struct ipv6hdr *ip6h, struct nd_msg *ns,
+				       struct neighbour *n, bool isrouter)
 {
 	struct net_device *dev = request->dev;
 	struct sk_buff *reply;
-	struct nd_msg *ns, *na;
+	struct nd_msg *na;
 	struct ipv6hdr *pip6;
-	u8 *daddr;
+	u8 *daddr, _daddr[ETH_ALEN];
 	int na_olen = 8; /* opt hdr + ETH_ALEN for target */
-	int ns_olen;
-	int i, len;
+	int len, remaining, offset;
 
 	if (dev == NULL)
 		return NULL;
 
+	/* Destination address is the "source link-layer address"
+	 * option if present and valid or the source Ethernet
+	 * address */
+	daddr = eth_hdr(request)->h_source;
+	remaining = htons(ip6h->payload_len) - sizeof(*ns);
+	offset = skb_network_offset(request) + sizeof(*ip6h) + sizeof(*ns);
+	while (remaining > sizeof(struct nd_opt_hdr)) {
+		struct nd_opt_hdr *ohdr, _ohdr;
+		ohdr = skb_header_pointer(request, offset, sizeof(_ohdr), &_ohdr);
+		if (!ohdr || !(len = ohdr->nd_opt_len<<3) || len > remaining)
+			return NULL;
+		if (ohdr->nd_opt_type == ND_OPT_SOURCE_LL_ADDR &&
+		    len == na_olen) {
+			daddr = skb_header_pointer(request, offset + sizeof(_ohdr),
+						   sizeof(_daddr), _daddr);
+			if (!daddr)
+				return NULL;
+			break;
+		}
+		remaining -= len;
+		offset += len;
+	}
+
 	len = LL_RESERVED_SPACE(dev) + sizeof(struct ipv6hdr) +
 		sizeof(*na) + na_olen + dev->needed_tailroom;
 	reply = alloc_skb(len, GFP_ATOMIC);
@@ -1530,16 +1553,6 @@ static struct sk_buff *vxlan_na_create(struct sk_buff *request,
 	skb_push(reply, sizeof(struct ethhdr));
 	skb_reset_mac_header(reply);
 
-	ns = (struct nd_msg *)skb_transport_header(request);
-
-	daddr = eth_hdr(request)->h_source;
-	ns_olen = request->len - skb_transport_offset(request) - sizeof(*ns);
-	for (i = 0; i < ns_olen-1; i += (ns->opt[i+1]<<3)) {
-		if (ns->opt[i] == ND_OPT_SOURCE_LL_ADDR) {
-			daddr = ns->opt + i + sizeof(struct nd_opt_hdr);
-			break;
-		}
-	}
 
 	/* Ethernet header */
 	ether_addr_copy(eth_hdr(reply)->h_dest, daddr);
@@ -1556,10 +1569,10 @@ static struct sk_buff *vxlan_na_create(struct sk_buff *request,
 	pip6 = ipv6_hdr(reply);
 	memset(pip6, 0, sizeof(struct ipv6hdr));
 	pip6->version = 6;
-	pip6->priority = ipv6_hdr(request)->priority;
+	pip6->priority = ip6h->priority;
 	pip6->nexthdr = IPPROTO_ICMPV6;
 	pip6->hop_limit = 255;
-	pip6->daddr = ipv6_hdr(request)->saddr;
+	pip6->daddr = ip6h->saddr;
 	pip6->saddr = *(struct in6_addr *)n->primary_key;
 
 	skb_pull(reply, sizeof(struct ipv6hdr));
@@ -1591,11 +1604,11 @@ static struct sk_buff *vxlan_na_create(struct sk_buff *request,
 	return reply;
 }
 
-static int neigh_reduce(struct net_device *dev, struct sk_buff *skb, __be32 vni)
+static int neigh_reduce(struct net_device *dev, struct sk_buff *skb,
+			struct ipv6hdr *iphdr, struct nd_msg *msg,
+			__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;
 	struct inet6_dev *in6_dev;
@@ -1604,14 +1617,8 @@ static int neigh_reduce(struct net_device *dev, struct sk_buff *skb, __be32 vni)
 	if (!in6_dev)
 		goto out;
 
-	iphdr = ipv6_hdr(skb);
 	daddr = &iphdr->daddr;
 
-	msg = (struct nd_msg *)skb_transport_header(skb);
-	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))
 		goto out;
@@ -1634,7 +1641,7 @@ static int neigh_reduce(struct net_device *dev, struct sk_buff *skb, __be32 vni)
 			goto out;
 		}
 
-		reply = vxlan_na_create(skb, n,
+		reply = vxlan_na_create(skb, iphdr, msg, n,
 					!!(f ? f->flags & NTF_ROUTER : 0));
 
 		neigh_release(n);
@@ -2242,16 +2249,20 @@ 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 &&
-			 pskb_may_pull(skb, sizeof(struct ipv6hdr)
-				       + sizeof(struct nd_msg)) &&
-			 ipv6_hdr(skb)->nexthdr == IPPROTO_ICMPV6) {
-				struct nd_msg *msg;
-
-				msg = (struct nd_msg *)skb_transport_header(skb);
-				if (msg->icmph.icmp6_code == 0 &&
-				    msg->icmph.icmp6_type == NDISC_NEIGHBOUR_SOLICITATION)
-					return neigh_reduce(dev, skb, vni);
+		else if (ntohs(eth->h_proto) == ETH_P_IPV6) {
+			struct ipv6hdr *hdr, _hdr;
+			struct nd_msg *msg, _msg;
+			if ((hdr = skb_header_pointer(skb,
+						      skb_network_offset(skb),
+						      sizeof(_hdr), &_hdr)) &&
+			    hdr->nexthdr == IPPROTO_ICMPV6 &&
+			    (msg = skb_header_pointer(skb,
+						      skb_network_offset(skb) +
+						      sizeof(_hdr),
+						      sizeof(_msg), &_msg)) &&
+			    msg->icmph.icmp6_code == 0 &&
+			    msg->icmph.icmp6_type == NDISC_NEIGHBOUR_SOLICITATION)
+				return neigh_reduce(dev, skb, hdr, msg, vni);
 		}
 #endif
 	}
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [net-next v2] vxlan: fix ND proxy when skb doesn't have transport header offset
  2017-03-29 20:47 [net-next v2] vxlan: fix ND proxy when skb doesn't have transport header offset Vincent Bernat
@ 2017-03-30  6:41 ` Vincent Bernat
  2017-03-30 13:36   ` Eric Dumazet
  0 siblings, 1 reply; 8+ messages in thread
From: Vincent Bernat @ 2017-03-30  6:41 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: Roopa Prabhu, David S. Miller, Jiri Benc, netdev

 ❦ 29 mars 2017 22:47 +0200, Vincent Bernat <vincent@bernat.im> :

> Parsing of neighbor discovery options is done earlier to ignore the
> whole packet in case of a malformed option. Moreover, the assumption the
> skb was linear is removed and options are extracted with
> skb_header_pointer() as well. The check on the source link-layer address
> option is also more strict (for Ethernet, we expect the length field to
> be 1).

There is some parsing implemented in net/ipv6/ndisc.c, notably
ndisc_parse_options(). I don't know if this is a good idea to reuse
that: it may have the expectation that some IP processing has already
been done (for example, the IPv6 length has already been checked, the
SKB is expected to be linear).
-- 
Watch out for off-by-one errors.
            - The Elements of Programming Style (Kernighan & Plauger)

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [net-next v2] vxlan: fix ND proxy when skb doesn't have transport header offset
  2017-03-30  6:41 ` Vincent Bernat
@ 2017-03-30 13:36   ` Eric Dumazet
  2017-03-30 14:56     ` Vincent Bernat
  2017-03-31  8:18     ` [net-next v3] " Vincent Bernat
  0 siblings, 2 replies; 8+ messages in thread
From: Eric Dumazet @ 2017-03-30 13:36 UTC (permalink / raw)
  To: Vincent Bernat; +Cc: Roopa Prabhu, David S. Miller, Jiri Benc, netdev

On Wed, Mar 29, 2017 at 11:41 PM, Vincent Bernat <vincent@bernat.im> wrote:
>  ❦ 29 mars 2017 22:47 +0200, Vincent Bernat <vincent@bernat.im> :
>
>> Parsing of neighbor discovery options is done earlier to ignore the
>> whole packet in case of a malformed option. Moreover, the assumption the
>> skb was linear is removed and options are extracted with
>> skb_header_pointer() as well. The check on the source link-layer address
>> option is also more strict (for Ethernet, we expect the length field to
>> be 1).
>
> There is some parsing implemented in net/ipv6/ndisc.c, notably
> ndisc_parse_options(). I don't know if this is a good idea to reuse
> that: it may have the expectation that some IP processing has already
> been done (for example, the IPv6 length has already been checked, the
> SKB is expected to be linear).

Forcing ICMP being linear is probably fine.

Prior to 91269e390d062b52 ("vxlan: using pskb_may_pull as early as possible")
this was happening (at the wrong place) in neigh_reduce() doing a :
if (!pskb_may_pull(skb, skb->len))
     goto out;

So the following would be ok (while incomplete of course)

diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c
index bdb6ae16d4a85bf9539199e189011bce104ba51a..cd032819d4a36d5ca94739f20f947f3f5a31aba3
100644
--- a/drivers/net/vxlan.c
+++ b/drivers/net/vxlan.c
@@ -2243,12 +2243,13 @@ static netdev_tx_t vxlan_xmit(struct sk_buff
*skb, struct net_device *dev)
                        return arp_reduce(dev, skb, vni);
 #if IS_ENABLED(CONFIG_IPV6)
                else if (ntohs(eth->h_proto) == ETH_P_IPV6 &&
-                        pskb_may_pull(skb, sizeof(struct ipv6hdr)
-                                      + sizeof(struct nd_msg)) &&
+                        skb->len >= sizeof(struct ipv6hdr) +
+                                    sizeof(struct nd_msg) &&
+                        pskb_may_pull(skb, skb->len) &&
                         ipv6_hdr(skb)->nexthdr == IPPROTO_ICMPV6) {
                                struct nd_msg *msg;

-                               msg = (struct nd_msg
*)skb_transport_header(skb);
+                               msg = (struct nd_msg *)(ipv6_hdr(skb) + 1);
                                if (msg->icmph.icmp6_code == 0 &&
                                    msg->icmph.icmp6_type ==
NDISC_NEIGHBOUR_SOLICITATION)
                                        return neigh_reduce(dev, skb, vni);

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [net-next v2] vxlan: fix ND proxy when skb doesn't have transport header offset
  2017-03-30 13:36   ` Eric Dumazet
@ 2017-03-30 14:56     ` Vincent Bernat
  2017-03-31  8:18     ` [net-next v3] " Vincent Bernat
  1 sibling, 0 replies; 8+ messages in thread
From: Vincent Bernat @ 2017-03-30 14:56 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: Roopa Prabhu, David S. Miller, Jiri Benc, netdev

 ❦ 30 mars 2017 06:36 -0700, Eric Dumazet <edumazet@google.com> :

>>> Parsing of neighbor discovery options is done earlier to ignore the
>>> whole packet in case of a malformed option. Moreover, the assumption the
>>> skb was linear is removed and options are extracted with
>>> skb_header_pointer() as well. The check on the source link-layer address
>>> option is also more strict (for Ethernet, we expect the length field to
>>> be 1).
>>
>> There is some parsing implemented in net/ipv6/ndisc.c, notably
>> ndisc_parse_options(). I don't know if this is a good idea to reuse
>> that: it may have the expectation that some IP processing has already
>> been done (for example, the IPv6 length has already been checked, the
>> SKB is expected to be linear).
>
> Forcing ICMP being linear is probably fine.
>
> Prior to 91269e390d062b52 ("vxlan: using pskb_may_pull as early as possible")
> this was happening (at the wrong place) in neigh_reduce() doing a :
> if (!pskb_may_pull(skb, skb->len))
>      goto out;

OK, I'll simplify the patch then.

> So the following would be ok (while incomplete of course)
>
> diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c
> index bdb6ae16d4a85bf9539199e189011bce104ba51a..cd032819d4a36d5ca94739f20f947f3f5a31aba3
> 100644
> --- a/drivers/net/vxlan.c
> +++ b/drivers/net/vxlan.c
> @@ -2243,12 +2243,13 @@ static netdev_tx_t vxlan_xmit(struct sk_buff
> *skb, struct net_device *dev)
>                         return arp_reduce(dev, skb, vni);
>  #if IS_ENABLED(CONFIG_IPV6)
>                 else if (ntohs(eth->h_proto) == ETH_P_IPV6 &&
> -                        pskb_may_pull(skb, sizeof(struct ipv6hdr)
> -                                      + sizeof(struct nd_msg)) &&
> +                        skb->len >= sizeof(struct ipv6hdr) +
> +                                    sizeof(struct nd_msg) &&
> +                        pskb_may_pull(skb, skb->len) &&
>                          ipv6_hdr(skb)->nexthdr == IPPROTO_ICMPV6) {
>                                 struct nd_msg *msg;
>
> -                               msg = (struct nd_msg
> *)skb_transport_header(skb);
> +                               msg = (struct nd_msg *)(ipv6_hdr(skb) + 1);
>                                 if (msg->icmph.icmp6_code == 0 &&
>                                     msg->icmph.icmp6_type ==
> NDISC_NEIGHBOUR_SOLICITATION)
>                                         return neigh_reduce(dev, skb, vni);

pskb_may_pull() is called while we only know this is an IPv6 packet, not
an ICMPv6 one. I'll keep skb_header_pointer to handle IPv6 header, then
I'll pull the whole ICMP packet (unless I am mistaken, of course).
-- 
The devil can cite Scripture for his purpose.
		-- William Shakespeare, "The Merchant of Venice"

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [net-next v3] vxlan: fix ND proxy when skb doesn't have transport header offset
  2017-03-30 13:36   ` Eric Dumazet
  2017-03-30 14:56     ` Vincent Bernat
@ 2017-03-31  8:18     ` Vincent Bernat
  2017-04-01 20:22       ` kbuild test robot
  1 sibling, 1 reply; 8+ messages in thread
From: Vincent Bernat @ 2017-03-31  8:18 UTC (permalink / raw)
  To: Eric Dumazet, Roopa Prabhu, David S. Miller, Jiri Benc, netdev,
	Cong Wang
  Cc: Vincent Bernat

When an incoming frame is tagged or when GRO is disabled, the skb
handled to vxlan_xmit() doesn't contain a valid transport header
offset. This makes ND proxying fail.

We combine two changes: replace use of skb_transport_offset() and ensure
the necessary amount of skb is linear just before using it:

 - In vxlan_xmit(), when determining if we have an ICMPv6 neighbor
   discovery packet, just check if it is an ICMPv6 packet and rely on
   neigh_reduce() to do more checks if this is the case. The use of
   pskb_may_pull() is replaced by skb_header_pointer() for just the IPv6
   header.

 - In neigh_reduce(), add pskb_may_pull() for IPv6 header and neighbor
   discovery message since this was removed from vxlan_xmit(). Replace
   skb_transport_header() with ipv6_hdr() + 1.

 - In vxlan_na_create(), replace first skb_transport_offset() with
   ipv6_hdr() + 1 and second with skb_network_offset() + sizeof(struct
   ipv6hdr). Additionally, ensure we pskb_may_pull() the whole skb as we
   need it to iterate over the options.

Signed-off-by: Vincent Bernat <vincent@bernat.im>
---
 drivers/net/vxlan.c | 28 ++++++++++++++--------------
 1 file changed, 14 insertions(+), 14 deletions(-)

diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c
index 1e54fb5c883a..54dda367de2b 100644
--- a/drivers/net/vxlan.c
+++ b/drivers/net/vxlan.c
@@ -1515,7 +1515,7 @@ static struct sk_buff *vxlan_na_create(struct sk_buff *request,
 	int ns_olen;
 	int i, len;
 
-	if (dev == NULL)
+	if (dev == NULL || !pskb_may_pull(request, request->len))
 		return NULL;
 
 	len = LL_RESERVED_SPACE(dev) + sizeof(struct ipv6hdr) +
@@ -1530,10 +1530,11 @@ static struct sk_buff *vxlan_na_create(struct sk_buff *request,
 	skb_push(reply, sizeof(struct ethhdr));
 	skb_reset_mac_header(reply);
 
-	ns = (struct nd_msg *)skb_transport_header(request);
+	ns = (struct nd_msg *)(ipv6_hdr(request) + 1);
 
 	daddr = eth_hdr(request)->h_source;
-	ns_olen = request->len - skb_transport_offset(request) - sizeof(*ns);
+	ns_olen = request->len - skb_network_offset(request) -
+		sizeof(struct ipv6hdr) - sizeof(*ns);
 	for (i = 0; i < ns_olen-1; i += (ns->opt[i+1]<<3)) {
 		if (ns->opt[i] == ND_OPT_SOURCE_LL_ADDR) {
 			daddr = ns->opt + i + sizeof(struct nd_opt_hdr);
@@ -1604,10 +1605,13 @@ static int neigh_reduce(struct net_device *dev, struct sk_buff *skb, __be32 vni)
 	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 *)skb_transport_header(skb);
+	msg = (struct nd_msg *)(iphdr + 1);
 	if (msg->icmph.icmp6_code != 0 ||
 	    msg->icmph.icmp6_type != NDISC_NEIGHBOUR_SOLICITATION)
 		goto out;
@@ -2238,21 +2242,17 @@ static netdev_tx_t vxlan_xmit(struct sk_buff *skb, struct net_device *dev)
 	}
 
 	if (vxlan->flags & VXLAN_F_PROXY) {
+		struct ipv6hdr *hdr, _hdr;
 		eth = eth_hdr(skb);
 		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 &&
-			 pskb_may_pull(skb, sizeof(struct ipv6hdr)
-				       + sizeof(struct nd_msg)) &&
-			 ipv6_hdr(skb)->nexthdr == IPPROTO_ICMPV6) {
-				struct nd_msg *msg;
-
-				msg = (struct nd_msg *)skb_transport_header(skb);
-				if (msg->icmph.icmp6_code == 0 &&
-				    msg->icmph.icmp6_type == NDISC_NEIGHBOUR_SOLICITATION)
-					return neigh_reduce(dev, skb, vni);
-		}
+			 (hdr = skb_header_pointer(skb,
+						   skb_network_offset(skb),
+						   sizeof(_hdr), &_hdr)) &&
+			 hdr->nexthdr == IPPROTO_ICMPV6)
+			return neigh_reduce(dev, skb, vni);
 #endif
 	}
 
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [net-next v3] vxlan: fix ND proxy when skb doesn't have transport header offset
  2017-03-31  8:18     ` [net-next v3] " Vincent Bernat
@ 2017-04-01 20:22       ` kbuild test robot
  2017-04-02  9:00         ` [PATCH net-next v4] " Vincent Bernat
  0 siblings, 1 reply; 8+ messages in thread
From: kbuild test robot @ 2017-04-01 20:22 UTC (permalink / raw)
  To: Vincent Bernat
  Cc: kbuild-all, Eric Dumazet, Roopa Prabhu, David S. Miller,
	Jiri Benc, netdev, Cong Wang, Vincent Bernat

[-- Attachment #1: Type: text/plain, Size: 1716 bytes --]

Hi Vincent,

[auto build test WARNING on net-next/master]

url:    https://github.com/0day-ci/linux/commits/Vincent-Bernat/vxlan-fix-ND-proxy-when-skb-doesn-t-have-transport-header-offset/20170401-182312
config: x86_64-randconfig-v0-04020359 (attached as .config)
compiler: gcc-4.4 (Debian 4.4.7-8) 4.4.7
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

All warnings (new ones prefixed by >>):

   drivers//net/vxlan.c: In function 'vxlan_xmit':
>> drivers//net/vxlan.c:2245: warning: unused variable '_hdr'
>> drivers//net/vxlan.c:2245: warning: unused variable 'hdr'

vim +/_hdr +2245 drivers//net/vxlan.c

  2229		skb_reset_mac_header(skb);
  2230	
  2231		if (vxlan->flags & VXLAN_F_COLLECT_METADATA) {
  2232			if (info && info->mode & IP_TUNNEL_INFO_BRIDGE &&
  2233			    info->mode & IP_TUNNEL_INFO_TX) {
  2234				vni = tunnel_id_to_key32(info->key.tun_id);
  2235			} else {
  2236				if (info && info->mode & IP_TUNNEL_INFO_TX)
  2237					vxlan_xmit_one(skb, dev, vni, NULL, false);
  2238				else
  2239					kfree_skb(skb);
  2240				return NETDEV_TX_OK;
  2241			}
  2242		}
  2243	
  2244		if (vxlan->flags & VXLAN_F_PROXY) {
> 2245			struct ipv6hdr *hdr, _hdr;
  2246			eth = eth_hdr(skb);
  2247			if (ntohs(eth->h_proto) == ETH_P_ARP)
  2248				return arp_reduce(dev, skb, vni);
  2249	#if IS_ENABLED(CONFIG_IPV6)
  2250			else if (ntohs(eth->h_proto) == ETH_P_IPV6 &&
  2251				 (hdr = skb_header_pointer(skb,
  2252							   skb_network_offset(skb),
  2253							   sizeof(_hdr), &_hdr)) &&

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 25014 bytes --]

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH net-next v4] vxlan: fix ND proxy when skb doesn't have transport header offset
  2017-04-01 20:22       ` kbuild test robot
@ 2017-04-02  9:00         ` Vincent Bernat
  2017-04-04  1:51           ` David Miller
  0 siblings, 1 reply; 8+ messages in thread
From: Vincent Bernat @ 2017-04-02  9:00 UTC (permalink / raw)
  To: Eric Dumazet, Roopa Prabhu, David S. Miller, Jiri Benc, netdev,
	Cong Wang
  Cc: Vincent Bernat

When an incoming frame is tagged or when GRO is disabled, the skb
handled to vxlan_xmit() doesn't contain a valid transport header
offset. This makes ND proxying fail.

We combine two changes: replace use of skb_transport_offset() and ensure
the necessary amount of skb is linear just before using it:

 - In vxlan_xmit(), when determining if we have an ICMPv6 neighbor
   discovery packet, just check if it is an ICMPv6 packet and rely on
   neigh_reduce() to do more checks if this is the case. The use of
   pskb_may_pull() is replaced by skb_header_pointer() for just the IPv6
   header.

 - In neigh_reduce(), add pskb_may_pull() for IPv6 header and neighbor
   discovery message since this was removed from vxlan_xmit(). Replace
   skb_transport_header() with ipv6_hdr() + 1.

 - In vxlan_na_create(), replace first skb_transport_offset() with
   ipv6_hdr() + 1 and second with skb_network_offset() + sizeof(struct
   ipv6hdr). Additionally, ensure we pskb_may_pull() the whole skb as we
   need it to iterate over the options.

Signed-off-by: Vincent Bernat <vincent@bernat.im>
---
 drivers/net/vxlan.c | 29 +++++++++++++++--------------
 1 file changed, 15 insertions(+), 14 deletions(-)

diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c
index 1e54fb5c883a..89cb86666267 100644
--- a/drivers/net/vxlan.c
+++ b/drivers/net/vxlan.c
@@ -1515,7 +1515,7 @@ static struct sk_buff *vxlan_na_create(struct sk_buff *request,
 	int ns_olen;
 	int i, len;
 
-	if (dev == NULL)
+	if (dev == NULL || !pskb_may_pull(request, request->len))
 		return NULL;
 
 	len = LL_RESERVED_SPACE(dev) + sizeof(struct ipv6hdr) +
@@ -1530,10 +1530,11 @@ static struct sk_buff *vxlan_na_create(struct sk_buff *request,
 	skb_push(reply, sizeof(struct ethhdr));
 	skb_reset_mac_header(reply);
 
-	ns = (struct nd_msg *)skb_transport_header(request);
+	ns = (struct nd_msg *)(ipv6_hdr(request) + 1);
 
 	daddr = eth_hdr(request)->h_source;
-	ns_olen = request->len - skb_transport_offset(request) - sizeof(*ns);
+	ns_olen = request->len - skb_network_offset(request) -
+		sizeof(struct ipv6hdr) - sizeof(*ns);
 	for (i = 0; i < ns_olen-1; i += (ns->opt[i+1]<<3)) {
 		if (ns->opt[i] == ND_OPT_SOURCE_LL_ADDR) {
 			daddr = ns->opt + i + sizeof(struct nd_opt_hdr);
@@ -1604,10 +1605,13 @@ static int neigh_reduce(struct net_device *dev, struct sk_buff *skb, __be32 vni)
 	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 *)skb_transport_header(skb);
+	msg = (struct nd_msg *)(iphdr + 1);
 	if (msg->icmph.icmp6_code != 0 ||
 	    msg->icmph.icmp6_type != NDISC_NEIGHBOUR_SOLICITATION)
 		goto out;
@@ -2242,16 +2246,13 @@ 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 &&
-			 pskb_may_pull(skb, sizeof(struct ipv6hdr)
-				       + sizeof(struct nd_msg)) &&
-			 ipv6_hdr(skb)->nexthdr == IPPROTO_ICMPV6) {
-				struct nd_msg *msg;
-
-				msg = (struct nd_msg *)skb_transport_header(skb);
-				if (msg->icmph.icmp6_code == 0 &&
-				    msg->icmph.icmp6_type == NDISC_NEIGHBOUR_SOLICITATION)
-					return neigh_reduce(dev, skb, vni);
+		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)
+				return neigh_reduce(dev, skb, vni);
 		}
 #endif
 	}
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH net-next v4] vxlan: fix ND proxy when skb doesn't have transport header offset
  2017-04-02  9:00         ` [PATCH net-next v4] " Vincent Bernat
@ 2017-04-04  1:51           ` David Miller
  0 siblings, 0 replies; 8+ messages in thread
From: David Miller @ 2017-04-04  1:51 UTC (permalink / raw)
  To: vincent; +Cc: edumazet, roopa, jbenc, netdev, xiyou.wangcong

From: Vincent Bernat <vincent@bernat.im>
Date: Sun,  2 Apr 2017 11:00:06 +0200

> When an incoming frame is tagged or when GRO is disabled, the skb
> handled to vxlan_xmit() doesn't contain a valid transport header
> offset. This makes ND proxying fail.
> 
> We combine two changes: replace use of skb_transport_offset() and ensure
> the necessary amount of skb is linear just before using it:
> 
>  - In vxlan_xmit(), when determining if we have an ICMPv6 neighbor
>    discovery packet, just check if it is an ICMPv6 packet and rely on
>    neigh_reduce() to do more checks if this is the case. The use of
>    pskb_may_pull() is replaced by skb_header_pointer() for just the IPv6
>    header.
> 
>  - In neigh_reduce(), add pskb_may_pull() for IPv6 header and neighbor
>    discovery message since this was removed from vxlan_xmit(). Replace
>    skb_transport_header() with ipv6_hdr() + 1.
> 
>  - In vxlan_na_create(), replace first skb_transport_offset() with
>    ipv6_hdr() + 1 and second with skb_network_offset() + sizeof(struct
>    ipv6hdr). Additionally, ensure we pskb_may_pull() the whole skb as we
>    need it to iterate over the options.
> 
> Signed-off-by: Vincent Bernat <vincent@bernat.im>

Applied, thanks.

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2017-04-04  1:51 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-03-29 20:47 [net-next v2] vxlan: fix ND proxy when skb doesn't have transport header offset Vincent Bernat
2017-03-30  6:41 ` Vincent Bernat
2017-03-30 13:36   ` Eric Dumazet
2017-03-30 14:56     ` Vincent Bernat
2017-03-31  8:18     ` [net-next v3] " Vincent Bernat
2017-04-01 20:22       ` kbuild test robot
2017-04-02  9:00         ` [PATCH net-next v4] " Vincent Bernat
2017-04-04  1:51           ` David Miller

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.