From 6caa91b6e58a563be7eeb2844fd2622158d78354 Mon Sep 17 00:00:00 2001 From: Simon Marchi Date: Tue, 19 Feb 2019 21:10:18 -0500 Subject: [PATCH] Fix error message and use-after-free on errors in nested sourced files Errors that happen in nested sourced files (when a sourced file sources another file) lead to a wrong error message, or use-after-free. For example, if I put this in "a.gdb": command_that_doesnt_exist and this in "b.gdb": source a.gdb and try to "source b.gdb" in GDB, the result may look like this: (gdb) source b.gdb b.gdb:1: Error in sourced command file: _that_doesnt_exist:1: Error in sourced command file: Undefined command: "command_that_doesnt_exist". Try "help". Notice the wrong file name where "a.gdb" should be. The exact result may differ, depending on the feelings of the memory allocator. What happens is: - The "source a.gdb" command is saved by command_line_append_input_line in command_line_input's static buffer. - Since we are sourcing a file, the script_from_file function stores the script name (a.gdb) in the source_file_name global. However, it doesn't do a copy, it just saves a pointer to command_line_input's static buffer. - The "command_that_doesnt_exist" command is saved by command_line_append_input_line in command_line_input's static buffer. Depending on what xrealloc does, source_file_name may now point to freed memory, or at the minimum the data it was pointing to was overwritten. - When the error is handled in script_from_file, we dererence source_file_name to print the name of the file in which the error occured. To fix it, I made source_file_name an std::string, so that keeps a copy of the file name instead of pointing to a buffer with a too small lifetime. With this patch, the expected filename is printed, and no use-after-free occurs: (gdb) source b.gdb b.gdb:1: Error in sourced command file: a.gdb:1: Error in sourced command file: Undefined command: "command_that_doesnt_exist". Try "help". I passed explicit template parameters to make_scoped_restore (), so that the second parameter is passed by reference and avoid a copy. It was not as obvious as I first thought to change gdb.base/source.exp to test this, because source commands inside sourced files are interpreted relative to GDB's current working directory, not the directory of the currently sourced file. As a workaround, I moved the snippet that tests errors after the snippet that adds the source directory to the search path. This way, the "source source-error-1.gdb" line in source-error.exp manages to find the file. For reference, here is what ASAN reports when use-after-free occurs: (gdb) source b.gdb ================================================================= ==18498==ERROR: AddressSanitizer: heap-use-after-free on address 0x60c000019847 at pc 0x7f1d3645de8e bp 0x7ffdcb892e50 sp 0x7ffdcb8925c8 READ of size 6 at 0x60c000019847 thread T0 #0 0x7f1d3645de8d in printf_common /build/gcc/src/gcc/libsanitizer/sanitizer_common/sanitizer_common_interceptors_format.inc:546 #1 0x7f1d36477175 in __interceptor_vasprintf /build/gcc/src/gcc/libsanitizer/sanitizer_common/sanitizer_common_interceptors.inc:1525 #2 0x5632eaffa277 in xstrvprintf(char const*, __va_list_tag*) /home/simark/src/binutils-gdb/gdb/common/common-utils.c:122 #3 0x5632eaff96d1 in throw_it /home/simark/src/binutils-gdb/gdb/common/common-exceptions.c:351 #4 0x5632eaff98df in throw_verror(errors, char const*, __va_list_tag*) /home/simark/src/binutils-gdb/gdb/common/common-exceptions.c:379 #5 0x5632eaff9a2a in throw_error(errors, char const*, ...) /home/simark/src/binutils-gdb/gdb/common/common-exceptions.c:394 #6 0x5632eafca21a in script_from_file(_IO_FILE*, char const*) /home/simark/src/binutils-gdb/gdb/cli/cli-script.c:1553 #7 0x5632eaf8a500 in source_script_from_stream /home/simark/src/binutils-gdb/gdb/cli/cli-cmds.c:569 #8 0x5632eaf8a735 in source_script_with_search /home/simark/src/binutils-gdb/gdb/cli/cli-cmds.c:605 #9 0x5632eaf8ab20 in source_command /home/simark/src/binutils-gdb/gdb/cli/cli-cmds.c:664 #10 0x5632eafa8b4a in do_const_cfunc /home/simark/src/binutils-gdb/gdb/cli/cli-decode.c:106 #11 0x5632eafb0687 in cmd_func(cmd_list_element*, char const*, int) /home/simark/src/binutils-gdb/gdb/cli/cli-decode.c:1892 #12 0x5632ebf3dd87 in execute_command(char const*, int) /home/simark/src/binutils-gdb/gdb/top.c:630 #13 0x5632eb3b25d3 in command_handler(char const*) /home/simark/src/binutils-gdb/gdb/event-top.c:583 #14 0x5632ebf3cf09 in read_command_file(_IO_FILE*) /home/simark/src/binutils-gdb/gdb/top.c:425 #15 0x5632eafca054 in script_from_file(_IO_FILE*, char const*) /home/simark/src/binutils-gdb/gdb/cli/cli-script.c:1547 #16 0x5632eaf8a500 in source_script_from_stream /home/simark/src/binutils-gdb/gdb/cli/cli-cmds.c:569 #17 0x5632eaf8a735 in source_script_with_search /home/simark/src/binutils-gdb/gdb/cli/cli-cmds.c:605 #18 0x5632eaf8ab20 in source_command /home/simark/src/binutils-gdb/gdb/cli/cli-cmds.c:664 #19 0x5632eafa8b4a in do_const_cfunc /home/simark/src/binutils-gdb/gdb/cli/cli-decode.c:106 #20 0x5632eafb0687 in cmd_func(cmd_list_element*, char const*, int) /home/simark/src/binutils-gdb/gdb/cli/cli-decode.c:1892 #21 0x5632ebf3dd87 in execute_command(char const*, int) /home/simark/src/binutils-gdb/gdb/top.c:630 #22 0x5632eb3b25d3 in command_handler(char const*) /home/simark/src/binutils-gdb/gdb/event-top.c:583 #23 0x5632eb3b2f87 in command_line_handler(std::unique_ptr >&&) /home/simark/src/binutils-gdb/gdb/event-top.c:770 #24 0x5632eb3b0fe1 in gdb_rl_callback_handler /home/simark/src/binutils-gdb/gdb/event-top.c:213 #25 0x5632ec1c8729 in rl_callback_read_char /home/simark/src/binutils-gdb/readline/callback.c:220 #26 0x5632eb3b0b8f in gdb_rl_callback_read_char_wrapper_noexcept /home/simark/src/binutils-gdb/gdb/event-top.c:175 #27 0x5632eb3b0da1 in gdb_rl_callback_read_char_wrapper /home/simark/src/binutils-gdb/gdb/event-top.c:192 #28 0x5632eb3b2186 in stdin_event_handler(int, void*) /home/simark/src/binutils-gdb/gdb/event-top.c:511 #29 0x5632eb3aa6a9 in handle_file_event /home/simark/src/binutils-gdb/gdb/event-loop.c:733 #30 0x5632eb3aaf41 in gdb_wait_for_event /home/simark/src/binutils-gdb/gdb/event-loop.c:859 #31 0x5632eb3a88ea in gdb_do_one_event() /home/simark/src/binutils-gdb/gdb/event-loop.c:347 #32 0x5632eb3a89bf in start_event_loop() /home/simark/src/binutils-gdb/gdb/event-loop.c:371 #33 0x5632eb76fbfc in captured_command_loop /home/simark/src/binutils-gdb/gdb/main.c:330 #34 0x5632eb772ea8 in captured_main /home/simark/src/binutils-gdb/gdb/main.c:1176 #35 0x5632eb773071 in gdb_main(captured_main_args*) /home/simark/src/binutils-gdb/gdb/main.c:1192 #36 0x5632eabfe7f9 in main /home/simark/src/binutils-gdb/gdb/gdb.c:32 #37 0x7f1d3554f222 in __libc_start_main (/usr/lib/libc.so.6+0x24222) #38 0x5632eabfe5dd in _start (/home/simark/build/binutils-gdb/gdb/gdb+0x195d5dd) 0x60c000019847 is located 7 bytes inside of 128-byte region [0x60c000019840,0x60c0000198c0) freed by thread T0 here: #0 0x7f1d36502491 in __interceptor_realloc /build/gcc/src/gcc/libsanitizer/asan/asan_malloc_linux.cc:105 #1 0x5632eaff9f47 in xrealloc /home/simark/src/binutils-gdb/gdb/common/common-utils.c:62 #2 0x5632eaff6b44 in buffer_grow(buffer*, char const*, unsigned long) /home/simark/src/binutils-gdb/gdb/common/buffer.c:40 #3 0x5632eb3b271d in command_line_append_input_line /home/simark/src/binutils-gdb/gdb/event-top.c:614 #4 0x5632eb3b28c6 in handle_line_of_input(buffer*, char const*, int, char const*) /home/simark/src/binutils-gdb/gdb/event-top.c:654 #5 0x5632ebf402a6 in command_line_input(char const*, char const*) /home/simark/src/binutils-gdb/gdb/top.c:1252 #6 0x5632ebf3cee9 in read_command_file(_IO_FILE*) /home/simark/src/binutils-gdb/gdb/top.c:422 #7 0x5632eafca054 in script_from_file(_IO_FILE*, char const*) /home/simark/src/binutils-gdb/gdb/cli/cli-script.c:1547 #8 0x5632eaf8a500 in source_script_from_stream /home/simark/src/binutils-gdb/gdb/cli/cli-cmds.c:569 #9 0x5632eaf8a735 in source_script_with_search /home/simark/src/binutils-gdb/gdb/cli/cli-cmds.c:605 #10 0x5632eaf8ab20 in source_command /home/simark/src/binutils-gdb/gdb/cli/cli-cmds.c:664 #11 0x5632eafa8b4a in do_const_cfunc /home/simark/src/binutils-gdb/gdb/cli/cli-decode.c:106 #12 0x5632eafb0687 in cmd_func(cmd_list_element*, char const*, int) /home/simark/src/binutils-gdb/gdb/cli/cli-decode.c:1892 #13 0x5632ebf3dd87 in execute_command(char const*, int) /home/simark/src/binutils-gdb/gdb/top.c:630 #14 0x5632eb3b25d3 in command_handler(char const*) /home/simark/src/binutils-gdb/gdb/event-top.c:583 #15 0x5632ebf3cf09 in read_command_file(_IO_FILE*) /home/simark/src/binutils-gdb/gdb/top.c:425 #16 0x5632eafca054 in script_from_file(_IO_FILE*, char const*) /home/simark/src/binutils-gdb/gdb/cli/cli-script.c:1547 #17 0x5632eaf8a500 in source_script_from_stream /home/simark/src/binutils-gdb/gdb/cli/cli-cmds.c:569 #18 0x5632eaf8a735 in source_script_with_search /home/simark/src/binutils-gdb/gdb/cli/cli-cmds.c:605 #19 0x5632eaf8ab20 in source_command /home/simark/src/binutils-gdb/gdb/cli/cli-cmds.c:664 #20 0x5632eafa8b4a in do_const_cfunc /home/simark/src/binutils-gdb/gdb/cli/cli-decode.c:106 #21 0x5632eafb0687 in cmd_func(cmd_list_element*, char const*, int) /home/simark/src/binutils-gdb/gdb/cli/cli-decode.c:1892 #22 0x5632ebf3dd87 in execute_command(char const*, int) /home/simark/src/binutils-gdb/gdb/top.c:630 #23 0x5632eb3b25d3 in command_handler(char const*) /home/simark/src/binutils-gdb/gdb/event-top.c:583 #24 0x5632eb3b2f87 in command_line_handler(std::unique_ptr >&&) /home/simark/src/binutils-gdb/gdb/event-top.c:770 #25 0x5632eb3b0fe1 in gdb_rl_callback_handler /home/simark/src/binutils-gdb/gdb/event-top.c:213 #26 0x5632ec1c8729 in rl_callback_read_char /home/simark/src/binutils-gdb/readline/callback.c:220 #27 0x5632eb3b0b8f in gdb_rl_callback_read_char_wrapper_noexcept /home/simark/src/binutils-gdb/gdb/event-top.c:175 #28 0x5632eb3b0da1 in gdb_rl_callback_read_char_wrapper /home/simark/src/binutils-gdb/gdb/event-top.c:192 #29 0x5632eb3b2186 in stdin_event_handler(int, void*) /home/simark/src/binutils-gdb/gdb/event-top.c:511 previously allocated by thread T0 here: #0 0x7f1d36502491 in __interceptor_realloc /build/gcc/src/gcc/libsanitizer/asan/asan_malloc_linux.cc:105 #1 0x5632eaff9f47 in xrealloc /home/simark/src/binutils-gdb/gdb/common/common-utils.c:62 #2 0x5632eaff6b44 in buffer_grow(buffer*, char const*, unsigned long) /home/simark/src/binutils-gdb/gdb/common/buffer.c:40 #3 0x5632eb3b271d in command_line_append_input_line /home/simark/src/binutils-gdb/gdb/event-top.c:614 #4 0x5632eb3b28c6 in handle_line_of_input(buffer*, char const*, int, char const*) /home/simark/src/binutils-gdb/gdb/event-top.c:654 #5 0x5632ebf402a6 in command_line_input(char const*, char const*) /home/simark/src/binutils-gdb/gdb/top.c:1252 #6 0x5632ebf3cee9 in read_command_file(_IO_FILE*) /home/simark/src/binutils-gdb/gdb/top.c:422 #7 0x5632eafca054 in script_from_file(_IO_FILE*, char const*) /home/simark/src/binutils-gdb/gdb/cli/cli-script.c:1547 #8 0x5632eaf8a500 in source_script_from_stream /home/simark/src/binutils-gdb/gdb/cli/cli-cmds.c:569 #9 0x5632eaf8a735 in source_script_with_search /home/simark/src/binutils-gdb/gdb/cli/cli-cmds.c:605 #10 0x5632eaf8ab20 in source_command /home/simark/src/binutils-gdb/gdb/cli/cli-cmds.c:664 #11 0x5632eafa8b4a in do_const_cfunc /home/simark/src/binutils-gdb/gdb/cli/cli-decode.c:106 #12 0x5632eafb0687 in cmd_func(cmd_list_element*, char const*, int) /home/simark/src/binutils-gdb/gdb/cli/cli-decode.c:1892 #13 0x5632ebf3dd87 in execute_command(char const*, int) /home/simark/src/binutils-gdb/gdb/top.c:630 #14 0x5632eb3b25d3 in command_handler(char const*) /home/simark/src/binutils-gdb/gdb/event-top.c:583 #15 0x5632eb3b2f87 in command_line_handler(std::unique_ptr >&&) /home/simark/src/binutils-gdb/gdb/event-top.c:770 #16 0x5632eb3b0fe1 in gdb_rl_callback_handler /home/simark/src/binutils-gdb/gdb/event-top.c:213 #17 0x5632ec1c8729 in rl_callback_read_char /home/simark/src/binutils-gdb/readline/callback.c:220 #18 0x5632eb3b0b8f in gdb_rl_callback_read_char_wrapper_noexcept /home/simark/src/binutils-gdb/gdb/event-top.c:175 #19 0x5632eb3b0da1 in gdb_rl_callback_read_char_wrapper /home/simark/src/binutils-gdb/gdb/event-top.c:192 #20 0x5632eb3b2186 in stdin_event_handler(int, void*) /home/simark/src/binutils-gdb/gdb/event-top.c:511 #21 0x5632eb3aa6a9 in handle_file_event /home/simark/src/binutils-gdb/gdb/event-loop.c:733 #22 0x5632eb3aaf41 in gdb_wait_for_event /home/simark/src/binutils-gdb/gdb/event-loop.c:859 #23 0x5632eb3a88ea in gdb_do_one_event() /home/simark/src/binutils-gdb/gdb/event-loop.c:347 #24 0x5632eb3a89bf in start_event_loop() /home/simark/src/binutils-gdb/gdb/event-loop.c:371 #25 0x5632eb76fbfc in captured_command_loop /home/simark/src/binutils-gdb/gdb/main.c:330 #26 0x5632eb772ea8 in captured_main /home/simark/src/binutils-gdb/gdb/main.c:1176 #27 0x5632eb773071 in gdb_main(captured_main_args*) /home/simark/src/binutils-gdb/gdb/main.c:1192 #28 0x5632eabfe7f9 in main /home/simark/src/binutils-gdb/gdb/gdb.c:32 #29 0x7f1d3554f222 in __libc_start_main (/usr/lib/libc.so.6+0x24222) SUMMARY: AddressSanitizer: heap-use-after-free /build/gcc/src/gcc/libsanitizer/sanitizer_common/sanitizer_common_interceptors_format.inc:546 in printf_common gdb/ChangeLog: * top.h (source_file_name): Change to std::string. * top.c (source_file_name): Likewise. (command_line_input): Adjust. * cli/cli-script.c (script_from_file): Adjust. gdb/testsuite/ChangeLog: * gdb.base/source.exp: Move "error in sourced script" code to the end. * gdb.base/source-error.gdb: Move contents to source-error-1.gdb. Add new code to source source-error-1.gdb. * gdb.base/source-error-1.gdb: New file, from previous source-error.gdb. --- gdb/ChangeLog | 7 +++++++ gdb/cli/cli-script.c | 7 ++++--- gdb/testsuite/ChangeLog | 9 +++++++++ gdb/testsuite/gdb.base/source-error-1.gdb | 22 ++++++++++++++++++++++ gdb/testsuite/gdb.base/source-error.gdb | 6 ++---- gdb/testsuite/gdb.base/source.exp | 16 +++++++++++----- gdb/top.c | 4 ++-- gdb/top.h | 2 +- 8 files changed, 58 insertions(+), 15 deletions(-) create mode 100644 gdb/testsuite/gdb.base/source-error-1.gdb diff --git a/gdb/ChangeLog b/gdb/ChangeLog index 4e429f4fa37..eb855819e7c 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,3 +1,10 @@ +2019-02-19 Simon Marchi + + * top.h (source_file_name): Change to std::string. + * top.c (source_file_name): Likewise. + (command_line_input): Adjust. + * cli/cli-script.c (script_from_file): Adjust. + 2019-02-19 Tom Tromey * ravenscar-thread.c diff --git a/gdb/cli/cli-script.c b/gdb/cli/cli-script.c index aaead00a101..85f00c75b3f 100644 --- a/gdb/cli/cli-script.c +++ b/gdb/cli/cli-script.c @@ -1537,8 +1537,9 @@ script_from_file (FILE *stream, const char *file) scoped_restore restore_line_number = make_scoped_restore (&source_line_number, 0); - scoped_restore resotre_file - = make_scoped_restore (&source_file_name, file); + scoped_restore restore_file + = make_scoped_restore (&source_file_name, + file); scoped_restore save_async = make_scoped_restore (¤t_ui->async, 0); @@ -1552,7 +1553,7 @@ script_from_file (FILE *stream, const char *file) prepended. */ throw_error (e.error, _("%s:%d: Error in sourced command file:\n%s"), - source_file_name, source_line_number, e.message); + source_file_name.c_str (), source_line_number, e.message); } END_CATCH } diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog index f50f1e3aca4..cd2a654ca58 100644 --- a/gdb/testsuite/ChangeLog +++ b/gdb/testsuite/ChangeLog @@ -1,3 +1,12 @@ +2019-02-19 Simon Marchi + + * gdb.base/source.exp: Move "error in sourced script" code to + the end. + * gdb.base/source-error.gdb: Move contents to + source-error-1.gdb. Add new code to source source-error-1.gdb. + * gdb.base/source-error-1.gdb: New file, from previous + source-error.gdb. + 2019-02-17 Tom Tromey * gdb.base/style.exp: Use -g3 to compile when possible. Add test diff --git a/gdb/testsuite/gdb.base/source-error-1.gdb b/gdb/testsuite/gdb.base/source-error-1.gdb new file mode 100644 index 00000000000..c879fd2af6f --- /dev/null +++ b/gdb/testsuite/gdb.base/source-error-1.gdb @@ -0,0 +1,22 @@ +# This testcase is part of GDB, the GNU debugger. + +# Copyright 2005-2019 Free Software Foundation, Inc. + +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 3 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see . + +# Test GDB's "source" command - reads in a GDB script. + +echo working line\n +x 0 +echo should not reach this line\n diff --git a/gdb/testsuite/gdb.base/source-error.gdb b/gdb/testsuite/gdb.base/source-error.gdb index c879fd2af6f..85e0d9f8cf1 100644 --- a/gdb/testsuite/gdb.base/source-error.gdb +++ b/gdb/testsuite/gdb.base/source-error.gdb @@ -1,6 +1,6 @@ # This testcase is part of GDB, the GNU debugger. -# Copyright 2005-2019 Free Software Foundation, Inc. +# Copyright 2019 Free Software Foundation, Inc. # This program is free software; you can redistribute it and/or modify # it under the terms of the GNU General Public License as published by @@ -17,6 +17,4 @@ # Test GDB's "source" command - reads in a GDB script. -echo working line\n -x 0 -echo should not reach this line\n +source source-error-1.gdb diff --git a/gdb/testsuite/gdb.base/source.exp b/gdb/testsuite/gdb.base/source.exp index c6ff65783da..a8fef098f10 100644 --- a/gdb/testsuite/gdb.base/source.exp +++ b/gdb/testsuite/gdb.base/source.exp @@ -23,10 +23,6 @@ standard_testfile structs.c gdb_start -gdb_test "source ${srcdir}/${subdir}/source-error.gdb" \ - "source-error.gdb:21: Error in sourced command file:\[\r\n\]*Cannot access memory at address 0x0.*" \ - "script contains error" - gdb_test "source -v ${srcdir}/${subdir}/source-test.gdb" \ "echo test source options.*" \ "source -v" @@ -66,4 +62,14 @@ gdb_test "source for-sure-nonexistant-file" \ gdb_test "source source-nofile.gdb" \ "warning: for-sure-nonexistant-file: No such file or directory\.\[\r\n\]*source error not fatal" -gdb_exit + +# Test commands that error out in sourced files, including in nested sourced +# files. +# +# This needs to come after the "dir" command tested above for source-error.gdb +# to find source-error-1.gdb. +gdb_test "source ${srcdir}/${subdir}/source-error.gdb" \ + [multi_line ".*source-error.gdb:20: Error in sourced command file:" \ + "source-error-1.gdb:21: Error in sourced command file:" \ + "Cannot access memory at address 0x0" ] \ + "script contains error" diff --git a/gdb/top.c b/gdb/top.c index c756a6978e8..22e6f7e29ab 100644 --- a/gdb/top.c +++ b/gdb/top.c @@ -400,7 +400,7 @@ quit_cover (void) /* NOTE 1999-04-29: This variable will be static again, once we modify gdb to use the event loop as the default command loop and we merge event-top.c into this file, top.c. */ -/* static */ const char *source_file_name; +/* static */ std::string source_file_name; /* Read commands from STREAM. */ void @@ -1221,7 +1221,7 @@ command_line_input (const char *prompt_arg, const char *annotation_suffix) gdb_flush (gdb_stdout); gdb_flush (gdb_stderr); - if (source_file_name != NULL) + if (!source_file_name.empty ()) ++source_line_number; if (from_tty && annotation_level > 1) diff --git a/gdb/top.h b/gdb/top.h index 1d860188c6d..025d9389d60 100644 --- a/gdb/top.h +++ b/gdb/top.h @@ -281,7 +281,7 @@ extern void gdb_init (char *); /* For use by event-top.c. */ /* Variables from top.c. */ extern int source_line_number; -extern const char *source_file_name; +extern std::string source_file_name; extern int history_expansion_p; extern int server_command; extern char *lim_at_start;