From de87340b2f9713baec65a3b291b5fd783e9be1dd Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 3 Feb 2023 15:38:22 +0100 Subject: [PATCH] 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') (cherry picked from commit 89734c75539b7a0b1e5918eb9a785f144dd3abe9) --- src/nmcli/common.c | 16 +++++++++++++--- 1 file 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; } /**