| Message ID | 20181011194433.20714-1-blp@ovn.org |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [ovs-dev] expr: Disallow < <= >= > comparisons against empty value set. | expand |
Looks good to me, thanks. Reviewed-by: Yifeng Sun <pkusunyifeng@gmail.com> On Thu, Oct 11, 2018 at 12:44 PM Ben Pfaff <blp@ovn.org> wrote: > OVN expression syntax does not allow a literal empty value set, like {}. > Rather, any literal value set has to have at least one value. However, > value sets that originate from address sets or from port groups can be > empty. In such a case, == and != comparisons are allowed but < <= >= > > should be errors. The actual implementation failed to properly disallow > the latter and instead tried to access the first element of the value set, > a bad read. This fixes the problem. > > Reported-at: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=10731 > Reported-at > <https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=10731Reported-at>: > https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=10767 > Signed-off-by > <https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=10767Signed-off-by>: > Ben Pfaff <blp@ovn.org> > --- > ovn/lib/expr.c | 5 +++++ > tests/ovn.at | 2 ++ > 2 files changed, 7 insertions(+) > > diff --git a/ovn/lib/expr.c b/ovn/lib/expr.c > index 148ac869e861..16dd1776b543 100644 > --- a/ovn/lib/expr.c > +++ b/ovn/lib/expr.c > @@ -578,6 +578,11 @@ make_cmp(struct expr_context *ctx, > f->symbol->name); > goto exit; > } > + if (!cs->n_values) { > + lexer_error(ctx->lexer, "Only == and != operators may be used > " > + "to compare a field against an empty value set."); > + goto exit; > + } > if (cs->values[0].masked) { > lexer_error(ctx->lexer, "Only == and != operators may be used > " > "with masked constants. Consider using subfields > " > diff --git a/tests/ovn.at b/tests/ovn.at > index 44475175d20a..94676a9aa802 100644 > --- a/tests/ovn.at > +++ b/tests/ovn.at > @@ -351,6 +351,8 @@ eth.dst[40] x => Syntax error at `x' expecting end of > input. > > ip4.src == {1.2.3.4, $set1, $unknownset} => Syntax error at `$unknownset' > expecting address set name. > eth.src == {$set3, badmac, 00:00:00:00:00:01} => Syntax error at `badmac' > expecting constant. > + > +ct_label > $set4 => Only == and != operators may be used to compare a > field against an empty value set. > ]]) > sed 's/ =>.*//' test-cases.txt > input.txt > sed 's/.* => //' test-cases.txt > expout > -- > 2.16.1 > > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev >
Thanks, applied and backported. On Thu, Oct 11, 2018 at 01:08:43PM -0700, Yifeng Sun wrote: > Looks good to me, thanks. > > Reviewed-by: Yifeng Sun <pkusunyifeng@gmail.com> > > On Thu, Oct 11, 2018 at 12:44 PM Ben Pfaff <blp@ovn.org> wrote: > > > OVN expression syntax does not allow a literal empty value set, like {}. > > Rather, any literal value set has to have at least one value. However, > > value sets that originate from address sets or from port groups can be > > empty. In such a case, == and != comparisons are allowed but < <= >= > > > should be errors. The actual implementation failed to properly disallow > > the latter and instead tried to access the first element of the value set, > > a bad read. This fixes the problem. > > > > Reported-at: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=10731 > > Reported-at > > <https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=10731Reported-at>: > > https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=10767 > > Signed-off-by > > <https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=10767Signed-off-by>: > > Ben Pfaff <blp@ovn.org> > > --- > > ovn/lib/expr.c | 5 +++++ > > tests/ovn.at | 2 ++ > > 2 files changed, 7 insertions(+) > > > > diff --git a/ovn/lib/expr.c b/ovn/lib/expr.c > > index 148ac869e861..16dd1776b543 100644 > > --- a/ovn/lib/expr.c > > +++ b/ovn/lib/expr.c > > @@ -578,6 +578,11 @@ make_cmp(struct expr_context *ctx, > > f->symbol->name); > > goto exit; > > } > > + if (!cs->n_values) { > > + lexer_error(ctx->lexer, "Only == and != operators may be used > > " > > + "to compare a field against an empty value set."); > > + goto exit; > > + } > > if (cs->values[0].masked) { > > lexer_error(ctx->lexer, "Only == and != operators may be used > > " > > "with masked constants. Consider using subfields > > " > > diff --git a/tests/ovn.at b/tests/ovn.at > > index 44475175d20a..94676a9aa802 100644 > > --- a/tests/ovn.at > > +++ b/tests/ovn.at > > @@ -351,6 +351,8 @@ eth.dst[40] x => Syntax error at `x' expecting end of > > input. > > > > ip4.src == {1.2.3.4, $set1, $unknownset} => Syntax error at `$unknownset' > > expecting address set name. > > eth.src == {$set3, badmac, 00:00:00:00:00:01} => Syntax error at `badmac' > > expecting constant. > > + > > +ct_label > $set4 => Only == and != operators may be used to compare a > > field against an empty value set. > > ]]) > > sed 's/ =>.*//' test-cases.txt > input.txt > > sed 's/.* => //' test-cases.txt > expout > > -- > > 2.16.1 > > > > _______________________________________________ > > dev mailing list > > dev@openvswitch.org > > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > >
diff --git a/ovn/lib/expr.c b/ovn/lib/expr.c index 148ac869e861..16dd1776b543 100644 --- a/ovn/lib/expr.c +++ b/ovn/lib/expr.c @@ -578,6 +578,11 @@ make_cmp(struct expr_context *ctx, f->symbol->name); goto exit; } + if (!cs->n_values) { + lexer_error(ctx->lexer, "Only == and != operators may be used " + "to compare a field against an empty value set."); + goto exit; + } if (cs->values[0].masked) { lexer_error(ctx->lexer, "Only == and != operators may be used " "with masked constants. Consider using subfields " diff --git a/tests/ovn.at b/tests/ovn.at index 44475175d20a..94676a9aa802 100644 --- a/tests/ovn.at +++ b/tests/ovn.at @@ -351,6 +351,8 @@ eth.dst[40] x => Syntax error at `x' expecting end of input. ip4.src == {1.2.3.4, $set1, $unknownset} => Syntax error at `$unknownset' expecting address set name. eth.src == {$set3, badmac, 00:00:00:00:00:01} => Syntax error at `badmac' expecting constant. + +ct_label > $set4 => Only == and != operators may be used to compare a field against an empty value set. ]]) sed 's/ =>.*//' test-cases.txt > input.txt sed 's/.* => //' test-cases.txt > expout
OVN expression syntax does not allow a literal empty value set, like {}. Rather, any literal value set has to have at least one value. However, value sets that originate from address sets or from port groups can be empty. In such a case, == and != comparisons are allowed but < <= >= > should be errors. The actual implementation failed to properly disallow the latter and instead tried to access the first element of the value set, a bad read. This fixes the problem. Reported-at: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=10731 Reported-at: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=10767 Signed-off-by: Ben Pfaff <blp@ovn.org> --- ovn/lib/expr.c | 5 +++++ tests/ovn.at | 2 ++ 2 files changed, 7 insertions(+)