Re: [PATCH] libdw: Return an error in dwarf_getlocation_attr for missing .debug_addr.

2018-06-10 Thread Mark Wielaard
On Fri, 2018-06-08 at 11:55 +0200, Mark Wielaard wrote:
> When constructing a "fake" Dwarf_Attribute for DW_OP_GNU_const_index,
> DW_OP_constx, DW_OP_GNU_addr_index or DW_OP_addrx, we would create a
> fake attribute pointing to the actual data in the .debug_addr section.
> 
> We would even do that if there was no .debug_addr section assuming
> dwarf_formaddr or dwarf_formudata would generate an error. But when
> there is no .debug_addr there is also no fake_addr_cu, so the
> dwarf_form* functions cannot check the value is correct (and crash).
> 
> Fix by returning an error early from dwarf_getlocation_attr indicating
> bad DWARF data.

Pushed to master.


[PATCH] libdw: Break long or circular DIE ref chains in dwarf_[has]attr_integrate.

2018-06-10 Thread Mark Wielaard
Bad DWARF could create a very long or circular DIE ref chain by linking
DW_AT_abstract_origin or DW_AT_specification to the DIE itself. Break
the chain after seeing a large number (16) of DIEs.

Signed-off-by: Mark Wielaard 
---
 libdw/ChangeLog | 6 ++
 libdw/dwarf_attr_integrate.c| 4 ++--
 libdw/dwarf_hasattr_integrate.c | 4 ++--
 3 files changed, 10 insertions(+), 4 deletions(-)

diff --git a/libdw/ChangeLog b/libdw/ChangeLog
index 1195cf6e..07a1346b 100644
--- a/libdw/ChangeLog
+++ b/libdw/ChangeLog
@@ -1,3 +1,9 @@
+2018-06-10  Mark Wielaard  
+
+   * dwarf_attr_integrate.c (dwarf_attr_integrate): Stop after 16 DIE
+   ref chains.
+   * dwarf_hasattr_integrate.c (dwarf_hasattr_integrate): Likewise.
+
 2018-06-08  Mark Wielaard  
 
* dwarf_getabbrev.c (dwarf_getabbrev): Check die and offset.
diff --git a/libdw/dwarf_attr_integrate.c b/libdw/dwarf_attr_integrate.c
index 748d988d..fc068638 100644
--- a/libdw/dwarf_attr_integrate.c
+++ b/libdw/dwarf_attr_integrate.c
@@ -38,7 +38,7 @@ dwarf_attr_integrate (Dwarf_Die *die, unsigned int 
search_name,
  Dwarf_Attribute *result)
 {
   Dwarf_Die die_mem;
-
+  int chain = 16; /* Largest DIE ref chain we will follow.  */
   do
 {
   Dwarf_Attribute *attr = INTUSE(dwarf_attr) (die, search_name, result);
@@ -53,7 +53,7 @@ dwarf_attr_integrate (Dwarf_Die *die, unsigned int 
search_name,
 
   die = INTUSE(dwarf_formref_die) (attr, &die_mem);
 }
-  while (die != NULL);
+  while (die != NULL && chain-- != 0);
 
   /* Not NULL if it didn't have abstract_origin and specification
  attributes.  If it is a split CU then see if the skeleton
diff --git a/libdw/dwarf_hasattr_integrate.c b/libdw/dwarf_hasattr_integrate.c
index 4d4b4c54..1d946280 100644
--- a/libdw/dwarf_hasattr_integrate.c
+++ b/libdw/dwarf_hasattr_integrate.c
@@ -37,7 +37,7 @@ int
 dwarf_hasattr_integrate (Dwarf_Die *die, unsigned int search_name)
 {
   Dwarf_Die die_mem;
-
+  int chain = 16; /* Largest DIE ref chain we will follow.  */
   do
 {
   if (INTUSE(dwarf_hasattr) (die, search_name))
@@ -53,7 +53,7 @@ dwarf_hasattr_integrate (Dwarf_Die *die, unsigned int 
search_name)
 
   die = INTUSE(dwarf_formref_die) (attr, &die_mem);
 }
-  while (die != NULL);
+  while (die != NULL && chain-- != 0);
 
   /* Not NULL if it didn't have abstract_origin and specification
  attributes.  If it is a split CU then see if the skeleton
-- 
2.17.0



Re: [PATCH] readelf, libdw: Handle too many directories or files in the line table better.

2018-06-10 Thread Mark Wielaard
On Fri, Jun 08, 2018 at 04:06:29PM +0200, Mark Wielaard wrote:
> The afl fuzzer found that the way we handle "too many" directories or files
> in the (DWARF5 style) line table badly. In the case of eu-readelf we would
> print an endless stream of "bad directory" or "bad file". Just stop printing
> when the end of data is reached. In the case of dwarf_getsrclines we would
> allocate a giant amount of memory, even if there was no data to actually
> read in. Sanity check that the directory and file counts seem reasonable
> compared to the amount of data left (assume we need at least 1 byte of
> data per form describing the dirs or files).

Pushed to master.


Re: [PATCH] tests: Fix cfi_debug_bias assert in varlocs.

2018-06-10 Thread Mark Wielaard
On Fri, Jun 08, 2018 at 04:06:55PM +0200, Mark Wielaard wrote:
> It is only a consistency issue if we actually have an cfi_debug and the
> cfi_debug_bias is not zero (because they come from the same file as the
> other debug data).

Pushed to master.


Re: [PATCH] libdw: Detect bad DWARF in store_implicit_value.

2018-06-10 Thread Mark Wielaard
On Fri, Jun 08, 2018 at 04:18:58PM +0200, Mark Wielaard wrote:
> The afl fuzzer running against the varlocs test detected we didn't report
> the value block of a DW_OP_implicit_value consistently when the DWARF was
> bad. Although this doesn't cause a crash it might result in consumers
> using dwarf_getlocation_implicit_value seeing an inconsistent block length
> value. To fix this detect and report bad DWARF data earlier.

Pushed to master.


[PATCH] readelf: Fix bounds check in print_form_data.

2018-06-10 Thread Mark Wielaard
The afl fuzzer found that we did a wrong check in print_form_data when
comparing the remaining bytes in the buffer to an (unsigned) value read.
We were casting the value to ptrdiff_t which is a signed value and so
might turn a really big unsigned value into a negative number. Since we
know the difference between readendp and readp is zero or greater, we
should cast the pointer difference to size_t (and unsigned type) instead
before comparing with the unsigned value.

Signed-off-by: Mark Wielaard 
---
 src/ChangeLog |  5 +
 src/readelf.c | 14 +++---
 2 files changed, 12 insertions(+), 7 deletions(-)

diff --git a/src/ChangeLog b/src/ChangeLog
index 8ebb5fb..6484b9a 100644
--- a/src/ChangeLog
+++ b/src/ChangeLog
@@ -1,3 +1,8 @@
+2018-06-10  Mark Wielaard  
+
+   * readelf.c (print_form_data): Don't cast value to ptrdiff_t, cast
+   ptrdiff_t to size_t.
+
 2018-06-08  Mark Wielaard  
 
* readelf.c (print_debug_rnglists_section): Calculate max_entries
diff --git a/src/readelf.c b/src/readelf.c
index bbaaf96..fbda6c1 100644
--- a/src/readelf.c
+++ b/src/readelf.c
@@ -7880,7 +7880,7 @@ print_form_data (Dwarf *dbg, int form, const unsigned 
char *readp,
   if (readendp - readp < 1)
goto invalid_data;
   get_uleb128 (val, readp, readendp);
-  if (readendp - readp < (ptrdiff_t) val)
+  if ((size_t) (readendp - readp) < val)
goto invalid_data;
   print_bytes (val, readp);
   readp += val;
@@ -7890,7 +7890,7 @@ print_form_data (Dwarf *dbg, int form, const unsigned 
char *readp,
   if (readendp - readp < 1)
goto invalid_data;
   val = *readp++;
-  if (readendp - readp < (ptrdiff_t) val)
+  if ((size_t) (readendp - readp) < val)
goto invalid_data;
   print_bytes (val, readp);
   readp += val;
@@ -7900,7 +7900,7 @@ print_form_data (Dwarf *dbg, int form, const unsigned 
char *readp,
   if (readendp - readp < 2)
goto invalid_data;
   val = read_2ubyte_unaligned_inc (dbg, readp);
-  if (readendp - readp < (ptrdiff_t) val)
+  if ((size_t) (readendp - readp) < val)
goto invalid_data;
   print_bytes (val, readp);
   readp += val;
@@ -7910,7 +7910,7 @@ print_form_data (Dwarf *dbg, int form, const unsigned 
char *readp,
   if (readendp - readp < 2)
goto invalid_data;
   val = read_4ubyte_unaligned_inc (dbg, readp);
-  if (readendp - readp < (ptrdiff_t) val)
+  if ((size_t) (readendp - readp) < val)
goto invalid_data;
   print_bytes (val, readp);
   readp += val;
@@ -7941,7 +7941,7 @@ print_form_data (Dwarf *dbg, int form, const unsigned 
char *readp,
 case DW_FORM_strp:
 case DW_FORM_line_strp:
 case DW_FORM_strp_sup:
-  if (readendp - readp < (ptrdiff_t) offset_len)
+  if ((size_t) (readendp - readp) < offset_len)
goto invalid_data;
   if (offset_len == 8)
val = read_8ubyte_unaligned_inc (dbg, readp);
@@ -7965,7 +7965,7 @@ print_form_data (Dwarf *dbg, int form, const unsigned 
char *readp,
   break;
 
 case DW_FORM_sec_offset:
-  if (readendp - readp < (ptrdiff_t) offset_len)
+  if ((size_t) (readendp - readp) < offset_len)
goto invalid_data;
   if (offset_len == 8)
val = read_8ubyte_unaligned_inc (dbg, readp);
@@ -7988,7 +7988,7 @@ print_form_data (Dwarf *dbg, int form, const unsigned 
char *readp,
{
  readp = data->d_buf + str_offsets_base + val;
  readendp = data->d_buf + data->d_size;
- if (readendp - readp < (ptrdiff_t) offset_len)
+ if ((size_t) (readendp - readp) < offset_len)
str = "???";
  else
{
-- 
1.8.3.1



Re: [PATCH] libdw: dwarf_get_units should handle existing failure to open Dwarf.

2018-06-10 Thread Mark Wielaard
On Fri, 2018-06-08 at 20:45 +0200, Mark Wielaard wrote:
> The other dwarf unit/cu iterators handle a NULL Dwarf handle as an
> existing error and return NULL. Don't crash.

Pushed to master.


Re: [PATCH] libdw: Check validity of dwarf_getabbrev arguments.

2018-06-10 Thread Mark Wielaard
On Fri, 2018-06-08 at 20:47 +0200, Mark Wielaard wrote:
> When the given Dwarf_Die was invalid we might crash and when the offset
> was totally bogus we might succeed with a random abbrev.

Pushed to master.


Re: [PATCH] tests: Don't assert on bad DW_OP_GNU_parameter_ref target in varlocs.

2018-06-10 Thread Mark Wielaard
On Fri, 2018-06-08 at 21:18 +0200, Mark Wielaard wrote:
> If the target of a DW_OP_GNU_parameter_ref isn't a DW_TAG_formal_parameter
> that is bad data (which varlocs should error on). But it isn't an internal
> consistency check (for which varlocs should assert).

Pushed to master.


Re: [PATCH] readelf: Calculate max_entries instead of needed bytes (and overflowing).

2018-06-10 Thread Mark Wielaard
On Fri, 2018-06-08 at 23:33 +0200, Mark Wielaard wrote:
> The afl fuzzer found that we would overflow the needed bytes when
> calculating how many index entries would fit in the .debug_loclists
> and .debug_rnglists tables. To fix this just calculate the max number
> of entries. If the offset entry count is larger than that, do emit
> an error, but print up to max_entries of offsets (so the user can
> more clearly see what is wrong with their table).

Pushed to master.