Re: [net-next v2] vxlan: fix ND proxy when skb doesn't have transport header offset
From: Vincent Bernat <hidden>
Date: 2017-03-30 14:56:31
❦ 30 mars 2017 06:36 -0700, Eric Dumazet [off-list ref] :
quoted
quoted
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.
quoted hunk ↗ jump to hunk
So the following would be ok (while incomplete of course)diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c index bdb6ae16d4a85bf9539199e189011bce104ba51a..cd032819d4a36d5ca94739f20f947f3f5a31aba3100644--- 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"