Re: libbacktrace patch: fix memory allocation buglet on DWARF line table with zero dirs

[Ian Lance Taylor via gcc-patches]

On Fri, May 19, 2017 at 6:18 AM, Than McIntosh  wrote:
>
> I wrote:
> == The attached patch to libbacktrace is intended to fix a memory
> == allocation bug involving reading of line table information.
>
> I've revised my previous patch to include a new test ("edtest") that
> verifies the fix.

Thanks.

I changed the use of mvifdiff to use ../move-if-change.  (We use
mvifdiff in libgo to make it less dependent on GCC infrastructure.)

Committed to mainline.

Ian

Re: libbacktrace patch: fix memory allocation buglet on DWARF line table with zero dirs

[Than McIntosh via gcc-patches]

Hi again,

I wrote:
== The attached patch to libbacktrace is intended to fix a memory
== allocation bug involving reading of line table information.

I've revised my previous patch to include a new test ("edtest") that
verifies the fix.

Thanks, Than

---

Patch (take 2):

diff --git a/Makefile.am b/Makefile.am
index ec494c4..d236261 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -96,6 +96,17 @@ stest_LDADD = libbacktrace.la

 check_PROGRAMS += stest

+edtest_SOURCES = edtest.c edtest2_build.c
+edtest_LDADD = libbacktrace.la
+
+check_PROGRAMS += edtest
+
+edtest2_build.c: gen_edtest2_build; @true
+gen_edtest2_build: $(srcdir)/edtest2.c
+ cat $(srcdir)/edtest2.c > tmp-edtest2_build.c
+ $(SHELL) $(srcdir)/mvifdiff.sh tmp-edtest2_build.c edtest2_build.c
+ echo timestamp > $@
+
 endif NATIVE

 # We can't use automake's automatic dependency tracking, because it
@@ -126,5 +137,6 @@ read.lo: config.h backtrace.h internal.h
 simple.lo: config.h backtrace.h internal.h
 sort.lo: config.h backtrace.h internal.h
 stest.lo: config.h backtrace.h internal.h
+edtest.lo: config.h backtrace.h internal.h
 state.lo: config.h backtrace.h backtrace-supported.h internal.h
 unknown.lo: config.h backtrace.h internal.h
diff --git a/Makefile.in b/Makefile.in
index f3a7a42..04c6689 100644
--- a/Makefile.in
+++ b/Makefile.in
@@ -85,7 +85,7 @@ build_triplet = @build@
 host_triplet = @host@
 target_triplet = @target@
 check_PROGRAMS = $(am__EXEEXT_1)
-@NATIVE_TRUE@am__append_1 = btest stest
+@NATIVE_TRUE@am__append_1 = btest stest edtest
 subdir = .
 DIST_COMMON = $(srcdir)/Makefile.in $(srcdir)/Makefile.am \
  $(top_srcdir)/configure $(am__configure_deps) \
@@ -135,13 +135,18 @@ am__DEPENDENCIES_1 =
 am_libbacktrace_la_OBJECTS = atomic.lo dwarf.lo fileline.lo posix.lo \
  print.lo sort.lo state.lo
 libbacktrace_la_OBJECTS = $(am_libbacktrace_la_OBJECTS)
-@NATIVE_TRUE@am__EXEEXT_1 = btest$(EXEEXT) stest$(EXEEXT)
+@NATIVE_TRUE@am__EXEEXT_1 = btest$(EXEEXT) stest$(EXEEXT) \
+@NATIVE_TRUE@ edtest$(EXEEXT)
 @NATIVE_TRUE@am_btest_OBJECTS = btest-btest.$(OBJEXT)
 btest_OBJECTS = $(am_btest_OBJECTS)
 @NATIVE_TRUE@btest_DEPENDENCIES = libbacktrace.la
 btest_LINK = $(LIBTOOL) --tag=CC $(AM_LIBTOOLFLAGS) $(LIBTOOLFLAGS) \
  --mode=link $(CCLD) $(btest_CFLAGS) $(CFLAGS) $(AM_LDFLAGS) \
  $(LDFLAGS) -o $@
+@NATIVE_TRUE@am_edtest_OBJECTS = edtest.$(OBJEXT) \
+@NATIVE_TRUE@ edtest2_build.$(OBJEXT)
+edtest_OBJECTS = $(am_edtest_OBJECTS)
+@NATIVE_TRUE@edtest_DEPENDENCIES = libbacktrace.la
 @NATIVE_TRUE@am_stest_OBJECTS = stest.$(OBJEXT)
 stest_OBJECTS = $(am_stest_OBJECTS)
 @NATIVE_TRUE@stest_DEPENDENCIES = libbacktrace.la
@@ -158,7 +163,7 @@ LINK = $(LIBTOOL) --tag=CC $(AM_LIBTOOLFLAGS)
$(LIBTOOLFLAGS) \
  --mode=link $(CCLD) $(AM_CFLAGS) $(CFLAGS) $(AM_LDFLAGS) \
  $(LDFLAGS) -o $@
 SOURCES = $(libbacktrace_la_SOURCES) $(EXTRA_libbacktrace_la_SOURCES) \
- $(btest_SOURCES) $(stest_SOURCES)
+ $(btest_SOURCES) $(edtest_SOURCES) $(stest_SOURCES)
 MULTISRCTOP =
 MULTIBUILDTOP =
 MULTIDIRS =
@@ -350,6 +355,8 @@ TESTS = $(check_PROGRAMS)
 @NATIVE_TRUE@btest_LDADD = libbacktrace.la
 @NATIVE_TRUE@stest_SOURCES = stest.c
 @NATIVE_TRUE@stest_LDADD = libbacktrace.la
+@NATIVE_TRUE@edtest_SOURCES = edtest.c edtest2_build.c
+@NATIVE_TRUE@edtest_LDADD = libbacktrace.la
 all: config.h
  $(MAKE) $(AM_MAKEFLAGS) all-am

@@ -452,6 +459,9 @@ clean-checkPROGRAMS:
 btest$(EXEEXT): $(btest_OBJECTS) $(btest_DEPENDENCIES)
$(EXTRA_btest_DEPENDENCIES)
  @rm -f btest$(EXEEXT)
  $(btest_LINK) $(btest_OBJECTS) $(btest_LDADD) $(LIBS)
+edtest$(EXEEXT): $(edtest_OBJECTS) $(edtest_DEPENDENCIES)
$(EXTRA_edtest_DEPENDENCIES)
+ @rm -f edtest$(EXEEXT)
+ $(LINK) $(edtest_OBJECTS) $(edtest_LDADD) $(LIBS)
 stest$(EXEEXT): $(stest_OBJECTS) $(stest_DEPENDENCIES)
$(EXTRA_stest_DEPENDENCIES)
  @rm -f stest$(EXEEXT)
  $(LINK) $(stest_OBJECTS) $(stest_LDADD) $(LIBS)
@@ -801,6 +811,12 @@ uninstall-am: uninstall-includeHEADERS
uninstall-libLTLIBRARIES
  uninstall-am uninstall-includeHEADERS uninstall-libLTLIBRARIES


+@NATIVE_TRUE@edtest2_build.c: gen_edtest2_build; @true
+@NATIVE_TRUE@gen_edtest2_build: $(srcdir)/edtest2.c
+@NATIVE_TRUE@ cat $(srcdir)/edtest2.c > tmp-edtest2_build.c
+@NATIVE_TRUE@ $(SHELL) $(srcdir)/mvifdiff.sh tmp-edtest2_build.c
edtest2_build.c
+@NATIVE_TRUE@ echo timestamp > $@
+
 # We can't use automake's automatic dependency tracking, because it
 # breaks when using bootstrap-lean.  Automatic dependency tracking
 # with GCC bootstrap will cause some of the objects to depend on
@@ -829,6 +845,7 @@ read.lo: config.h backtrace.h internal.h
 simple.lo: config.h backtrace.h internal.h
 sort.lo: config.h backtrace.h internal.h
 stest.lo: config.h backtrace.h internal.h
+edtest.lo: config.h backtrace.h internal.h
 state.lo: config.h backtrace.h backtrace-supported.h internal.h
 unknown.lo: config.h backtrace.h internal.h

diff --git a/dwarf.c b/dwarf.c
index b182567..c1bfbf5 100644
--- a/dwarf.c
+++ b/dwarf.c
@@ -1648,16 +1648,15 @@ add_line (struct backtrace_state *state,
struct 

libbacktrace patch: fix memory allocation buglet on DWARF line table with zero dirs

[Than McIntosh via gcc-patches]

Hello,

The attached patch to libbacktrace is intended to fix a memory
allocation bug involving reading of line table information.

The scenario of interest takes place when libbacktrace reads a DWARF
line table whose directory count is zero (an unusual case). If the
memory allocator invocation triggers a call to mmap, this can cause
the size passed to mmap to be zero, resulting in an EINVAL error.  The
fix is to detect the zero-directory case and avoid invoking the
allocation helper with a zero size.

Questions + comments welcome.

Thanks, Than

---

diff --git a/libbacktrace/dwarf.c b/libbacktrace/dwarf.c
index 80c64034092..bcb2c0991ef 100644
--- a/libbacktrace/dwarf.c
+++ b/libbacktrace/dwarf.c
@@ -1563,16 +1563,15 @@ add_line (struct backtrace_state *state,
struct dwarf_data *ddata,
   return 1;
 }

-/* Free the line header information.  If FREE_FILENAMES is true we
-   free the file names themselves, otherwise we leave them, as there
-   may be line structures pointing to them.  */
+/* Free the line header information.  */

 static void
 free_line_header (struct backtrace_state *state, struct line_header *hdr,
   backtrace_error_callback error_callback, void *data)
 {
-  backtrace_free (state, hdr->dirs, hdr->dirs_count * sizeof (const char *),
-  error_callback, data);
+  if (hdr->dirs_count != 0)
+backtrace_free (state, hdr->dirs, hdr->dirs_count * sizeof (const char *),
+error_callback, data);
   backtrace_free (state, hdr->filenames,
   hdr->filenames_count * sizeof (char *),
   error_callback, data);
@@ -1633,12 +1632,16 @@ read_line_header (struct backtrace_state
*state, struct unit *u,
   ++hdr->dirs_count;
 }

-  hdr->dirs = ((const char **)
-   backtrace_alloc (state,
- hdr->dirs_count * sizeof (const char *),
- line_buf->error_callback, line_buf->data));
-  if (hdr->dirs == NULL)
-return 0;
+  hdr->dirs = NULL;
+  if (hdr->dirs_count != 0)
+{
+  hdr->dirs = ((const char **)
+   backtrace_alloc (state,
+hdr->dirs_count * sizeof (const char *),
+line_buf->error_callback, line_buf->data));
+  if (hdr->dirs == NULL)
+return 0;
+}

   i = 0;
   while (*hdr_buf.buf != '\0')