cli: avoid leak in readline_cb() overwriting previous line

Such leaks show up in valgrind, and are simply bugs.

Also, various callers (not all of them, which is another bug!) like to
take ownership of the returned string and free it. That means, we leave
a dangling pointer in the global variable, which is very ugly and error
prone.

Also, the callers like to free the string with g_free(), which is not
appropriate for the "rl_string" memory which was allocated by readline.
It must be freed with free(). Avoid that, by cloning the string using
the glib allocator.

Fixes: 995229181cac ('cli: remove editor thread')

1 files changed, 13 insertions, 3 deletions

diff --git a/src/nmcli/common.c b/src/nmcli/common.c
index 9fdc412720..008a3cec28 100644
--- a/src/nmcli/common.c
+++ b/src/nmcli/common.c
@@ -895,7 +895,10 @@ static void
 readline_cb(char *line)
 {
     rl_got_line = TRUE;
-    rl_string   = line;
+
+    free(rl_string);
+    rl_string = line;
+
     rl_callback_handler_remove();
 }
 
@@ -910,13 +913,15 @@ static char *
 nmc_readline_helper(const NmcConfig *nmc_config, const char *prompt)
 {
     GSource *io_source;
+    char    *result;
 
     nmc_set_in_readline(TRUE);
 
     io_source = nm_g_unix_fd_add_source(STDIN_FILENO, G_IO_IN, stdin_ready_cb, NULL);
 
 read_again:
-    rl_string   = NULL;
+    nm_clear_free(&rl_string);
+
     rl_got_line = FALSE;
     rl_callback_handler_install(prompt, readline_cb);
 
@@ -964,7 +969,12 @@ read_again:
 
     nmc_set_in_readline(FALSE);
 
-    return rl_string;
+    if (!rl_string)
+        return NULL;
+
+    result = g_strdup(rl_string);
+    nm_clear_free(&rl_string);
+    return result;
 }
 
 /**