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 <amu...@redhat.com>
---
v2: Address comments from Simon:
    - Rearrange the cleanup according to suggestion.
    - Allow free(NULL).
---
 utilities/ovn-dbctl.c | 76 ++++++++++++++++++++++++++-----------------
 1 file changed, 46 insertions(+), 30 deletions(-)

diff --git a/utilities/ovn-dbctl.c b/utilities/ovn-dbctl.c
index f0ee0ba05..dbe6a745a 100644
--- a/utilities/ovn-dbctl.c
+++ b/utilities/ovn-dbctl.c
@@ -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)
@@ -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);
     }
 
@@ -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)");
         }
@@ -206,11 +217,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);
         }
 
@@ -239,21 +247,15 @@ cleanup:
         }
         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);
 }
 
@@ -1240,40 +1242,54 @@ 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);
 }
-- 
2.39.1

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

Reply via email to