[ovs-dev,v3] expr: Access expr_constant.mask only when its type is EXPR_C_INTEGER

Message ID 1539212576-13027-1-git-send-email-pkusunyifeng@gmail.com
State Superseded
Headers show
Series
  • [ovs-dev,v3] expr: Access expr_constant.mask only when its type is EXPR_C_INTEGER
Related show

Commit Message

Yifeng Sun Oct. 10, 2018, 11:02 p.m.
It is unsafe to access expr_constant.masked when its type
is EXPR_C_STRING as its value is uninitialized. This patch
fixes this issue.

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: Yifeng Sun <pkusunyifeng@gmail.com>
---
v1->v2: Fix email subject by adding [ovs-dev]
v2->v3: Inspect through code to make sure expr_constant is accessed correctly
by its type, thanks Ben for the review!

 ovn/lib/expr.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Ben Pfaff Oct. 11, 2018, 7:48 p.m. | #1
On Wed, Oct 10, 2018 at 04:02:56PM -0700, Yifeng Sun wrote:
> It is unsafe to access expr_constant.masked when its type
> is EXPR_C_STRING as its value is uninitialized. This patch
> fixes this issue.
> 
> 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: Yifeng Sun <pkusunyifeng@gmail.com>
> ---
> v1->v2: Fix email subject by adding [ovs-dev]
> v2->v3: Inspect through code to make sure expr_constant is accessed correctly
> by its type, thanks Ben for the review!

Thanks for the fix.

There was something odd about this, which was that if the field is a
string then it would be EXPR_L_NOMINAL, and so the problem should have
been caught in the test for nominal variables.  Also, the triggering
test case was "ct_label > $set4", and ct_label is not a string field.
Looking closer, $set4 is an empty set, like {}.  That means that the
real underlying problem was that the code was not properly disallowing
empty sets.  So, I sent the following patch:
        https://patchwork.ozlabs.org/patch/982674/

Thanks,

Ben.

Patch

diff --git a/ovn/lib/expr.c b/ovn/lib/expr.c
index 5880fd2e7289..0fbe109783da 100644
--- a/ovn/lib/expr.c
+++ b/ovn/lib/expr.c
@@ -581,7 +581,7 @@  make_cmp(struct expr_context *ctx,
                         f->symbol->name);
             goto exit;
         }
-        if (cs->values[0].masked) {
+        if (cs->type == EXPR_C_INTEGER && cs->values[0].masked) {
             lexer_error(ctx->lexer, "Only == and != operators may be used "
                         "with masked constants.  Consider using subfields "
                         "instead (e.g. eth.src[0..15] > 0x1111 in place of "