[ovs-dev] [PATCH ovn] ovn-nbctl: Fix double-free of parsed commands on error path.

[Numan Siddique]

On Wed, Nov 25, 2020 at 10:02 PM Dumitru Ceara <dceara at redhat.com> wrote:
>
> Reported-by: Daniel Alvarez <dalvarez at redhat.com>
> Fixes: 8fb54e16378c ("ovn-nbctl: Cleanup allocated memory to keep valgrind happy.")
> Signed-off-by: Dumitru Ceara <dceara at 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 at openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>