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

Yifeng Sun pkusunyifeng at gmail.com
Thu Oct 25 21:24:37 UTC 2018


Sorry there is a compiling warning. I will fix it and submit v2.

On Thu, Oct 25, 2018 at 12:38 PM Yifeng Sun <pkusunyifeng at gmail.com> wrote:

> 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
> <https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=10800Signed-off-by>:
> Yifeng Sun <pkusunyifeng at gmail.com>
> ---
>  lib/odp-util.c | 63
> +++++++++++++++++++++++++++++++++++++++++++++++-----------
>  1 file changed, 51 insertions(+), 12 deletions(-)
>
> diff --git a/lib/odp-util.c b/lib/odp-util.c
> index 1a52f3bc5ad9..019fba413a66 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;
> --
> 2.7.4
>
>


More information about the dev mailing list