On Fri, Jan 21, 2022 at 8:50 AM Mark Michelson <[email protected]> wrote:
>
> Thanks Dumitru,
>
> Acked-by: Mark Michelson <[email protected]>

Thanks.  I applied this patch to the main branch.

Numan

>
> On 1/21/22 04:42, Dumitru Ceara wrote:
> > Reported by UB Sanitizer:
> >    lib/actions.c:2309:54: runtime error: applying zero offset to null 
> > pointer
> >        #0 0x4fa41b in free_gen_options /root/ovn/lib/actions.c:2309:54
> >        #1 0x4d6f87 in ovnact_free /root/ovn/lib/actions.c:4297:9
> >        #2 0x4d6f87 in ovnacts_free /root/ovn/lib/actions.c:4315:13
> >        #3 0x4d67b6 in ovnacts_parse /root/ovn/lib/actions.c:4189:9
> >        #4 0x4d74a7 in ovnacts_parse_string /root/ovn/lib/actions.c:4208:5
> >        #5 0x49d3e7 in test_parse_actions /root/ovn/tests/test-ovn.c:1324:17
> >
> >    utilities/ovn-dbctl.c:467:16: runtime error: applying zero offset to 
> > null pointer
> >        #0 0x498a56 in has_option /root/ovn/utilities/ovn-dbctl.c:467:16
> >        #1 0x497f91 in ovn_dbctl_main /root/ovn/utilities/ovn-dbctl.c:167:13
> >        #2 0x4a0b46 in main /root/ovn/utilities/ovn-sbctl.c:1526:12
> >        #3 0x7ff3cdd56b74 in __libc_start_main (/lib64/libc.so.6+0x27b74)
> >        #4 0x47147d in _start (/root/ovn/utilities/ovn-sbctl+0x47147d)
> >
> >    utilities/ovn-dbctl.c:1063:29: runtime error: applying zero offset to 
> > null pointer
> >        #0 0x4bf407 in server_cmd_run /root/ovn/utilities/ovn-dbctl.c:1063:29
> >        #1 0x739bdc in process_command /root/ovs/lib/unixctl.c:310:13
> >        #2 0x73853e in run_connection /root/ovs/lib/unixctl.c:344:17
> >        #3 0x738228 in unixctl_server_run /root/ovs/lib/unixctl.c:395:21
> >        #4 0x4badf5 in server_loop /root/ovn/utilities/ovn-dbctl.c:1128:9
> >        #5 0x4badf5 in ovn_dbctl_main /root/ovn/utilities/ovn-dbctl.c:196:9
> >
> > Signed-off-by: Dumitru Ceara <[email protected]>
> > ---
> > v2: remove trailing whitespace.
> > ---
> >   lib/actions.c         | 44 +++++++++++++++++++++++--------------------
> >   utilities/ovn-dbctl.c | 39 +++++++++++++++++++++-----------------
> >   2 files changed, 46 insertions(+), 37 deletions(-)
> >
> > diff --git a/lib/actions.c b/lib/actions.c
> > index a78d01198..d5d8391bb 100644
> > --- a/lib/actions.c
> > +++ b/lib/actions.c
> > @@ -133,7 +133,8 @@ encode_setup_args(const struct arg args[], size_t 
> > n_args,
> >                     struct ofpbuf *ofpacts)
> >   {
> >       /* 1. Save all of the destinations that will be modified. */
> > -    for (const struct arg *a = args; a < &args[n_args]; a++) {
> > +    for (size_t i = 0; i < n_args; i++) {
> > +        const struct arg *a = &args[i];
> >           ovs_assert(a->src.n_bits == mf_from_id(a->dst)->n_bits);
> >           if (a->src.field->id != a->dst) {
> >               init_stack(ofpact_put_STACK_PUSH(ofpacts), a->dst);
> > @@ -149,7 +150,8 @@ encode_setup_args(const struct arg args[], size_t 
> > n_args,
> >       }
> >
> >       /* 3. Pop the sources into the destinations. */
> > -    for (const struct arg *a = args; a < &args[n_args]; a++) {
> > +    for (size_t i = 0; i < n_args; i++) {
> > +        const struct arg *a = &args[i];
> >           if (a->src.field->id != a->dst) {
> >               init_stack(ofpact_put_STACK_POP(ofpacts), a->dst);
> >           }
> > @@ -1686,8 +1688,8 @@ format_TRIGGER_EVENT(const struct 
> > ovnact_controller_event *event,
> >   {
> >       ds_put_format(s, "trigger_event(event = \"%s\"",
> >                     event_to_string(event->event_type));
> > -    for (const struct ovnact_gen_option *o = event->options;
> > -         o < &event->options[event->n_options]; o++) {
> > +    for (size_t i = 0; i < event->n_options; i++) {
> > +        const struct ovnact_gen_option *o = &event->options[i];
> >           ds_put_cstr(s, ", ");
> >           ds_put_format(s, "%s = ", o->option->name);
> >           expr_constant_set_format(&o->value, s);
> > @@ -1840,8 +1842,8 @@ static void
> >   encode_event_empty_lb_backends_opts(struct ofpbuf *ofpacts,
> >           const struct ovnact_controller_event *event)
> >   {
> > -    for (const struct ovnact_gen_option *o = event->options;
> > -         o < &event->options[event->n_options]; o++) {
> > +    for (size_t i = 0; i < event->n_options; i++) {
> > +        const struct ovnact_gen_option *o = &event->options[i];
> >
> >           /* All empty_lb_backends fields are of type 'str' */
> >           ovs_assert(!strcmp(o->option->type, "str"));
> > @@ -2295,7 +2297,8 @@ parse_gen_opt(struct action_context *ctx, struct 
> > ovnact_gen_option *o,
> >   static const struct ovnact_gen_option *
> >   find_opt(const struct ovnact_gen_option *options, size_t n, size_t code)
> >   {
> > -    for (const struct ovnact_gen_option *o = options; o < &options[n]; 
> > o++) {
> > +    for (size_t i = 0; i < n; i++) {
> > +        const struct ovnact_gen_option *o = &options[i];
> >           if (o->option->code == code) {
> >               return o;
> >           }
> > @@ -2306,7 +2309,8 @@ find_opt(const struct ovnact_gen_option *options, 
> > size_t n, size_t code)
> >   static void
> >   free_gen_options(struct ovnact_gen_option *options, size_t n)
> >   {
> > -    for (struct ovnact_gen_option *o = options; o < &options[n]; o++) {
> > +    for (size_t i = 0; i < n; i++) {
> > +        struct ovnact_gen_option *o = &options[i];
> >           expr_constant_set_destroy(&o->value);
> >       }
> >       free(options);
> > @@ -2317,8 +2321,8 @@ validate_empty_lb_backends(struct action_context *ctx,
> >                              const struct ovnact_gen_option *options,
> >                              size_t n_options)
> >   {
> > -    for (const struct ovnact_gen_option *o = options;
> > -         o < &options[n_options]; o++) {
> > +    for (size_t i = 0; i < n_options; i++) {
> > +        const struct ovnact_gen_option *o = &options[i];
> >           const union expr_constant *c = o->value.values;
> >           struct sockaddr_storage ss;
> >           struct uuid uuid;
> > @@ -2492,8 +2496,8 @@ format_put_opts(const char *name, const struct 
> > ovnact_put_opts *pdo,
> >   {
> >       expr_field_format(&pdo->dst, s);
> >       ds_put_format(s, " = %s(", name);
> > -    for (const struct ovnact_gen_option *o = pdo->options;
> > -         o < &pdo->options[pdo->n_options]; o++) {
> > +    for (size_t i = 0; i < pdo->n_options; i++) {
> > +        const struct ovnact_gen_option *o = &pdo->options[i];
> >           if (o != pdo->options) {
> >               ds_put_cstr(s, ", ");
> >           }
> > @@ -2754,8 +2758,8 @@ encode_PUT_DHCPV4_OPTS(const struct ovnact_put_opts 
> > *pdo,
> >           ofpbuf_put(ofpacts, c->string, opt_header[1]);
> >       }
> >
> > -    for (const struct ovnact_gen_option *o = pdo->options;
> > -         o < &pdo->options[pdo->n_options]; o++) {
> > +    for (size_t i = 0; i < pdo->n_options; i++) {
> > +        const struct ovnact_gen_option *o = &pdo->options[i];
> >           if (o != offerip_opt && o != boot_opt && o != boot_alt_opt) {
> >               encode_put_dhcpv4_option(o, ofpacts);
> >           }
> > @@ -2777,8 +2781,8 @@ encode_PUT_DHCPV6_OPTS(const struct ovnact_put_opts 
> > *pdo,
> >       ovs_be32 ofs = htonl(dst.ofs);
> >       ofpbuf_put(ofpacts, &ofs, sizeof ofs);
> >
> > -    for (const struct ovnact_gen_option *o = pdo->options;
> > -         o < &pdo->options[pdo->n_options]; o++) {
> > +    for (size_t i = 0; i < pdo->n_options; i++) {
> > +        const struct ovnact_gen_option *o = &pdo->options[i];
> >           encode_put_dhcpv6_option(o, ofpacts);
> >       }
> >
> > @@ -2949,8 +2953,8 @@ parse_put_nd_ra_opts(struct action_context *ctx, 
> > const struct expr_field *dst,
> >       bool prefix_set = false;
> >       bool slla_present = false;
> >       /* Let's validate the options. */
> > -    for (struct ovnact_gen_option *o = po->options;
> > -            o < &po->options[po->n_options]; o++) {
> > +    for (size_t i = 0; i < po->n_options; i++) {
> > +        const struct ovnact_gen_option *o = &po->options[i];
> >           const union expr_constant *c = o->value.values;
> >           if (o->value.n_values > 1) {
> >               lexer_error(ctx->lexer, "Invalid value for \"%s\" option",
> > @@ -3120,8 +3124,8 @@ encode_PUT_ND_RA_OPTS(const struct ovnact_put_opts 
> > *po,
> >       ra->mo_flags = 0;
> >       ra->router_lifetime = htons(IPV6_ND_RA_LIFETIME);
> >
> > -    for (const struct ovnact_gen_option *o = po->options;
> > -         o < &po->options[po->n_options]; o++) {
> > +    for (size_t i = 0; i < po->n_options; i++) {
> > +        const struct ovnact_gen_option *o = &po->options[i];
> >           encode_put_nd_ra_option(o, ofpacts, ra_offset);
> >       }
> >
> > diff --git a/utilities/ovn-dbctl.c b/utilities/ovn-dbctl.c
> > index 791caabb2..a292e589d 100644
> > --- a/utilities/ovn-dbctl.c
> > +++ b/utilities/ovn-dbctl.c
> > @@ -221,8 +221,8 @@ ovn_dbctl_main(int argc, char *argv[],
> >   cleanup:
> >           free(args);
> >
> > -        struct ctl_command *c;
> > -        for (c = commands; c < &commands[n_commands]; c++) {
> > +        for (size_t i = 0; i < n_commands; i++) {
> > +            struct ctl_command *c = &commands[i];
> >               ds_destroy(&c->output);
> >               table_destroy(c->table);
> >               free(c->table);
> > @@ -463,8 +463,8 @@ static bool
> >   has_option(const struct ovs_cmdl_parsed_option *parsed_options, size_t n,
> >              int option)
> >   {
> > -    for (const struct ovs_cmdl_parsed_option *po = parsed_options;
> > -         po < &parsed_options[n]; po++) {
> > +    for (size_t i = 0; i < n; i++) {
> > +        const struct ovs_cmdl_parsed_option *po = &parsed_options[i];
> >           if (po->o->val == option) {
> >               return true;
> >           }
> > @@ -510,8 +510,8 @@ apply_options_direct(const struct ovn_dbctl_options 
> > *dbctl_options,
> >                        const struct ovs_cmdl_parsed_option *parsed_options,
> >                        size_t n, struct shash *local_options)
> >   {
> > -    for (const struct ovs_cmdl_parsed_option *po = parsed_options;
> > -         po < &parsed_options[n]; po++) {
> > +    for (size_t i = 0; i < n; i++) {
> > +        const struct ovs_cmdl_parsed_option *po = &parsed_options[i];
> >           bool handled;
> >           char *error = handle_main_loop_option(dbctl_options,
> >                                                 po->o->val, po->arg, 
> > &handled);
> > @@ -620,7 +620,8 @@ run_prerequisites(const struct ovn_dbctl_options 
> > *dbctl_options,
> >   {
> >       dbctl_options->add_base_prerequisites(idl, wait_type);
> >
> > -    for (struct ctl_command *c = commands; c < &commands[n_commands]; c++) 
> > {
> > +    for (size_t i = 0; i < n_commands; i++) {
> > +        struct ctl_command *c = &commands[i];
> >           if (c->syntax->prerequisites) {
> >               struct ctl_context ctx;
> >
> > @@ -685,7 +686,6 @@ do_dbctl(const struct ovn_dbctl_options *dbctl_options,
> >       struct ovsdb_idl_txn *txn;
> >       enum ovsdb_idl_txn_status status;
> >       struct ovsdb_symbol_table *symtab;
> > -    struct ctl_command *c;
> >       struct shash_node *node;
> >       char *error = NULL;
> >
> > @@ -701,13 +701,15 @@ do_dbctl(const struct ovn_dbctl_options 
> > *dbctl_options,
> >       dbctl_options->pre_execute(idl, txn, wait_type);
> >
> >       symtab = ovsdb_symbol_table_create();
> > -    for (c = commands; c < &commands[n_commands]; c++) {
> > +    for (size_t i = 0; i < n_commands; i++) {
> > +        struct ctl_command *c = &commands[i];
> >           ds_init(&c->output);
> >           c->table = NULL;
> >       }
> >       struct ctl_context *ctx = dbctl_options->ctx_create();
> >       ctl_context_init(ctx, NULL, idl, txn, symtab, NULL);
> > -    for (c = commands; c < &commands[n_commands]; c++) {
> > +    for (size_t i = 0; i < n_commands; i++) {
> > +        struct ctl_command *c = &commands[i];
> >           ctl_context_init_command(ctx, c);
> >           if (c->syntax->run) {
> >               (c->syntax->run)(ctx);
> > @@ -750,7 +752,8 @@ do_dbctl(const struct ovn_dbctl_options *dbctl_options,
> >       long long int start_time = time_wall_msec();
> >       status = ovsdb_idl_txn_commit_block(txn);
> >       if (status == TXN_UNCHANGED || status == TXN_SUCCESS) {
> > -        for (c = commands; c < &commands[n_commands]; c++) {
> > +        for (size_t i = 0; i < n_commands; i++) {
> > +            struct ctl_command *c = &commands[i];
> >               if (c->syntax->postprocess) {
> >                   ctl_context_init(ctx, c, idl, txn, symtab, NULL);
> >                   (c->syntax->postprocess)(ctx);
> > @@ -795,7 +798,8 @@ do_dbctl(const struct ovn_dbctl_options *dbctl_options,
> >           OVS_NOT_REACHED();
> >       }
> >
> > -    for (c = commands; c < &commands[n_commands]; c++) {
> > +    for (size_t i = 0; i < n_commands; i++) {
> > +        struct ctl_command *c = &commands[i];
> >           struct ds *ds = &c->output;
> >
> >           if (c->table) {
> > @@ -1043,7 +1047,8 @@ server_cmd_run(struct unixctl_conn *conn, int argc, 
> > const char **argv_,
> >
> >       struct ds output = DS_EMPTY_INITIALIZER;
> >       table_format_reset();
> > -    for (struct ctl_command *c = commands; c < &commands[n_commands]; c++) 
> > {
> > +    for (size_t i = 0; i < n_commands; i++) {
> > +        struct ctl_command *c = &commands[i];
> >           if (c->table) {
> >               table_format(c->table, &table_style, &output);
> >           } else if (oneline) {
> > @@ -1058,8 +1063,8 @@ server_cmd_run(struct unixctl_conn *conn, int argc, 
> > const char **argv_,
> >   out:
> >       free(error);
> >
> > -    struct ctl_command *c;
> > -    for (c = commands; c < &commands[n_commands]; c++) {
> > +    for (size_t i = 0; i < n_commands; i++) {
> > +        struct ctl_command *c = &commands[i];
> >           ds_destroy(&c->output);
> >           table_destroy(c->table);
> >           free(c->table);
> > @@ -1147,8 +1152,8 @@ dbctl_client(const struct ovn_dbctl_options 
> > *dbctl_options,
> >   {
> >       struct svec args = SVEC_EMPTY_INITIALIZER;
> >
> > -    for (const struct ovs_cmdl_parsed_option *po = parsed_options;
> > -         po < &parsed_options[n]; po++) {
> > +    for (size_t i = 0; i < n; i++) {
> > +        const struct ovs_cmdl_parsed_option *po = &parsed_options[i];
> >           optarg = po->arg;
> >           switch (po->o->val) {
> >           case OPT_DB:
> >
>
> _______________________________________________
> dev mailing list
> [email protected]
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to