Message ID | 1540496281-11513-1-git-send-email-pkusunyifeng@gmail.com |
---|---|
State | Superseded |
Headers | show |
Series | [ovs-dev] odp-util: Properly handle the return values of scan_XXX functions | expand |
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@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@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 > >
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;
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@gmail.com> --- lib/odp-util.c | 63 +++++++++++++++++++++++++++++++++++++++++++++++----------- 1 file changed, 51 insertions(+), 12 deletions(-)