From e2c83c61f0d4c1cfa88fe7a6fef1cb2d4efc7a02 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) (cherry picked from commit de87340b2f9713baec65a3b291b5fd783e9be1dd) --- 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 ff4c15c37e..788b71099c 100644 --- a/src/nmcli/common.c +++ b/src/nmcli/common.c @@ -874,7 +874,10 @@ static void readline_cb(char *line) { rl_got_line = TRUE; - rl_string = line; + + free(rl_string); + rl_string = line; + rl_callback_handler_remove(); } @@ -889,13 +892,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); @@ -943,7 +948,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; } /**