* [PATCH 1/2] tools/btmgmt: Fix crash in completion in interactive mode @ 2015-02-12 16:30 Szymon Janc 2015-02-12 16:30 ` [PATCH 2/2] tools/btmgmt: Handle commands tables in consistent way Szymon Janc 2015-02-12 20:02 ` [PATCH 1/2] tools/btmgmt: Fix crash in completion in interactive mode Johan Hedberg 0 siblings, 2 replies; 4+ messages in thread From: Szymon Janc @ 2015-02-12 16:30 UTC (permalink / raw) To: linux-bluetooth; +Cc: Szymon Janc Use separate indexes while iterating over all_cmd and interactive_cmd. Fix following crash: [mgmt]# ==2224== Invalid read of size 1 ==2224== at 0x4A092F2: strlen (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so) ==2224== by 0x323C8860AD: strdup (in /usr/lib64/libc-2.18.so) ==2224== by 0x323EC1D550: rl_completion_matches (in /usr/lib64/libreadline.so.6.2) ==2224== by 0x402BBC: cmd_completion (btmgmt.c:3427) ==2224== by 0x323EC1D608: ??? (in /usr/lib64/libreadline.so.6.2) ==2224== by 0x323EC1D783: rl_complete_internal (in /usr/lib64/libreadline.so.6.2) ==2224== by 0x323EC156DD: _rl_dispatch_subseq (in /usr/lib64/libreadline.so.6.2) ==2224== by 0x323EC159FF: readline_internal_char (in /usr/lib64/libreadline.so.6.2) ==2224== by 0x323EC2AB6C: rl_callback_read_char (in /usr/lib64/libreadline.so.6.2) ==2224== by 0x4032E8: prompt_read (btmgmt.c:3551) ==2224== by 0x419048: io_callback (io-mainloop.c:123) ==2224== by 0x419842: mainloop_run (mainloop.c:157) ==2224== Address 0x68 is not stack'd, malloc'd or (recently) free'd --- tools/btmgmt.c | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/tools/btmgmt.c b/tools/btmgmt.c index e262350..0686ed6 100644 --- a/tools/btmgmt.c +++ b/tools/btmgmt.c @@ -3375,23 +3375,24 @@ static struct cmd_info interactive_cmd[] = { static char *cmd_generator(const char *text, int state) { - static int index, len; + static int i, j, len; const char *cmd; if (!state) { - index = 0; + i = 0; + j = 0; len = strlen(text); } - while ((cmd = all_cmd[index].cmd)) { - index++; + while ((cmd = all_cmd[i].cmd)) { + i++; if (!strncmp(cmd, text, len)) return strdup(cmd); } - while ((cmd = interactive_cmd[index].cmd)) { - index++; + while ((cmd = interactive_cmd[j].cmd)) { + j++; if (!strncmp(cmd, text, len)) return strdup(cmd); -- 1.9.3 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH 2/2] tools/btmgmt: Handle commands tables in consistent way 2015-02-12 16:30 [PATCH 1/2] tools/btmgmt: Fix crash in completion in interactive mode Szymon Janc @ 2015-02-12 16:30 ` Szymon Janc 2015-02-12 20:02 ` [PATCH 1/2] tools/btmgmt: Fix crash in completion in interactive mode Johan Hedberg 1 sibling, 0 replies; 4+ messages in thread From: Szymon Janc @ 2015-02-12 16:30 UTC (permalink / raw) To: linux-bluetooth; +Cc: Szymon Janc In some places commands tables were accessed by fixed element count and in some places it was assumed that last element is zeroed. This could lead to accessing invalid memory since tables were not terminated with zeroed element (I couldn't crash the tool but this is most likely only due to static memory being zeroed). Be consistent and terminate arrays with zeroed element and use this for iterating. --- tools/btmgmt.c | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/tools/btmgmt.c b/tools/btmgmt.c index 0686ed6..c8fc9f6 100644 --- a/tools/btmgmt.c +++ b/tools/btmgmt.c @@ -3289,6 +3289,7 @@ static struct cmd_info all_cmd[] = { { "add-device", cmd_add_device, "Add Device" }, { "del-device", cmd_del_device, "Remove Device" }, { "clr-devices",cmd_clr_devices,"Clear Devices" }, + { } }; static void cmd_quit(struct mgmt *mgmt, uint16_t index, @@ -3371,6 +3372,7 @@ static struct cmd_info interactive_cmd[] = { { "quit", cmd_quit, "Exit program" }, { "exit", cmd_quit, "Exit program" }, { "help", NULL, "List supported commands" }, + { } }; static char *cmd_generator(const char *text, int state) @@ -3432,12 +3434,11 @@ static char **cmd_completion(const char *text, int start, int end) return matches; } -static struct cmd_info *find_cmd(const char *cmd, struct cmd_info table[], - size_t cmd_count) +static struct cmd_info *find_cmd(const char *cmd, struct cmd_info table[]) { - size_t i; + int i; - for (i = 0; i < cmd_count; i++) { + for (i = 0; table[i].cmd; i++) { if (!strcmp(table[i].cmd, cmd)) return &table[i]; } @@ -3478,9 +3479,9 @@ static void rl_handler(char *input) argv = w.we_wordv; argc = w.we_wordc; - c = find_cmd(cmd, all_cmd, NELEM(all_cmd)); + c = find_cmd(cmd, all_cmd); if (!c && interactive) - c = find_cmd(cmd, interactive_cmd, NELEM(interactive_cmd)); + c = find_cmd(cmd, interactive_cmd); if (c && c->func) { c->func(mgmt, mgmt_index, argc, argv); @@ -3494,7 +3495,7 @@ static void rl_handler(char *input) print("Available commands:"); - for (i = 0; i < NELEM(all_cmd); i++) { + for (i = 0; all_cmd[i].cmd; i++) { c = &all_cmd[i]; if (c->doc) print(" %s %-*s %s", c->cmd, @@ -3504,7 +3505,7 @@ static void rl_handler(char *input) if (!interactive) goto free_we; - for (i = 0; i < NELEM(interactive_cmd); i++) { + for (i = 0; interactive_cmd[i].cmd; i++) { c = &interactive_cmd[i]; if (c->doc) print(" %s %-*s %s", c->cmd, @@ -3603,7 +3604,7 @@ int main(int argc, char *argv[]) if (argc > 0) { struct cmd_info *c; - c = find_cmd(argv[0], all_cmd, NELEM(all_cmd)); + c = find_cmd(argv[0], all_cmd); if (!c) { fprintf(stderr, "Unknown command: %s\n", argv[0]); mgmt_unref(mgmt); -- 1.9.3 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH 1/2] tools/btmgmt: Fix crash in completion in interactive mode 2015-02-12 16:30 [PATCH 1/2] tools/btmgmt: Fix crash in completion in interactive mode Szymon Janc 2015-02-12 16:30 ` [PATCH 2/2] tools/btmgmt: Handle commands tables in consistent way Szymon Janc @ 2015-02-12 20:02 ` Johan Hedberg 2015-02-12 20:14 ` Szymon Janc 1 sibling, 1 reply; 4+ messages in thread From: Johan Hedberg @ 2015-02-12 20:02 UTC (permalink / raw) To: Szymon Janc; +Cc: linux-bluetooth Hi Szymon, On Thu, Feb 12, 2015, Szymon Janc wrote: > Use separate indexes while iterating over all_cmd and interactive_cmd. > Fix following crash: > > [mgmt]# ==2224== Invalid read of size 1 > ==2224== at 0x4A092F2: strlen (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so) > ==2224== by 0x323C8860AD: strdup (in /usr/lib64/libc-2.18.so) > ==2224== by 0x323EC1D550: rl_completion_matches (in /usr/lib64/libreadline.so.6.2) > ==2224== by 0x402BBC: cmd_completion (btmgmt.c:3427) > ==2224== by 0x323EC1D608: ??? (in /usr/lib64/libreadline.so.6.2) > ==2224== by 0x323EC1D783: rl_complete_internal (in /usr/lib64/libreadline.so.6.2) > ==2224== by 0x323EC156DD: _rl_dispatch_subseq (in /usr/lib64/libreadline.so.6.2) > ==2224== by 0x323EC159FF: readline_internal_char (in /usr/lib64/libreadline.so.6.2) > ==2224== by 0x323EC2AB6C: rl_callback_read_char (in /usr/lib64/libreadline.so.6.2) > ==2224== by 0x4032E8: prompt_read (btmgmt.c:3551) > ==2224== by 0x419048: io_callback (io-mainloop.c:123) > ==2224== by 0x419842: mainloop_run (mainloop.c:157) > ==2224== Address 0x68 is not stack'd, malloc'd or (recently) free'd > --- > tools/btmgmt.c | 13 +++++++------ > 1 file changed, 7 insertions(+), 6 deletions(-) This patch has been applied. Thanks. For your second patch I went actually in the other directions and used NELEM() everywhere. I prefer that since it's a stronger guarantee of the table length than having to remember to put an empty element at the end of it. Johan ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH 1/2] tools/btmgmt: Fix crash in completion in interactive mode 2015-02-12 20:02 ` [PATCH 1/2] tools/btmgmt: Fix crash in completion in interactive mode Johan Hedberg @ 2015-02-12 20:14 ` Szymon Janc 0 siblings, 0 replies; 4+ messages in thread From: Szymon Janc @ 2015-02-12 20:14 UTC (permalink / raw) To: Johan Hedberg; +Cc: Szymon Janc, linux-bluetooth Hi Johan, On Thursday 12 February 2015 22:02:10 Johan Hedberg wrote: > Hi Szymon, > > On Thu, Feb 12, 2015, Szymon Janc wrote: > > Use separate indexes while iterating over all_cmd and interactive_cmd. > > Fix following crash: > > > > [mgmt]# ==2224== Invalid read of size 1 > > ==2224== at 0x4A092F2: strlen (in > > /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so) ==2224== by > > 0x323C8860AD: strdup (in /usr/lib64/libc-2.18.so) > > ==2224== by 0x323EC1D550: rl_completion_matches (in > > /usr/lib64/libreadline.so.6.2) ==2224== by 0x402BBC: cmd_completion > > (btmgmt.c:3427) > > ==2224== by 0x323EC1D608: ??? (in /usr/lib64/libreadline.so.6.2) > > ==2224== by 0x323EC1D783: rl_complete_internal (in > > /usr/lib64/libreadline.so.6.2) ==2224== by 0x323EC156DD: > > _rl_dispatch_subseq (in /usr/lib64/libreadline.so.6.2) ==2224== by > > 0x323EC159FF: readline_internal_char (in /usr/lib64/libreadline.so.6.2) > > ==2224== by 0x323EC2AB6C: rl_callback_read_char (in > > /usr/lib64/libreadline.so.6.2) ==2224== by 0x4032E8: prompt_read > > (btmgmt.c:3551) > > ==2224== by 0x419048: io_callback (io-mainloop.c:123) > > ==2224== by 0x419842: mainloop_run (mainloop.c:157) > > ==2224== Address 0x68 is not stack'd, malloc'd or (recently) free'd > > --- > > > > tools/btmgmt.c | 13 +++++++------ > > 1 file changed, 7 insertions(+), 6 deletions(-) > > This patch has been applied. Thanks. > > For your second patch I went actually in the other directions and used > NELEM() everywhere. I prefer that since it's a stronger guarantee of the > table length than having to remember to put an empty element at the end > of it. > > Johan > -- > To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" > in the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html I've decided to go with it since I used it in RFC serie where command table is passed to common interactive code. But I guess it shouldn't be a problem to pass length along with it. -- Szymon K. Janc szymon.janc@gmail.com ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2015-02-12 20:14 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2015-02-12 16:30 [PATCH 1/2] tools/btmgmt: Fix crash in completion in interactive mode Szymon Janc 2015-02-12 16:30 ` [PATCH 2/2] tools/btmgmt: Handle commands tables in consistent way Szymon Janc 2015-02-12 20:02 ` [PATCH 1/2] tools/btmgmt: Fix crash in completion in interactive mode Johan Hedberg 2015-02-12 20:14 ` Szymon Janc
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.