miniflow_extract: Properly handle small IP packets.

Ethernet frames may contain padding after the IP payload.  When
parsing IP packets, check the IP total size (IPv4) or IP payload size
(IPv6) to detect the size of l2 padding.  The l2 padding size is
stored in the ofpbuf to prevent ofpbuf_pull from entering the padding,
as well as to allow ofpbuf_l4_size() to return the size of the IP
payload without the l2 padding.

This helps avoiding parsing truncated transport headers, for example.

Signed-off-by: Jarno Rajahalme <jrajahalme@nicira.com>
Acked-by: Ben Pfaff <blp@nicira.com>

4 files changed, 53 insertions, 11 deletions

diff --git a/lib/flow.c b/lib/flow.c
index a81dca414..7f28b4d43 100644
--- a/lib/flow.c
+++ b/lib/flow.c
@@ -423,6 +423,7 @@ miniflow_extract(struct ofpbuf *packet, const struct pkt_metadata *md,
     if (OVS_LIKELY(dl_type == htons(ETH_TYPE_IP))) {
         const struct ip_header *nh = data;
         int ip_len;
+        uint16_t tot_len;
 
         if (OVS_UNLIKELY(size < IP_HEADER_LEN)) {
             goto out;
@@ -432,6 +433,18 @@ miniflow_extract(struct ofpbuf *packet, const struct pkt_metadata *md,
         if (OVS_UNLIKELY(ip_len < IP_HEADER_LEN)) {
             goto out;
         }
+        if (OVS_UNLIKELY(size < ip_len)) {
+            goto out;
+        }
+        tot_len = ntohs(nh->ip_tot_len);
+        if (OVS_UNLIKELY(tot_len > size)) {
+            goto out;
+        }
+        if (OVS_UNLIKELY(size - tot_len > UINT8_MAX)) {
+            goto out;
+        }
+        ofpbuf_set_l2_pad_size(packet, size - tot_len);
+        size = tot_len;   /* Never pull padding. */
 
         /* Push both source and destination address at once. */
         miniflow_push_words(mf, nw_src, &nh->ip_src, 2);
@@ -445,20 +458,28 @@ miniflow_extract(struct ofpbuf *packet, const struct pkt_metadata *md,
                 nw_frag |= FLOW_NW_FRAG_LATER;
             }
         }
-        if (OVS_UNLIKELY(size < ip_len)) {
-            goto out;
-        }
         data_pull(&data, &size, ip_len);
-
     } else if (dl_type == htons(ETH_TYPE_IPV6)) {
         const struct ovs_16aligned_ip6_hdr *nh;
         ovs_be32 tc_flow;
+        uint16_t plen;
 
         if (OVS_UNLIKELY(size < sizeof *nh)) {
             goto out;
         }
         nh = data_pull(&data, &size, sizeof *nh);
 
+        plen = ntohs(nh->ip6_plen);
+        if (OVS_UNLIKELY(plen > size)) {
+            goto out;
+        }
+        /* Jumbo Payload option not supported yet. */
+        if (OVS_UNLIKELY(size - plen > UINT8_MAX)) {
+            goto out;
+        }
+        ofpbuf_set_l2_pad_size(packet, size - plen);
+        size = plen;   /* Never pull padding. */
+
         miniflow_push_words(mf, ipv6_src, &nh->ip6_src,
                             sizeof nh->ip6_src / 4);
         miniflow_push_words(mf, ipv6_dst, &nh->ip6_dst,
diff --git a/lib/ofpbuf.c b/lib/ofpbuf.c
index 7ccc92f27..1d3cd0330 100644
--- a/lib/ofpbuf.c
+++ b/lib/ofpbuf.c
@@ -28,6 +28,7 @@ ofpbuf_init__(struct ofpbuf *b, size_t allocated, enum ofpbuf_source source)
     b->allocated = allocated;
     b->source = source;
     b->frame = NULL;
+    b->l2_pad_size = 0;
     b->l2_5_ofs = b->l3_ofs = b->l4_ofs = UINT16_MAX;
     list_poison(&b->list_node);
 }
@@ -199,6 +200,7 @@ ofpbuf_clone_with_headroom(const struct ofpbuf *buffer, size_t headroom)
 
         new_buffer->frame = (char *) buffer->frame + data_delta;
     }
+    new_buffer->l2_pad_size = buffer->l2_pad_size;
     new_buffer->l2_5_ofs = buffer->l2_5_ofs;
     new_buffer->l3_ofs = buffer->l3_ofs;
     new_buffer->l4_ofs = buffer->l4_ofs;
diff --git a/lib/ofpbuf.h b/lib/ofpbuf.h
index 53c43fb16..4b924214f 100644
--- a/lib/ofpbuf.h
+++ b/lib/ofpbuf.h
@@ -67,13 +67,15 @@ struct ofpbuf {
     uint32_t allocated;         /* Number of bytes allocated. */
 
     void *frame;                /* Packet frame start, or NULL. */
+    enum ofpbuf_source source;  /* Source of memory allocated as 'base'. */
+    uint8_t l2_pad_size;        /* Detected l2 padding size.
+                                 * Padding is non-pullable. */
     uint16_t l2_5_ofs;          /* MPLS label stack offset from 'frame', or
                                  * UINT16_MAX */
     uint16_t l3_ofs;            /* Network-level header offset from 'frame',
                                    or UINT16_MAX. */
     uint16_t l4_ofs;            /* Transport-level header offset from 'frame',
                                    or UINT16_MAX. */
-    enum ofpbuf_source source;  /* Source of memory allocated as 'base'. */
     struct list list_node;      /* Private list element for use by owner. */
 };
 
@@ -89,6 +91,8 @@ void * ofpbuf_resize_l2(struct ofpbuf *, int increment);
 void * ofpbuf_resize_l2_5(struct ofpbuf *, int increment);
 static inline void * ofpbuf_l2(const struct ofpbuf *);
 static inline void ofpbuf_set_frame(struct ofpbuf *, void *);
+static inline uint8_t ofpbuf_l2_pad_size(const struct ofpbuf *);
+static inline void ofpbuf_set_l2_pad_size(struct ofpbuf *, uint8_t);
 static inline void * ofpbuf_l2_5(const struct ofpbuf *);
 static inline void ofpbuf_set_l2_5(struct ofpbuf *, void *);
 static inline void * ofpbuf_l3(const struct ofpbuf *);
@@ -244,7 +248,7 @@ static inline void ofpbuf_clear(struct ofpbuf *b)
 static inline void *ofpbuf_pull(struct ofpbuf *b, size_t size)
 {
     void *data = ofpbuf_data(b);
-    ovs_assert(ofpbuf_size(b) >= size);
+    ovs_assert(ofpbuf_size(b) - ofpbuf_l2_pad_size(b) >= size);
     ofpbuf_set_data(b, (char*)ofpbuf_data(b) + size);
     ofpbuf_set_size(b, ofpbuf_size(b) - size);
     return data;
@@ -255,7 +259,8 @@ static inline void *ofpbuf_pull(struct ofpbuf *b, size_t size)
  * null pointer without modifying 'b'. */
 static inline void *ofpbuf_try_pull(struct ofpbuf *b, size_t size)
 {
-    return ofpbuf_size(b) >= size ? ofpbuf_pull(b, size) : NULL;
+    return ofpbuf_size(b) - ofpbuf_l2_pad_size(b) >= size
+        ? ofpbuf_pull(b, size) : NULL;
 }
 
 static inline struct ofpbuf *ofpbuf_from_list(const struct list *list)
@@ -281,11 +286,23 @@ static inline void * ofpbuf_l2(const struct ofpbuf *b)
 static inline void ofpbuf_set_frame(struct ofpbuf *b, void *packet)
 {
     b->frame = packet;
+    b->l2_pad_size = 0;
     b->l2_5_ofs = UINT16_MAX;
     b->l3_ofs = UINT16_MAX;
     b->l4_ofs = UINT16_MAX;
 }
 
+static inline uint8_t ofpbuf_l2_pad_size(const struct ofpbuf *b)
+{
+    return b->l2_pad_size;
+}
+
+static inline void ofpbuf_set_l2_pad_size(struct ofpbuf *b, uint8_t pad_size)
+{
+    ovs_assert(pad_size <= ofpbuf_size(b));
+    b->l2_pad_size = pad_size;
+}
+
 static inline void * ofpbuf_l2_5(const struct ofpbuf *b)
 {
     return b->l2_5_ofs != UINT16_MAX ? (char *)b->frame + b->l2_5_ofs : NULL;
@@ -319,7 +336,9 @@ static inline void ofpbuf_set_l4(struct ofpbuf *b, void *l4)
 static inline size_t ofpbuf_l4_size(const struct ofpbuf *b)
 {
     return b->l4_ofs != UINT16_MAX
-        ? (const char *)ofpbuf_tail(b) - (const char *)ofpbuf_l4(b) : 0;
+        ? (const char *)ofpbuf_tail(b) - (const char *)ofpbuf_l4(b)
+        - ofpbuf_l2_pad_size(b)
+        : 0;
 }
 
 static inline const void *ofpbuf_get_tcp_payload(const struct ofpbuf *b)
diff --git a/tests/ofproto.at b/tests/ofproto.at
index def078360..475e4781f 100644
--- a/tests/ofproto.at
+++ b/tests/ofproto.at
@@ -1748,7 +1748,7 @@ in_port=0,vlan_tci=0x0000,dl_src=00:10:20:30:40:50,dl_dst=00:01:02:03:04:05,dl_t
     fi
 
     # OFPT_PACKET_IN, OFPR_INVALID_TTL (controller_id=0)
-    ovs-ofctl packet-out br0 controller dec_ttl '002583dfb4000026b98cb0f908004500003fb7e200000011339bac11370dac100002d7730035002b8f6d86fb0100000100000000000006626c702d7873066e696369726103636f6d00000f00'
+    ovs-ofctl packet-out br0 controller dec_ttl '002583dfb4000026b98cb0f908004500003eb7e200000011339bac11370dac100002d7730035002b8f6d86fb0100000100000000000006626c702d7873066e696369726103636f6d00000f00'
     if test X"$1" = X"OFPR_INVALID_TTL"; then shift;
         echo >>expout "OFPT_PACKET_IN: total_len=76 in_port=CONTROLLER (via invalid_ttl) data_len=76 (unbuffered)
 udp,in_port=0,vlan_tci=0x0000,dl_src=00:26:b9:8c:b0:f9,dl_dst=00:25:83:df:b4:00,nw_src=172.17.55.13,nw_dst=172.16.0.2,nw_tos=0,nw_ecn=0,nw_ttl=0,tp_src=55155,tp_dst=53 udp_csum:8f6d"
@@ -1852,7 +1852,7 @@ in_port=0,vlan_tci=0x0000,dl_src=00:10:20:30:40:50,dl_dst=00:01:02:03:04:05,dl_t
     fi
 
     # OFPT_PACKET_IN, OFPR_INVALID_TTL (controller_id=0)
-    ovs-ofctl -O OpenFlow12 packet-out br0 none dec_ttl '002583dfb4000026b98cb0f908004500003fb7e200000011339bac11370dac100002d7730035002b8f6d86fb0100000100000000000006626c702d7873066e696369726103636f6d00000f00'
+    ovs-ofctl -O OpenFlow12 packet-out br0 none dec_ttl '002583dfb4000026b98cb0f908004500003eb7e200000011339bac11370dac100002d7730035002b8f6d86fb0100000100000000000006626c702d7873066e696369726103636f6d00000f00'
     if test X"$1" = X"OFPR_INVALID_TTL"; then shift;
         echo >>expout "OFPT_PACKET_IN (OF1.2): total_len=76 in_port=ANY (via invalid_ttl) data_len=76 (unbuffered)
 udp,in_port=0,vlan_tci=0x0000,dl_src=00:26:b9:8c:b0:f9,dl_dst=00:25:83:df:b4:00,nw_src=172.17.55.13,nw_dst=172.16.0.2,nw_tos=0,nw_ecn=0,nw_ttl=0,tp_src=55155,tp_dst=53 udp_csum:8f6d"
@@ -1956,7 +1956,7 @@ in_port=0,vlan_tci=0x0000,dl_src=00:10:20:30:40:50,dl_dst=00:01:02:03:04:05,dl_t
     fi
 
     # OFPT_PACKET_IN, OFPR_INVALID_TTL (controller_id=0)
-    ovs-ofctl -O OpenFlow13 packet-out br0 none dec_ttl '002583dfb4000026b98cb0f908004500003fb7e200000011339bac11370dac100002d7730035002b8f6d86fb0100000100000000000006626c702d7873066e696369726103636f6d00000f00'
+    ovs-ofctl -O OpenFlow13 packet-out br0 none dec_ttl '002583dfb4000026b98cb0f908004500003eb7e200000011339bac11370dac100002d7730035002b8f6d86fb0100000100000000000006626c702d7873066e696369726103636f6d00000f00'
     if test X"$1" = X"OFPR_INVALID_TTL"; then shift;
         echo >>expout "OFPT_PACKET_IN (OF1.3): total_len=76 in_port=ANY (via invalid_ttl) data_len=76 (unbuffered)
 udp,in_port=0,vlan_tci=0x0000,dl_src=00:26:b9:8c:b0:f9,dl_dst=00:25:83:df:b4:00,nw_src=172.17.55.13,nw_dst=172.16.0.2,nw_tos=0,nw_ecn=0,nw_ttl=0,tp_src=55155,tp_dst=53 udp_csum:8f6d"