* [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.