flow: Ignore invalid ICMPv6 fields when parsing packets

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.horman@netronome.com>
Signed-off-by: Jesse Gross <jesse@nicira.com>

diff --git a/lib/flow.c b/lib/flow.c
index 3e99d5e..5df23a9 100644 (file)
--- a/lib/flow.c
+++ b/lib/flow.c
@@ -334,7 +334,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])
@@ -345,7 +345,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) {
@@ -355,7 +355,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
@@ -378,15 +378,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'
@@ -715,18 +718,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);
             }
         }
     }