| Message ID | 1606321830-28524-1-git-send-email-dceara@redhat.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [ovs-dev] ovn-nbctl: Fix double-free of parsed commands on error path. | expand |
On Wed, Nov 25, 2020 at 10:02 PM Dumitru Ceara <dceara@redhat.com> wrote: > > Reported-by: Daniel Alvarez <dalvarez@redhat.com> > Fixes: 8fb54e16378c ("ovn-nbctl: Cleanup allocated memory to keep valgrind happy.") > Signed-off-by: Dumitru Ceara <dceara@redhat.com> Thanks Dumitru and Daniel. I applied this patch to master and backported to branches 20.09, 20.06 and 20.03. Numan > --- > tests/ovn-nbctl.at | 12 ++++++++++++ > utilities/ovn-nbctl.c | 17 ++++++----------- > 2 files changed, 18 insertions(+), 11 deletions(-) > > diff --git a/tests/ovn-nbctl.at b/tests/ovn-nbctl.at > index 11091d8..01edfcb 100644 > --- a/tests/ovn-nbctl.at > +++ b/tests/ovn-nbctl.at > @@ -1976,6 +1976,18 @@ AT_CHECK([ovn-nbctl list forwarding_group], [0], []) > > ]) > > +dnl --------------------------------------------------------------------- > + > +OVN_NBCTL_TEST([ovn_nbctl_negative], [basic negative tests], [ > +AT_CHECK([ovn-nbctl --id=@ls create logical_switch name=foo -- \ > + set logical_switch foo1 name=bar], > + [1], [], [dnl > +ovn-nbctl: no row "foo1" in table Logical_Switch > +]) > +]) > + > +dnl --------------------------------------------------------------------- > + > AT_SETUP([ovn-nbctl - daemon retry connection]) > OVN_NBCTL_TEST_START daemon > AT_CHECK([kill `cat ovsdb-server.pid`]) > diff --git a/utilities/ovn-nbctl.c b/utilities/ovn-nbctl.c > index a050f8d..d19e1b6 100644 > --- a/utilities/ovn-nbctl.c > +++ b/utilities/ovn-nbctl.c > @@ -6430,12 +6430,6 @@ out_error: > the_idl_txn = NULL; > > ovsdb_symbol_table_destroy(symtab); > - for (c = commands; c < &commands[n_commands]; c++) { > - ds_destroy(&c->output); > - table_destroy(c->table); > - free(c->table); > - } > - > return error; > } > > @@ -6851,17 +6845,18 @@ server_cmd_run(struct unixctl_conn *conn, int argc, const char **argv_, > } else { > ds_put_cstr(&output, ds_cstr_ro(&c->output)); > } > - > - ds_destroy(&c->output); > - table_destroy(c->table); > - free(c->table); > } > unixctl_command_reply(conn, ds_cstr_ro(&output)); > ds_destroy(&output); > > out: > free(error); > - for (struct ctl_command *c = commands; c < &commands[n_commands]; c++) { > + > + struct ctl_command *c; > + for (c = commands; c < &commands[n_commands]; c++) { > + ds_destroy(&c->output); > + table_destroy(c->table); > + free(c->table); > shash_destroy_free_data(&c->options); > } > free(commands); > -- > 1.8.3.1 > > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev >
diff --git a/tests/ovn-nbctl.at b/tests/ovn-nbctl.at index 11091d8..01edfcb 100644 --- a/tests/ovn-nbctl.at +++ b/tests/ovn-nbctl.at @@ -1976,6 +1976,18 @@ AT_CHECK([ovn-nbctl list forwarding_group], [0], []) ]) +dnl --------------------------------------------------------------------- + +OVN_NBCTL_TEST([ovn_nbctl_negative], [basic negative tests], [ +AT_CHECK([ovn-nbctl --id=@ls create logical_switch name=foo -- \ + set logical_switch foo1 name=bar], + [1], [], [dnl +ovn-nbctl: no row "foo1" in table Logical_Switch +]) +]) + +dnl --------------------------------------------------------------------- + AT_SETUP([ovn-nbctl - daemon retry connection]) OVN_NBCTL_TEST_START daemon AT_CHECK([kill `cat ovsdb-server.pid`]) diff --git a/utilities/ovn-nbctl.c b/utilities/ovn-nbctl.c index a050f8d..d19e1b6 100644 --- a/utilities/ovn-nbctl.c +++ b/utilities/ovn-nbctl.c @@ -6430,12 +6430,6 @@ out_error: the_idl_txn = NULL; ovsdb_symbol_table_destroy(symtab); - for (c = commands; c < &commands[n_commands]; c++) { - ds_destroy(&c->output); - table_destroy(c->table); - free(c->table); - } - return error; } @@ -6851,17 +6845,18 @@ server_cmd_run(struct unixctl_conn *conn, int argc, const char **argv_, } else { ds_put_cstr(&output, ds_cstr_ro(&c->output)); } - - ds_destroy(&c->output); - table_destroy(c->table); - free(c->table); } unixctl_command_reply(conn, ds_cstr_ro(&output)); ds_destroy(&output); out: free(error); - for (struct ctl_command *c = commands; c < &commands[n_commands]; c++) { + + struct ctl_command *c; + for (c = commands; c < &commands[n_commands]; c++) { + ds_destroy(&c->output); + table_destroy(c->table); + free(c->table); shash_destroy_free_data(&c->options); } free(commands);
Reported-by: Daniel Alvarez <dalvarez@redhat.com> Fixes: 8fb54e16378c ("ovn-nbctl: Cleanup allocated memory to keep valgrind happy.") Signed-off-by: Dumitru Ceara <dceara@redhat.com> --- tests/ovn-nbctl.at | 12 ++++++++++++ utilities/ovn-nbctl.c | 17 ++++++----------- 2 files changed, 18 insertions(+), 11 deletions(-)