On 25-01-19 14:17, Tom de Vries wrote: > On 25-01-19 01:51, Ian Lance Taylor wrote: >> On Thu, Jan 24, 2019 at 4:11 PM Nathan Sidwell <nat...@acm.org> wrote: >>> >>> I just tripped over a segfault in libbacktrace. We apply strrchr to a >>> possibly NULL filename, with predictable results when it is. >>> >>> elf.c:3044 passes NULL as the filename parm: >>> ret = elf_add (state, NULL, d, base_address, error_callback, data, >>> fileline_fn, found_sym, found_dwarf, NULL, 0, 1, >>> NULL, >>> 0); >>> >>> This gets to elf_open_debugfile_by_debuglink which passes it on through: >>> ddescriptor = elf_find_debugfile_by_debuglink (state, filename, >>> debuglink_name, >>> error_callback, data); >>> >>> this patch avoids the strrchr when filename is null. I reordered the >>> way prefix & prefix len got set, finding it prefereable to: >>> slash = filename ? NULL : strrchr (filename, '/'); >>> but if you prefer to avoid the assignment in the conditional I'm fine >>> with that too. >> >> Yeah, please don't do an assignment in a conditional. >> >> Why don't we just pass "" instead of NULL in the call to elf_add? If >> that works, that is OK. >> > > With this refactoring preamble ...
... I can more easily add a test-case btest_dwz_gnudebuglink, which triggers a segfault due to strrchr being called with a NULL string. This patch fixes it by passing "" instead of NULL, in the call to elf_add at line 3083 (for .gnu_debugaltlink), not the call to elf_add at line 3044 (for .gnu_debuglink) mentioned above. Nathan, does this fix the problem for you? If not, can you provide a reproducer, or give a hint on how one could be constructed? Thanks, - Tom
[libbacktrace] Fix strrchr segfault Currently, when handling a libbacktrace testcase t with .gnu_debuglink to t.debug, and t.debug having a .gnu_debugaltlink to t.alt.debug, an segfault is triggered in when calling strrchr with a NULL string from elf_find_debugfile_by_debuglink. The NULL string originates from the elf_add called for the .gnu_debugaltlink, which uses NULL as filename argument. Fix this by using "" as filename argument instead. 2019-01-25 Tom de Vries <tdevr...@suse.de> * elf.c (elf_add): When handling .gnu_debugaltlink, Call elf_add with filename == "". * Makefile.am (TESTS): Add btest_dwz_gnudebuglink. * Makefile.in: Regenerate. --- libbacktrace/Makefile.am | 6 ++++++ libbacktrace/Makefile.in | 22 +++++++++++++++------- libbacktrace/elf.c | 2 +- 3 files changed, 22 insertions(+), 8 deletions(-) diff --git a/libbacktrace/Makefile.am b/libbacktrace/Makefile.am index 2f1d9af89c3..997a535dff4 100644 --- a/libbacktrace/Makefile.am +++ b/libbacktrace/Makefile.am @@ -190,6 +190,12 @@ if HAVE_DWZ TESTS += btest_dwz +if HAVE_OBJCOPY_DEBUGLINK + +TESTS += btest_dwz_gnudebuglink + +endif HAVE_OBJCOPY_DEBUGLINK + endif HAVE_DWZ stest_SOURCES = stest.c diff --git a/libbacktrace/Makefile.in b/libbacktrace/Makefile.in index 0b73e3d6981..f04577066f8 100644 --- a/libbacktrace/Makefile.in +++ b/libbacktrace/Makefile.in @@ -127,11 +127,12 @@ check_PROGRAMS = $(am__EXEEXT_1) $(am__EXEEXT_2) $(am__EXEEXT_3) @NATIVE_TRUE@ ztest ztest_alloc edtest edtest_alloc @NATIVE_TRUE@am__append_2 = allocfail.sh @HAVE_DWZ_TRUE@@NATIVE_TRUE@am__append_3 = btest_dwz -@HAVE_ZLIB_TRUE@@NATIVE_TRUE@am__append_4 = -lz +@HAVE_DWZ_TRUE@@HAVE_OBJCOPY_DEBUGLINK_TRUE@@NATIVE_TRUE@am__append_4 = btest_dwz_gnudebuglink @HAVE_ZLIB_TRUE@@NATIVE_TRUE@am__append_5 = -lz -@HAVE_PTHREAD_TRUE@@NATIVE_TRUE@am__append_6 = ttest ttest_alloc -@HAVE_OBJCOPY_DEBUGLINK_TRUE@@NATIVE_TRUE@am__append_7 = btest_gnudebuglink -@HAVE_COMPRESSED_DEBUG_TRUE@@NATIVE_TRUE@am__append_8 = ctestg ctesta \ +@HAVE_ZLIB_TRUE@@NATIVE_TRUE@am__append_6 = -lz +@HAVE_PTHREAD_TRUE@@NATIVE_TRUE@am__append_7 = ttest ttest_alloc +@HAVE_OBJCOPY_DEBUGLINK_TRUE@@NATIVE_TRUE@am__append_8 = btest_gnudebuglink +@HAVE_COMPRESSED_DEBUG_TRUE@@NATIVE_TRUE@am__append_9 = ctestg ctesta \ @HAVE_COMPRESSED_DEBUG_TRUE@@NATIVE_TRUE@ ctestg_alloc \ @HAVE_COMPRESSED_DEBUG_TRUE@@NATIVE_TRUE@ ctesta_alloc subdir = . @@ -789,7 +790,7 @@ libbacktrace_la_LIBADD = \ libbacktrace_la_DEPENDENCIES = $(libbacktrace_la_LIBADD) TESTS = $(check_PROGRAMS) $(am__append_2) $(am__append_3) \ - $(am__append_7) + $(am__append_4) $(am__append_8) @NATIVE_TRUE@check_LTLIBRARIES = libbacktrace_alloc.la \ @NATIVE_TRUE@ libbacktrace_noformat.la \ @NATIVE_TRUE@ libbacktrace_instrumented_alloc.la @@ -834,9 +835,9 @@ TESTS = $(check_PROGRAMS) $(am__append_2) $(am__append_3) \ @NATIVE_TRUE@stest_alloc_LDADD = libbacktrace_alloc.la @NATIVE_TRUE@ztest_SOURCES = ztest.c testlib.c @NATIVE_TRUE@ztest_CFLAGS = -DSRCDIR=\"$(srcdir)\" -@NATIVE_TRUE@ztest_LDADD = libbacktrace.la $(am__append_4) \ +@NATIVE_TRUE@ztest_LDADD = libbacktrace.la $(am__append_5) \ @NATIVE_TRUE@ $(CLOCK_GETTIME_LINK) -@NATIVE_TRUE@ztest_alloc_LDADD = libbacktrace_alloc.la $(am__append_5) \ +@NATIVE_TRUE@ztest_alloc_LDADD = libbacktrace_alloc.la $(am__append_6) \ @NATIVE_TRUE@ $(CLOCK_GETTIME_LINK) @NATIVE_TRUE@ztest_alloc_SOURCES = $(ztest_SOURCES) @NATIVE_TRUE@ztest_alloc_CFLAGS = $(ztest_CFLAGS) @@ -1585,6 +1586,13 @@ btest_dwz.log: btest_dwz --log-file $$b.log --trs-file $$b.trs \ $(am__common_driver_flags) $(AM_LOG_DRIVER_FLAGS) $(LOG_DRIVER_FLAGS) -- $(LOG_COMPILE) \ "$$tst" $(AM_TESTS_FD_REDIRECT) +btest_dwz_gnudebuglink.log: btest_dwz_gnudebuglink + @p='btest_dwz_gnudebuglink'; \ + b='btest_dwz_gnudebuglink'; \ + $(am__check_pre) $(LOG_DRIVER) --test-name "$$f" \ + --log-file $$b.log --trs-file $$b.trs \ + $(am__common_driver_flags) $(AM_LOG_DRIVER_FLAGS) $(LOG_DRIVER_FLAGS) -- $(LOG_COMPILE) \ + "$$tst" $(AM_TESTS_FD_REDIRECT) btest_gnudebuglink.log: btest_gnudebuglink @p='btest_gnudebuglink'; \ b='btest_gnudebuglink'; \ diff --git a/libbacktrace/elf.c b/libbacktrace/elf.c index 01cf6f9cff7..632477f3099 100644 --- a/libbacktrace/elf.c +++ b/libbacktrace/elf.c @@ -3080,7 +3080,7 @@ elf_add (struct backtrace_state *state, const char *filename, int descriptor, if (debugaltlink_view_valid) backtrace_release_view (state, &debugaltlink_view, error_callback, data); - ret = elf_add (state, NULL, d, base_address, error_callback, data, + ret = elf_add (state, "", d, base_address, error_callback, data, fileline_fn, found_sym, found_dwarf, NULL, 0, 1, NULL, 0); if (ret < 0)
