Skip to content

Commit

Permalink
dbctl: Fix a couple of memory leaks
Browse files Browse the repository at this point in the history
Nothing is being freed wherever we are calling
ctl_fatal which is fine because the program is
about to shutdown anyway however one of the
leaks was caught by address sanitizer.
Fix most of the leaks that are happening before
call to ctl_fatal.

Direct leak of 64 byte(s) in 1 object(s) allocated from:
    #0 0x568d70 in __interceptor_realloc.part.0 asan_malloc_linux.cpp.o
    #1 0xa530a5 in xrealloc__ /workspace/ovn/ovs/lib/util.c:147:9
    #2 0xa530a5 in xrealloc /workspace/ovn/ovs/lib/util.c:179:12
    #3 0xa530a5 in x2nrealloc /workspace/ovn/ovs/lib/util.c:239:12
    #4 0xa2ee57 in svec_expand /workspace/ovn/ovs/lib/svec.c:92:23
    #5 0xa2ef6e in svec_terminate /workspace/ovn/ovs/lib/svec.c:116:5
    #6 0x82c117 in ovs_cmdl_env_parse_all /workspace/ovn/ovs/lib/command-line.c:98:5
    #7 0x5ad70d in ovn_dbctl_main /workspace/ovn/utilities/ovn-dbctl.c:132:20
    #8 0x5b58c7 in main /workspace/ovn/utilities/ovn-nbctl.c:7943:12

Indirect leak of 10 byte(s) in 1 object(s) allocated from:
    #0 0x569c07 in malloc (/workspace/ovn/utilities/ovn-nbctl+0x569c07) (BuildId: 3a287416f70de43f52382f0336980c876f655f90)
    #1 0xa52d4c in xmalloc__ /workspace/ovn/ovs/lib/util.c:137:15
    #2 0xa52d4c in xmalloc /workspace/ovn/ovs/lib/util.c:172:12
    #3 0xa52d4c in xmemdup0 /workspace/ovn/ovs/lib/util.c:193:15
    #4 0xa2e580 in svec_add /workspace/ovn/ovs/lib/svec.c:71:27
    #5 0x82c041 in ovs_cmdl_env_parse_all /workspace/ovn/ovs/lib/command-line.c:91:5
    #6 0x5ad70d in ovn_dbctl_main /workspace/ovn/utilities/ovn-dbctl.c:132:20
    #7 0x5b58c7 in main /workspace/ovn/utilities/ovn-nbctl.c:7943:12

Indirect leak of 8 byte(s) in 2 object(s) allocated from:
    #0 0x569c07 in malloc (/workspace/ovn/utilities/ovn-nbctl+0x569c07)
    #1 0xa52d4c in xmalloc__ /workspace/ovn/ovs/lib/util.c:137:15
    #2 0xa52d4c in xmalloc /workspace/ovn/ovs/lib/util.c:172:12
    #3 0xa52d4c in xmemdup0 /workspace/ovn/ovs/lib/util.c:193:15
    #4 0xa2e580 in svec_add /workspace/ovn/ovs/lib/svec.c:71:27
    #5 0x82c0b6 in ovs_cmdl_env_parse_all /workspace/ovn/ovs/lib/command-line.c:96:9
    #6 0x5ad70d in ovn_dbctl_main /workspace/ovn/utilities/ovn-dbctl.c:132:20
    #7 0x5b58c7 in main /workspace/ovn/utilities/ovn-nbctl.c:7943:12

Signed-off-by: Ales Musil <amusil@redhat.com>
Reviewed-by: Simon Horman <simon.horman@corigine.com>
Signed-off-by: Dumitru Ceara <dceara@redhat.com>
(cherry picked from commit 577a797)
  • Loading branch information
almusil authored and dceara committed Mar 2, 2023
1 parent b779b14 commit dfb3dcf
Showing 1 changed file with 45 additions and 30 deletions.
75 changes: 45 additions & 30 deletions utilities/ovn-dbctl.c
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,15 @@ static void server_loop(const struct ovn_dbctl_options *dbctl_options,
struct ovsdb_idl *idl, int argc, char *argv[]);
static void ovn_dbctl_exit(int status);

static void
destroy_argv(int argc, char **argv)
{
for (int i = 0; i < argc; i++) {
free(argv[i]);
}
free(argv);
}

int
ovn_dbctl_main(int argc, char *argv[],
const struct ovn_dbctl_options *dbctl_options)
Expand Down Expand Up @@ -151,6 +160,7 @@ ovn_dbctl_main(int argc, char *argv[],
char *error_s = ovs_cmdl_parse_all(argc, argv_, get_all_options(),
&parsed_options, &n_parsed_options);
if (error_s) {
destroy_argv(argc, argv_);
ctl_fatal("%s", error_s);
}

Expand Down Expand Up @@ -179,6 +189,7 @@ ovn_dbctl_main(int argc, char *argv[],
bool daemon_mode = false;
if (get_detach()) {
if (argc != optind) {
destroy_argv(argc, argv_);
ctl_fatal("non-option arguments not supported with --detach "
"(use --help for help)");
}
Expand All @@ -204,11 +215,8 @@ ovn_dbctl_main(int argc, char *argv[],
if (error) {
ovsdb_idl_destroy(idl);
idl = the_idl = NULL;
destroy_argv(argc, argv_);

for (int i = 0; i < argc; i++) {
free(argv_[i]);
}
free(argv_);
ctl_fatal("%s", error);
}

Expand Down Expand Up @@ -237,21 +245,15 @@ ovn_dbctl_main(int argc, char *argv[],
}
free(commands);
if (error) {
for (int i = 0; i < argc; i++) {
free(argv_[i]);
}
free(argv_);
destroy_argv(argc, argv_);
ctl_fatal("%s", error);
}
}

ovsdb_idl_destroy(idl);
idl = the_idl = NULL;
destroy_argv(argc, argv_);

for (int i = 0; i < argc; i++) {
free(argv_[i]);
}
free(argv_);
exit(EXIT_SUCCESS);
}

Expand Down Expand Up @@ -1238,40 +1240,53 @@ dbctl_client(const struct ovn_dbctl_options *dbctl_options,

ctl_timeout_setup(timeout);

char *cmd_result = NULL;
char *cmd_error = NULL;
struct jsonrpc *client;
int exit_status;
char *error_str;

int error = unixctl_client_create(socket_name, &client);
if (error) {
ctl_fatal("%s: could not connect to %s daemon (%s); "
"unset %s to avoid using daemon",
socket_name, program_name, ovs_strerror(error),
dbctl_options->daemon_env_var_name);
error_str = xasprintf("%s: could not connect to %s daemon (%s); "
"unset %s to avoid using daemon",
socket_name, program_name, ovs_strerror(error),
dbctl_options->daemon_env_var_name);
goto log_error;
}

char *cmd_result;
char *cmd_error;
error = unixctl_client_transact(client, "run",
args.n, args.names,
&cmd_result, &cmd_error);
if (error) {
ctl_fatal("%s: transaction error (%s)",
socket_name, ovs_strerror(error));
error_str = xasprintf("%s: transaction error (%s)",
socket_name, ovs_strerror(error));
goto log_error;
}
svec_destroy(&args);

int exit_status;
if (cmd_error) {
exit_status = EXIT_FAILURE;
fprintf(stderr, "%s: %s", program_name, cmd_error);
} else {
exit_status = EXIT_SUCCESS;
fputs(cmd_result, stdout);
goto error;
}

exit_status = EXIT_SUCCESS;
fputs(cmd_result, stdout);
goto cleanup;

log_error:
VLOG_ERR("%s", error_str);
ovs_error(0, "%s", error_str);
free(error_str);

error:
exit_status = EXIT_FAILURE;

cleanup:
free(cmd_result);
free(cmd_error);
jsonrpc_close(client);
for (int i = 0; i < argc; i++) {
free(argv[i]);
}
free(argv);
svec_destroy(&args);
destroy_argv(argc, argv);

exit(exit_status);
}

0 comments on commit dfb3dcf

Please sign in to comment.