[ovs-dev] odp-util: Set a limit for nested parse_odp_key_mask_attr call

Message ID 1541623337-16025-3-git-send-email-pkusunyifeng@gmail.com
State Accepted
Headers show
Series
  • [ovs-dev] odp-util: Set a limit for nested parse_odp_key_mask_attr call
Related show

Commit Message

Yifeng Sun Nov. 7, 2018, 8:42 p.m.
This patch puts a limit on the nested depth in flow key string to avoid
stackoverflow. An example to show this issue is a key string contains
thousands of nested encaps. In addition, a new test is added for this fix.

Reported-at: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=11149
Signed-off-by: Yifeng Sun <pkusunyifeng@gmail.com>
---
 lib/odp-util.c | 36 +++++++++++++++++++++++++++++-------
 tests/odp.at   |  9 +++++++++
 2 files changed, 38 insertions(+), 7 deletions(-)

Patch

diff --git a/lib/odp-util.c b/lib/odp-util.c
index f50de7fd275c..627baaa397ed 100644
--- a/lib/odp-util.c
+++ b/lib/odp-util.c
@@ -57,8 +57,15 @@  VLOG_DEFINE_THIS_MODULE(odp_util);
 static const char *delimiters = ", \t\r\n";
 static const char *delimiters_end = ", \t\r\n)";
 
-static int parse_odp_key_mask_attr(const char *, const struct simap *port_names,
-                              struct ofpbuf *, struct ofpbuf *);
+#define MAX_ODP_NESTED 32
+
+struct parse_odp_context {
+    const struct simap *port_names;
+    int depth; /* Current nested depth of odp string. */
+};
+
+static int parse_odp_key_mask_attr(struct parse_odp_context *, const char *,
+                                   struct ofpbuf *, struct ofpbuf *);
 static void format_odp_key_attr(const struct nlattr *a,
                                 const struct nlattr *ma,
                                 const struct hmap *portno_names, struct ds *ds,
@@ -2216,9 +2223,12 @@  parse_odp_action(const char *s, const struct simap *port_names,
         struct ofpbuf maskbuf = OFPBUF_STUB_INITIALIZER(mask);
         struct nlattr *nested, *key;
         size_t size;
+        struct parse_odp_context context = (struct parse_odp_context) {
+            .port_names = port_names,
+        };
 
         start_ofs = nl_msg_start_nested(actions, OVS_ACTION_ATTR_SET);
-        retval = parse_odp_key_mask_attr(s + 4, port_names, actions, &maskbuf);
+        retval = parse_odp_key_mask_attr(&context, s + 4, actions, &maskbuf);
         if (retval < 0) {
             ofpbuf_uninit(&maskbuf);
             return retval;
@@ -5268,7 +5278,8 @@  erspan_to_attr(struct ofpbuf *a, const void *data_)
 /* scan_port needs one extra argument. */
 #define SCAN_SINGLE_PORT(NAME, TYPE, ATTR)  \
     SCAN_BEGIN(NAME, TYPE) {                            \
-        len = scan_port(s, &skey, &smask, port_names);  \
+        len = scan_port(s, &skey, &smask,               \
+                        context->port_names);           \
         if (len == 0) {                                 \
             return -EINVAL;                             \
         }                                               \
@@ -5404,7 +5415,7 @@  parse_odp_nsh_key_mask_attr(const char *s, struct ofpbuf *key,
 }
 
 static int
-parse_odp_key_mask_attr(const char *s, const struct simap *port_names,
+parse_odp_key_mask_attr(struct parse_odp_context *context, const char *s,
                         struct ofpbuf *key, struct ofpbuf *mask)
 {
     SCAN_SINGLE("skb_priority(", uint32_t, u32, OVS_KEY_ATTR_PRIORITY);
@@ -5557,6 +5568,11 @@  parse_odp_key_mask_attr(const char *s, const struct simap *port_names,
         const char *start = s;
         size_t encap, encap_mask = 0;
 
+        if (context->depth + 1 == MAX_ODP_NESTED) {
+            return -EINVAL;
+        }
+        context->depth++;
+
         encap = nl_msg_start_nested(key, OVS_KEY_ATTR_ENCAP);
         if (mask) {
             encap_mask = nl_msg_start_nested(mask, OVS_KEY_ATTR_ENCAP);
@@ -5568,13 +5584,15 @@  parse_odp_key_mask_attr(const char *s, const struct simap *port_names,
 
             s += strspn(s, delimiters);
             if (!*s) {
+                context->depth--;
                 return -EINVAL;
             } else if (*s == ')') {
                 break;
             }
 
-            retval = parse_odp_key_mask_attr(s, port_names, key, mask);
+            retval = parse_odp_key_mask_attr(context, s, key, mask);
             if (retval < 0) {
+                context->depth--;
                 return retval;
             }
             s += retval;
@@ -5585,6 +5603,7 @@  parse_odp_key_mask_attr(const char *s, const struct simap *port_names,
         if (mask) {
             nl_msg_end_nested(mask, encap_mask);
         }
+        context->depth--;
 
         return s - start;
     }
@@ -5611,6 +5630,9 @@  odp_flow_from_string(const char *s, const struct simap *port_names,
                      struct ofpbuf *key, struct ofpbuf *mask)
 {
     const size_t old_size = key->size;
+    struct parse_odp_context context = (struct parse_odp_context) {
+        .port_names = port_names,
+    };
     for (;;) {
         int retval;
 
@@ -5630,7 +5652,7 @@  odp_flow_from_string(const char *s, const struct simap *port_names,
             s += s[0] == ' ' ? 1 : 0;
         }
 
-        retval = parse_odp_key_mask_attr(s, port_names, key, mask);
+        retval = parse_odp_key_mask_attr(&context, s, key, mask);
         if (retval < 0) {
             key->size = old_size;
             return -retval;
diff --git a/tests/odp.at b/tests/odp.at
index aad89e80aee2..1cff727aead6 100644
--- a/tests/odp.at
+++ b/tests/odp.at
@@ -394,3 +394,12 @@  AT_CHECK([echo 'encap_nsh@:{@' | ovstest test-odp parse-actions
 odp_actions_from_string: error
 ])
 AT_CLEANUP
+
+AT_SETUP([OVS datapath keys parsing and formatting - 33 nested encap ])
+AT_DATA([odp-in.txt], [dnl
+encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap()))))))))))))))))))))))))))))))))
+])
+AT_CHECK_UNQUOTED([ovstest test-odp parse-keys < odp-in.txt], [0], [dnl
+odp_flow_from_string: error
+])
+AT_CLEANUP