2018-03-05 08:41:11

by ERAMOTO Masaya

[permalink] [raw]
Subject: [PATCH BlueZ 1/2] shared/shell: Fix memory leak when parsing args

When running the command having mandatory arguments, memory leak occurs
as below:

1 bytes in 1 blocks are definitely lost in loss record 2 of 184
at 0x4C2FB0F: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
by 0x5679C99: strdup (strdup.c:42)
by 0x13A976: cmd_exec (shell.c:327)
by 0x13A976: menu_exec (shell.c:375)
by 0x13AEA4: shell_exec (shell.c:418)
by 0x13BBF1: rl_handler (shell.c:563)
by 0x53C8D72: rl_callback_read_char (in /lib/x86_64-linux-gnu/libreadline.so.7.0)
by 0x13ACE0: input_read (shell.c:1018)
by 0x13C90A: watch_callback (io-glib.c:170)
by 0x4E86E24: g_main_context_dispatch (in /lib/x86_64-linux-gnu/libglib-2.0.so.0.5400.1)
by 0x4E871EF: ??? (in /lib/x86_64-linux-gnu/libglib-2.0.so.0.5400.1)
by 0x4E87501: g_main_loop_run (in /lib/x86_64-linux-gnu/libglib-2.0.so.0.5400.1)
by 0x13D244: mainloop_run (mainloop-glib.c:73)

7 bytes in 1 blocks are definitely lost in loss record 12 of 184
at 0x4C2FB0F: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
by 0x5679C99: strdup (strdup.c:42)
by 0x139EEA: strdelimit (util.c:991)
by 0x13B66A: args_completion (shell.c:668)
by 0x13B66A: menu_completion (shell.c:695)
by 0x13B836: shell_completion (shell.c:723)
by 0x53B98B6: ??? (in /lib/x86_64-linux-gnu/libreadline.so.7.0)
by 0x53B9A99: rl_complete_internal (in /lib/x86_64-linux-gnu/libreadline.so.7.0)
by 0x53B02EE: _rl_dispatch_subseq (in /lib/x86_64-linux-gnu/libreadline.so.7.0)
by 0x53B07B5: readline_internal_char (in /lib/x86_64-linux-gnu/libreadline.so.7.0)
by 0x53C8F84: rl_callback_read_char (in /lib/x86_64-linux-gnu/libreadline.so.7.0)
by 0x13ACE0: input_read (shell.c:1018)
by 0x13C90A: watch_callback (io-glib.c:170)

9 bytes in 1 blocks are definitely lost in loss record 21 of 184
at 0x4C2FB0F: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
by 0x5679C99: strdup (strdup.c:42)
by 0x13B53F: args_completion (shell.c:654)
by 0x13B53F: menu_completion (shell.c:695)
by 0x13B836: shell_completion (shell.c:723)
by 0x53B98B6: ??? (in /lib/x86_64-linux-gnu/libreadline.so.7.0)
by 0x53B9A99: rl_complete_internal (in /lib/x86_64-linux-gnu/libreadline.so.7.0)
by 0x53B02EE: _rl_dispatch_subseq (in /lib/x86_64-linux-gnu/libreadline.so.7.0)
by 0x53B07B5: readline_internal_char (in /lib/x86_64-linux-gnu/libreadline.so.7.0)
by 0x53C8F84: rl_callback_read_char (in /lib/x86_64-linux-gnu/libreadline.so.7.0)
by 0x13ACE0: input_read (shell.c:1018)
by 0x13C90A: watch_callback (io-glib.c:170)
by 0x4E86E24: g_main_context_dispatch (in /lib/x86_64-linux-gnu/libglib-2.0.so.0.5400.1)

9 bytes in 1 blocks are definitely lost in loss record 22 of 184
at 0x4C2FB0F: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
by 0x5679CE9: strndup (strndup.c:43)
by 0x13A9EB: cmd_exec (shell.c:301)
by 0x13A9EB: menu_exec (shell.c:375)
by 0x13AEA4: shell_exec (shell.c:418)
by 0x13BBF1: rl_handler (shell.c:563)
by 0x53C8D72: rl_callback_read_char (in /lib/x86_64-linux-gnu/libreadline.so.7.0)
by 0x13ACE0: input_read (shell.c:1018)
by 0x13C90A: watch_callback (io-glib.c:170)
by 0x4E86E24: g_main_context_dispatch (in /lib/x86_64-linux-gnu/libglib-2.0.so.0.5400.1)
by 0x4E871EF: ??? (in /lib/x86_64-linux-gnu/libglib-2.0.so.0.5400.1)
by 0x4E87501: g_main_loop_run (in /lib/x86_64-linux-gnu/libglib-2.0.so.0.5400.1)
by 0x13D244: mainloop_run (mainloop-glib.c:73)
---
src/shared/shell.c | 12 +++++++++++-
1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/src/shared/shell.c b/src/shared/shell.c
index fc4c9b5a5..f962f21e8 100644
--- a/src/shared/shell.c
+++ b/src/shared/shell.c
@@ -313,9 +313,12 @@ static int cmd_exec(const struct bt_shell_menu_entry *entry,
if (parse_args(man, &w, "<>", flags) < 0) {
print_text(COLOR_HIGHLIGHT,
"Unable to parse mandatory command arguments: %s", man );
+ free(man);
return -EINVAL;
}

+ free(man);
+
/* Check if there are enough arguments */
if ((unsigned) argc - 1 < w.we_wordc) {
print_text(COLOR_HIGHLIGHT, "Missing %s argument",
@@ -330,9 +333,12 @@ optional:
if (parse_args(opt, &w, "[]", flags) < 0) {
print_text(COLOR_HIGHLIGHT,
"Unable to parse optional command arguments: %s", opt);
+ free(opt);
return -EINVAL;
}

+ free(opt);
+
/* Check if there are too many arguments */
if ((unsigned) argc - 1 > w.we_wordc && !w.we_offs) {
print_text(COLOR_HIGHLIGHT, "Too many arguments: %d > %zu",
@@ -649,7 +655,7 @@ static char **args_completion(const struct bt_shell_menu_entry *entry, int argc,
return NULL;

if (!entry->arg)
- goto done;
+ goto end;

str = strdup(entry->arg);

@@ -664,6 +670,8 @@ static char **args_completion(const struct bt_shell_menu_entry *entry, int argc,
if (!strrchr(entry->arg, '/'))
goto done;

+ free(str);
+
/* Split values separated by / */
str = strdelimit(args.we_wordv[index], "/", ' ');

@@ -674,6 +682,8 @@ static char **args_completion(const struct bt_shell_menu_entry *entry, int argc,
matches = rl_completion_matches(text, arg_generator);

done:
+ free(str);
+end:
if (!matches && text[0] == '\0')
bt_shell_printf("Usage: %s %s\n", entry->cmd,
entry->arg ? entry->arg : "");
--
2.14.1



2018-03-07 09:30:10

by Luiz Augusto von Dentz

[permalink] [raw]
Subject: Re: [PATCH BlueZ 1/2] shared/shell: Fix memory leak when parsing args

Hi Eramoto,

On Mon, Mar 5, 2018 at 10:41 AM, ERAMOTO Masaya
<[email protected]> wrote:
> When running the command having mandatory arguments, memory leak occurs
> as below:
>
> 1 bytes in 1 blocks are definitely lost in loss record 2 of 184
> at 0x4C2FB0F: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
> by 0x5679C99: strdup (strdup.c:42)
> by 0x13A976: cmd_exec (shell.c:327)
> by 0x13A976: menu_exec (shell.c:375)
> by 0x13AEA4: shell_exec (shell.c:418)
> by 0x13BBF1: rl_handler (shell.c:563)
> by 0x53C8D72: rl_callback_read_char (in /lib/x86_64-linux-gnu/libreadline.so.7.0)
> by 0x13ACE0: input_read (shell.c:1018)
> by 0x13C90A: watch_callback (io-glib.c:170)
> by 0x4E86E24: g_main_context_dispatch (in /lib/x86_64-linux-gnu/libglib-2.0.so.0.5400.1)
> by 0x4E871EF: ??? (in /lib/x86_64-linux-gnu/libglib-2.0.so.0.5400.1)
> by 0x4E87501: g_main_loop_run (in /lib/x86_64-linux-gnu/libglib-2.0.so.0.5400.1)
> by 0x13D244: mainloop_run (mainloop-glib.c:73)
>
> 7 bytes in 1 blocks are definitely lost in loss record 12 of 184
> at 0x4C2FB0F: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
> by 0x5679C99: strdup (strdup.c:42)
> by 0x139EEA: strdelimit (util.c:991)
> by 0x13B66A: args_completion (shell.c:668)
> by 0x13B66A: menu_completion (shell.c:695)
> by 0x13B836: shell_completion (shell.c:723)
> by 0x53B98B6: ??? (in /lib/x86_64-linux-gnu/libreadline.so.7.0)
> by 0x53B9A99: rl_complete_internal (in /lib/x86_64-linux-gnu/libreadline.so.7.0)
> by 0x53B02EE: _rl_dispatch_subseq (in /lib/x86_64-linux-gnu/libreadline.so.7.0)
> by 0x53B07B5: readline_internal_char (in /lib/x86_64-linux-gnu/libreadline.so.7.0)
> by 0x53C8F84: rl_callback_read_char (in /lib/x86_64-linux-gnu/libreadline.so.7.0)
> by 0x13ACE0: input_read (shell.c:1018)
> by 0x13C90A: watch_callback (io-glib.c:170)
>
> 9 bytes in 1 blocks are definitely lost in loss record 21 of 184
> at 0x4C2FB0F: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
> by 0x5679C99: strdup (strdup.c:42)
> by 0x13B53F: args_completion (shell.c:654)
> by 0x13B53F: menu_completion (shell.c:695)
> by 0x13B836: shell_completion (shell.c:723)
> by 0x53B98B6: ??? (in /lib/x86_64-linux-gnu/libreadline.so.7.0)
> by 0x53B9A99: rl_complete_internal (in /lib/x86_64-linux-gnu/libreadline.so.7.0)
> by 0x53B02EE: _rl_dispatch_subseq (in /lib/x86_64-linux-gnu/libreadline.so.7.0)
> by 0x53B07B5: readline_internal_char (in /lib/x86_64-linux-gnu/libreadline.so.7.0)
> by 0x53C8F84: rl_callback_read_char (in /lib/x86_64-linux-gnu/libreadline.so.7.0)
> by 0x13ACE0: input_read (shell.c:1018)
> by 0x13C90A: watch_callback (io-glib.c:170)
> by 0x4E86E24: g_main_context_dispatch (in /lib/x86_64-linux-gnu/libglib-2.0.so.0.5400.1)
>
> 9 bytes in 1 blocks are definitely lost in loss record 22 of 184
> at 0x4C2FB0F: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
> by 0x5679CE9: strndup (strndup.c:43)
> by 0x13A9EB: cmd_exec (shell.c:301)
> by 0x13A9EB: menu_exec (shell.c:375)
> by 0x13AEA4: shell_exec (shell.c:418)
> by 0x13BBF1: rl_handler (shell.c:563)
> by 0x53C8D72: rl_callback_read_char (in /lib/x86_64-linux-gnu/libreadline.so.7.0)
> by 0x13ACE0: input_read (shell.c:1018)
> by 0x13C90A: watch_callback (io-glib.c:170)
> by 0x4E86E24: g_main_context_dispatch (in /lib/x86_64-linux-gnu/libglib-2.0.so.0.5400.1)
> by 0x4E871EF: ??? (in /lib/x86_64-linux-gnu/libglib-2.0.so.0.5400.1)
> by 0x4E87501: g_main_loop_run (in /lib/x86_64-linux-gnu/libglib-2.0.so.0.5400.1)
> by 0x13D244: mainloop_run (mainloop-glib.c:73)
> ---
> src/shared/shell.c | 12 +++++++++++-
> 1 file changed, 11 insertions(+), 1 deletion(-)
>
> diff --git a/src/shared/shell.c b/src/shared/shell.c
> index fc4c9b5a5..f962f21e8 100644
> --- a/src/shared/shell.c
> +++ b/src/shared/shell.c
> @@ -313,9 +313,12 @@ static int cmd_exec(const struct bt_shell_menu_entry *entry,
> if (parse_args(man, &w, "<>", flags) < 0) {
> print_text(COLOR_HIGHLIGHT,
> "Unable to parse mandatory command arguments: %s", man );
> + free(man);
> return -EINVAL;
> }
>
> + free(man);
> +
> /* Check if there are enough arguments */
> if ((unsigned) argc - 1 < w.we_wordc) {
> print_text(COLOR_HIGHLIGHT, "Missing %s argument",
> @@ -330,9 +333,12 @@ optional:
> if (parse_args(opt, &w, "[]", flags) < 0) {
> print_text(COLOR_HIGHLIGHT,
> "Unable to parse optional command arguments: %s", opt);
> + free(opt);
> return -EINVAL;
> }
>
> + free(opt);
> +
> /* Check if there are too many arguments */
> if ((unsigned) argc - 1 > w.we_wordc && !w.we_offs) {
> print_text(COLOR_HIGHLIGHT, "Too many arguments: %d > %zu",
> @@ -649,7 +655,7 @@ static char **args_completion(const struct bt_shell_menu_entry *entry, int argc,
> return NULL;
>
> if (!entry->arg)
> - goto done;
> + goto end;
>
> str = strdup(entry->arg);
>
> @@ -664,6 +670,8 @@ static char **args_completion(const struct bt_shell_menu_entry *entry, int argc,
> if (!strrchr(entry->arg, '/'))
> goto done;
>
> + free(str);
> +
> /* Split values separated by / */
> str = strdelimit(args.we_wordv[index], "/", ' ');
>
> @@ -674,6 +682,8 @@ static char **args_completion(const struct bt_shell_menu_entry *entry, int argc,
> matches = rl_completion_matches(text, arg_generator);
>
> done:
> + free(str);
> +end:
> if (!matches && text[0] == '\0')
> bt_shell_printf("Usage: %s %s\n", entry->cmd,
> entry->arg ? entry->arg : "");
> --
> 2.14.1

Applied, thanks.

--
Luiz Augusto von Dentz

2018-03-05 08:41:52

by ERAMOTO Masaya

[permalink] [raw]
Subject: [PATCH BlueZ 2/2] shared/shell: Fix memory leak of wordexp with we_offs

This sets the we_offs variable to zero before do wordfree(). Since it
frees from the elements of the we_wordv array pointed to by the we_offs
variable, memory leak occurs as below:

101 bytes in 1 blocks are definitely lost in loss record 122 of 184
at 0x4C2FA3F: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
by 0x4C31D84: realloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
by 0x56E7CD2: w_addchar (wordexp.c:104)
by 0x56E7CD2: parse_dquote (wordexp.c:2200)
by 0x56E7CD2: wordexp (wordexp.c:2349)
by 0x13A6A5: parse_args (shell.c:262)
by 0x13A94D: cmd_exec (shell.c:313)
by 0x13A94D: menu_exec (shell.c:375)
by 0x13AEA4: shell_exec (shell.c:418)
by 0x13BBF1: rl_handler (shell.c:563)
by 0x53C8D72: rl_callback_read_char (in /lib/x86_64-linux-gnu/libreadline.so.7.0)
by 0x13ACE0: input_read (shell.c:1018)
by 0x13C90A: watch_callback (io-glib.c:170)
by 0x4E86E24: g_main_context_dispatch (in /lib/x86_64-linux-gnu/libglib-2.0.so.0.5400.1)
by 0x4E871EF: ??? (in /lib/x86_64-linux-gnu/libglib-2.0.so.0.5400.1)

117 (16 direct, 101 indirect) bytes in 1 blocks are definitely lost in loss record 125 of 184
at 0x4C31D2F: realloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
by 0x56E81C3: w_addword (wordexp.c:182)
by 0x56E81C3: wordexp (wordexp.c:2447)
by 0x13A6A5: parse_args (shell.c:262)
by 0x13B55A: args_completion (shell.c:656)
by 0x13B55A: menu_completion (shell.c:695)
by 0x13B836: shell_completion (shell.c:723)
by 0x53B98B6: ??? (in /lib/x86_64-linux-gnu/libreadline.so.7.0)
by 0x53B9A99: rl_complete_internal (in /lib/x86_64-linux-gnu/libreadline.so.7.0)
by 0x53B02EE: _rl_dispatch_subseq (in /lib/x86_64-linux-gnu/libreadline.so.7.0)
by 0x53B07B5: readline_internal_char (in /lib/x86_64-linux-gnu/libreadline.so.7.0)
by 0x53C8F84: rl_callback_read_char (in /lib/x86_64-linux-gnu/libreadline.so.7.0)
by 0x13ACE0: input_read (shell.c:1018)
by 0x13C90A: watch_callback (io-glib.c:170)
---
src/shared/shell.c | 6 ++++++
1 file changed, 6 insertions(+)

diff --git a/src/shared/shell.c b/src/shared/shell.c
index f962f21e8..3b680d7b0 100644
--- a/src/shared/shell.c
+++ b/src/shared/shell.c
@@ -346,6 +346,7 @@ optional:
goto fail;
}

+ w.we_offs = 0;
wordfree(&w);

exec:
@@ -359,6 +360,7 @@ exec:
return 0;

fail:
+ w.we_offs = 0;
wordfree(&w);
return -EINVAL;
}
@@ -675,6 +677,9 @@ static char **args_completion(const struct bt_shell_menu_entry *entry, int argc,
/* Split values separated by / */
str = strdelimit(args.we_wordv[index], "/", ' ');

+ args.we_offs = 0;
+ wordfree(&args);
+
if (wordexp(str, &args, WRDE_NOCMD))
goto done;

@@ -688,6 +693,7 @@ end:
bt_shell_printf("Usage: %s %s\n", entry->cmd,
entry->arg ? entry->arg : "");

+ args.we_offs = 0;
wordfree(&args);
return matches;
}
--
2.14.1