[ovs-dev] [PATCH v2] odp-util: Properly handle the return values of scan_XXX functions

Yifeng Sun pkusunyifeng at gmail.com
Thu Oct 25 21:49:14 UTC 2018


Functions like scan_u8, return 0 when they failed to scan the expected
values. Function scan_geneve failed to check this situation. This leads
to using of uninitialized value of opt_len_mask. This patch fixes it
and further inspects and fixes all the problematic places where
the return values of scan_XXX functions are not properly handled.

Reported-at: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=10800
Signed-off-by: Yifeng Sun <pkusunyifeng at gmail.com>
---
v1->v2: Fix a compiler warning,
        ovs-dev-bounces complains [ovs-dev] in the subject, remove it.

 lib/odp-util.c | 65 ++++++++++++++++++++++++++++++++++++++++++++++------------
 1 file changed, 52 insertions(+), 13 deletions(-)

diff --git a/lib/odp-util.c b/lib/odp-util.c
index 1a52f3bc5ad9..626a03b7686f 100644
--- a/lib/odp-util.c
+++ b/lib/odp-util.c
@@ -1915,8 +1915,8 @@ find_end:
 
                     s += n;
                     retval = scan_u128(s, &ct_label.value, &ct_label.mask);
-                    if (retval < 0) {
-                        return retval;
+                    if (retval == 0) {
+                        return -EINVAL;
                     }
                     s += retval;
                     continue;
@@ -4806,10 +4806,15 @@ scan_vxlan_gbp(const char *s, uint32_t *key, uint32_t *mask)
     const char *s_base = s;
     ovs_be16 id = 0, id_mask = 0;
     uint8_t flags = 0, flags_mask = 0;
+    int len;
 
     if (!strncmp(s, "id=", 3)) {
         s += 3;
-        s += scan_be16(s, &id, mask ? &id_mask : NULL);
+        len = scan_be16(s, &id, mask ? &id_mask : NULL);
+        if (len == 0) {
+            return -EINVAL;
+        }
+        s += len;
     }
 
     if (s[0] == ',') {
@@ -4817,7 +4822,11 @@ scan_vxlan_gbp(const char *s, uint32_t *key, uint32_t *mask)
     }
     if (!strncmp(s, "flags=", 6)) {
         s += 6;
-        s += scan_u8(s, &flags, mask ? &flags_mask : NULL);
+        len = scan_u8(s, &flags, mask ? &flags_mask : NULL);
+        if (len == 0) {
+            return -EINVAL;
+        }
+        s += len;
     }
 
     if (!strncmp(s, "))", 2)) {
@@ -4843,10 +4852,15 @@ scan_erspan_metadata(const char *s,
     uint32_t idx = 0, idx_mask = 0;
     uint8_t ver = 0, dir = 0, hwid = 0;
     uint8_t ver_mask = 0, dir_mask = 0, hwid_mask = 0;
+    int len;
 
     if (!strncmp(s, "ver=", 4)) {
         s += 4;
-        s += scan_u8(s, &ver, mask ? &ver_mask : NULL);
+        len = scan_u8(s, &ver, mask ? &ver_mask : NULL);
+        if (len == 0) {
+            return -EINVAL;
+        }
+        s += len;
     }
 
     if (s[0] == ',') {
@@ -4856,7 +4870,11 @@ scan_erspan_metadata(const char *s,
     if (ver == 1) {
         if (!strncmp(s, "idx=", 4)) {
             s += 4;
-            s += scan_u32(s, &idx, mask ? &idx_mask : NULL);
+            len = scan_u32(s, &idx, mask ? &idx_mask : NULL);
+            if (len == 0) {
+                return -EINVAL;
+            }
+            s += len;
         }
 
         if (!strncmp(s, ")", 1)) {
@@ -4872,14 +4890,22 @@ scan_erspan_metadata(const char *s,
     } else if (ver == 2) {
         if (!strncmp(s, "dir=", 4)) {
             s += 4;
-            s += scan_u8(s, &dir, mask ? &dir_mask : NULL);
+            len = scan_u8(s, &dir, mask ? &dir_mask : NULL);
+            if (len == 0) {
+                return -EINVAL;
+            }
+            s += len;
         }
         if (s[0] == ',') {
             s++;
         }
         if (!strncmp(s, "hwid=", 5)) {
             s += 5;
-            s += scan_u8(s, &hwid, mask ? &hwid_mask : NULL);
+            len = scan_u8(s, &hwid, mask ? &hwid_mask : NULL);
+            if (len == 0) {
+                return -EINVAL;
+            }
+            s += len;
         }
 
         if (!strncmp(s, ")", 1)) {
@@ -4905,6 +4931,7 @@ scan_geneve(const char *s, struct geneve_scan *key, struct geneve_scan *mask)
     struct geneve_opt *opt = key->d;
     struct geneve_opt *opt_mask = mask ? mask->d : NULL;
     int len_remain = sizeof key->d;
+    int len;
 
     while (s[0] == '{' && len_remain >= sizeof *opt) {
         int data_len = 0;
@@ -4914,8 +4941,12 @@ scan_geneve(const char *s, struct geneve_scan *key, struct geneve_scan *mask)
 
         if (!strncmp(s, "class=", 6)) {
             s += 6;
-            s += scan_be16(s, &opt->opt_class,
-                           mask ? &opt_mask->opt_class : NULL);
+            len = scan_be16(s, &opt->opt_class,
+                            mask ? &opt_mask->opt_class : NULL);
+            if (len == 0) {
+                return -EINVAL;
+            }
+            s += len;
         } else if (mask) {
             memset(&opt_mask->opt_class, 0, sizeof opt_mask->opt_class);
         }
@@ -4925,7 +4956,11 @@ scan_geneve(const char *s, struct geneve_scan *key, struct geneve_scan *mask)
         }
         if (!strncmp(s, "type=", 5)) {
             s += 5;
-            s += scan_u8(s, &opt->type, mask ? &opt_mask->type : NULL);
+            len = scan_u8(s, &opt->type, mask ? &opt_mask->type : NULL);
+            if (len == 0) {
+                return -EINVAL;
+            }
+            s += len;
         } else if (mask) {
             memset(&opt_mask->type, 0, sizeof opt_mask->type);
         }
@@ -4936,7 +4971,11 @@ scan_geneve(const char *s, struct geneve_scan *key, struct geneve_scan *mask)
         if (!strncmp(s, "len=", 4)) {
             uint8_t opt_len, opt_len_mask;
             s += 4;
-            s += scan_u8(s, &opt_len, mask ? &opt_len_mask : NULL);
+            len = scan_u8(s, &opt_len, mask ? &opt_len_mask : NULL);
+            if (len == 0) {
+                return -EINVAL;
+            }
+            s += len;
 
             if (opt_len > 124 || opt_len % 4 || opt_len > len_remain) {
                 return 0;
@@ -4981,7 +5020,7 @@ scan_geneve(const char *s, struct geneve_scan *key, struct geneve_scan *mask)
     }
 
     if (s[0] == ')') {
-        int len = sizeof key->d - len_remain;
+        len = sizeof key->d - len_remain;
 
         s++;
         key->len = len;
-- 
2.7.4



More information about the dev mailing list