Re: [ovs-dev] [PATCH v2] expr: Properly initialize expr_constant

[Yifeng Sun]

Yes, I saw that members of expr_constant are accessed in many places without
checking its type. I will work out another version.

For the subject, it is weird that I can't see [ovs-dev] added.

Thanks for your review.
Yifeng


On Tue, Oct 2, 2018 at 2:57 PM Ben Pfaff  wrote:

> On Tue, Oct 02, 2018 at 11:37:02AM -0700, Yifeng Sun wrote:
> > expr_constant.masked may be uninitialized when its type is EXPR_C_STRING.
> > This patch fixes this issue.
> >
> > Reported-at: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=10731
> > Signed-off-by: Yifeng Sun 
>
> Thanks for working to fix the bugs reported by the fuzzer.
>
> I don't understand this fix.  expr_constant is a union.  If an instance
> has type EXPR_C_STRING, then its 'masked' member must not be accessed,
> because that member is only relevant when it has type EXPR_C_INTEGER.
>
> > v1->v2: Fix email subject by adding [ovs-dev]
>
> I don't understand this either.  The mailing list itself adds [ovs-dev].
>
> Thanks,
>
> Ben.
>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v2] expr: Properly initialize expr_constant

[Ben Pfaff]

On Tue, Oct 02, 2018 at 11:37:02AM -0700, Yifeng Sun wrote:
> expr_constant.masked may be uninitialized when its type is EXPR_C_STRING.
> This patch fixes this issue.
> 
> Reported-at: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=10731
> Signed-off-by: Yifeng Sun 

Thanks for working to fix the bugs reported by the fuzzer.

I don't understand this fix.  expr_constant is a union.  If an instance
has type EXPR_C_STRING, then its 'masked' member must not be accessed,
because that member is only relevant when it has type EXPR_C_INTEGER.

> v1->v2: Fix email subject by adding [ovs-dev]

I don't understand this either.  The mailing list itself adds [ovs-dev].

Thanks,

Ben.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH v2] expr: Properly initialize expr_constant

[Yifeng Sun]

expr_constant.masked may be uninitialized when its type is EXPR_C_STRING.
This patch fixes this issue.

Reported-at: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=10731
Signed-off-by: Yifeng Sun 
---
v1->v2: Fix email subject by adding [ovs-dev]

 include/ovn/expr.h | 10 +-
 ovn/lib/expr.c |  4 
 2 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/include/ovn/expr.h b/include/ovn/expr.h
index 3995e62f066c..8d3cab399bf0 100644
--- a/include/ovn/expr.h
+++ b/include/ovn/expr.h
@@ -450,7 +450,9 @@ enum expr_constant_type {
 EXPR_C_STRING
 };
 
-/* A string or integer constant (one must know which from context). */
+/* A string or integer constant (one must know which from context).
+ * It should be initialized through expr_constant_init().
+ */
 union expr_constant {
 /* Integer constant.
  *
@@ -469,6 +471,12 @@ union expr_constant {
 char *string;
 };
 
+static inline void
+expr_constant_init(union expr_constant *c)
+{
+memset(c, 0, sizeof *c);
+}
+
 bool expr_constant_parse(struct lexer *, const struct expr_field *,
  union expr_constant *);
 void expr_constant_format(const union expr_constant *,
diff --git a/ovn/lib/expr.c b/ovn/lib/expr.c
index 148ac869e861..0a2cef2f0ee9 100644
--- a/ovn/lib/expr.c
+++ b/ovn/lib/expr.c
@@ -805,6 +805,7 @@ parse_constant(struct expr_context *ctx, struct 
expr_constant_set *cs,
 }
 
 union expr_constant *c = >values[cs->n_values++];
+expr_constant_init(c);
 c->value = ctx->lexer->token.value;
 c->format = ctx->lexer->token.format;
 c->masked = ctx->lexer->token.type == LEX_T_MASKED_INTEGER;
@@ -1004,6 +1005,7 @@ expr_const_sets_add(struct shash *const_sets, const char 
*name,
   values[i], lex.token.type);
 } else {
 union expr_constant *c = >values[cs->n_values++];
+expr_constant_init(c);
 c->value = lex.token.value;
 c->format = lex.token.format;
 c->masked = lex.token.type == LEX_T_MASKED_INTEGER;
@@ -1017,6 +1019,7 @@ expr_const_sets_add(struct shash *const_sets, const char 
*name,
 cs->type = EXPR_C_STRING;
 for (size_t i = 0; i < n_values; i++) {
 union expr_constant *c = >values[cs->n_values++];
+expr_constant_init(c);
 c->string = xstrdup(values[i]);
 }
 }
@@ -1120,6 +1123,7 @@ expr_parse_primary(struct expr_context *ctx, bool *atomic)
 *atomic = true;
 
 union expr_constant *cst = xzalloc(sizeof *cst);
+expr_constant_init(cst);
 cst->format = LEX_F_HEXADECIMAL;
 cst->masked = false;
 
-- 
2.7.4

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev