There is a miss-match between the handling of invalid ICMPv6 fields in the
implementations of parse_icmpv6() in user-space and in the kernel datapath.
This patch addresses that by modifying the user-space implementation to
match that of the kernel datapath; processing is terminated without
rather than with an error and partial information is cleared.
With these changes the user-space implementation of parse_icmpv6()
never returns an error. Accordingly the return type and caller have been
updated.
The original motivation for this is to allow matching the ICMPv6 type and
code of packets with invalid neighbour discovery options although only the
change around the '(!opt_len || opt_len > *sizep)' conditional is necessary
to achieve that goal.
Signed-off-by: Simon Horman <simon.hor...@netronome.com>
---
v2
* Harmonise the entire user-space implementation of parse_icmpv6() with
kernel code rather than just the area around the '(!opt_len || opt_len >
*sizep)' conditional. As suggested by Jesse Gross.
* Update patch subject from
"flow: Ignore miss-sized IPv6 neighbour discovery options" to
"flow: Ignore invalid ICMPv6 fields"
---
lib/flow.c | 35 ++++++++++++++++++-----------------
1 file changed, 18 insertions(+), 17 deletions(-)
diff --git a/lib/flow.c b/lib/flow.c
index fdca1e0743ea..1b7a3d5b7889 100644
--- a/lib/flow.c
+++ b/lib/flow.c
@@ -366,7 +366,7 @@ parse_ethertype(const void **datap, size_t *sizep)
return htons(FLOW_DL_TYPE_NONE);
}
-static inline bool
+static inline void
parse_icmpv6(const void **datap, size_t *sizep, const struct icmp6_hdr *icmp,
const struct in6_addr **nd_target,
uint8_t arp_buf[2][ETH_ADDR_LEN])
@@ -377,7 +377,7 @@ parse_icmpv6(const void **datap, size_t *sizep, const
struct icmp6_hdr *icmp,
*nd_target = data_try_pull(datap, sizep, sizeof **nd_target);
if (OVS_UNLIKELY(!*nd_target)) {
- return false;
+ return;
}
while (*sizep >= 8) {
@@ -387,7 +387,7 @@ parse_icmpv6(const void **datap, size_t *sizep, const
struct icmp6_hdr *icmp,
int opt_len = nd_opt->nd_opt_len * 8;
if (!opt_len || opt_len > *sizep) {
- goto invalid;
+ return;
}
/* Store the link layer address if the appropriate option is
@@ -410,15 +410,18 @@ parse_icmpv6(const void **datap, size_t *sizep, const
struct icmp6_hdr *icmp,
}
if (OVS_UNLIKELY(!data_try_pull(datap, sizep, opt_len))) {
- goto invalid;
+ return;
}
}
}
- return true;
+ return;
invalid:
- return false;
+ *nd_target = NULL;
+ memset(arp_buf[0], 0, ETH_ADDR_LEN);
+ memset(arp_buf[1], 0, ETH_ADDR_LEN);
+ return;
}
/* Initializes 'flow' members from 'packet' and 'md'
@@ -764,18 +767,16 @@ miniflow_extract(struct dp_packet *packet, struct
miniflow *dst)
const struct icmp6_hdr *icmp = data_pull(&data, &size,
sizeof *icmp);
memset(arp_buf, 0, sizeof arp_buf);
- if (OVS_LIKELY(parse_icmpv6(&data, &size, icmp, &nd_target,
- arp_buf))) {
- if (nd_target) {
- miniflow_push_words(mf, nd_target, nd_target,
- sizeof *nd_target / 8);
- }
- miniflow_push_macs(mf, arp_sha, arp_buf);
- miniflow_pad_to_64(mf, tcp_flags);
- miniflow_push_be16(mf, tp_src, htons(icmp->icmp6_type));
- miniflow_push_be16(mf, tp_dst, htons(icmp->icmp6_code));
- miniflow_pad_to_64(mf, igmp_group_ip4);
+ parse_icmpv6(&data, &size, icmp, &nd_target, arp_buf);
+ if (nd_target) {
+ miniflow_push_words(mf, nd_target, nd_target,
+ sizeof *nd_target / 8);
}
+ miniflow_push_macs(mf, arp_sha, arp_buf);
+ miniflow_pad_to_64(mf, tcp_flags);
+ miniflow_push_be16(mf, tp_src, htons(icmp->icmp6_type));
+ miniflow_push_be16(mf, tp_dst, htons(icmp->icmp6_code));
+ miniflow_pad_to_64(mf, igmp_group_ip4);
}
}
}
--
2.1.4
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
