[PATCH] libdw, readelf: Don't handle DW_FORM_data16 as expression block/location.

[PATCH] libdw, readelf: Don't handle DW_FORM_data16 as expression block/location.

[Mark Wielaard]

Also found by afl-fuzz on the varlocs testcase.
DW_FORM_data16 is constant from according to the DWARF5 spec.
But since it is 128bits it isn't really representable as Dwarf_Word.
So we treat it as block form. But we cannot treat it as an expression
block. Make sure readelf prints it as a regular block and that
dwarf_getlocation[s|_addr] doesn't treat it as location expression.

Signed-off-by: Mark Wielaard <mark@klomp.org>
---
 libdw/dwarf_getlocation.c | 44 +++++++++++++++++++++++++++++++-------------
 src/ChangeLog             |  5 +++++
 src/readelf.c             | 15 ++++++++++-----
 3 files changed, 46 insertions(+), 18 deletions(-)

diff --git a/libdw/dwarf_getlocation.c b/libdw/dwarf_getlocation.c
index 7f294fe..fc59a2a 100644
--- a/libdw/dwarf_getlocation.c
+++ b/libdw/dwarf_getlocation.c
@@ -174,6 +174,8 @@ check_constant_offset (Dwarf_Attribute *attr,
     default:
       return 1;
 
+      /* Note, we don't regard DW_FORM_data16 as a constant form,
+	 even though technically it is according to the standard.  */
     case DW_FORM_data1:
     case DW_FORM_data2:
     case DW_FORM_data4:
@@ -665,7 +667,13 @@ dwarf_getlocation (Dwarf_Attribute *attr, Dwarf_Op **llbuf, size_t *listlen)
   if (result != 1)
     return result;
 
-  /* If it has a block form, it's a single location expression.  */
+  /* If it has a block form, it's a single location expression.
+     Except for DW_FORM_data16, which is a 128bit constant.  */
+  if (attr->form == DW_FORM_data16)
+    {
+      __libdw_seterrno (DWARF_E_NO_BLOCK);
+      return -1;
+    }
   Dwarf_Block block;
   if (INTUSE(dwarf_formblock) (attr, &block) != 0)
     return -1;
@@ -863,9 +871,11 @@ dwarf_getlocation_addr (Dwarf_Attribute *attr, Dwarf_Addr address,
   if (llbufs == NULL)
     maxlocs = SIZE_MAX;
 
-  /* If it has a block form, it's a single location expression.  */
+  /* If it has a block form, it's a single location expression.
+     Except for DW_FORM_data16, which is a 128bit constant.  */
   Dwarf_Block block;
-  if (INTUSE(dwarf_formblock) (attr, &block) == 0)
+  if (attr->form != DW_FORM_data16
+      && INTUSE(dwarf_formblock) (attr, &block) == 0)
     {
       if (maxlocs == 0)
 	return 0;
@@ -876,11 +886,14 @@ dwarf_getlocation_addr (Dwarf_Attribute *attr, Dwarf_Addr address,
       return listlens[0] == 0 ? 0 : 1;
     }
 
-  int error = INTUSE(dwarf_errno) ();
-  if (unlikely (error != DWARF_E_NO_BLOCK))
+  if (attr->form != DW_FORM_data16)
     {
-      __libdw_seterrno (error);
-      return -1;
+      int error = INTUSE(dwarf_errno) ();
+      if (unlikely (error != DWARF_E_NO_BLOCK))
+	{
+	  __libdw_seterrno (error);
+	  return -1;
+	}
     }
 
   int result = check_constant_offset (attr, &llbufs[0], &listlens[0]);
@@ -938,9 +951,11 @@ dwarf_getlocations (Dwarf_Attribute *attr, ptrdiff_t offset, Dwarf_Addr *basep,
 
   if (offset == 0)
     {
-      /* If it has a block form, it's a single location expression.  */
+      /* If it has a block form, it's a single location expression.
+	 Except for DW_FORM_data16, which is a 128bit constant.  */
       Dwarf_Block block;
-      if (INTUSE(dwarf_formblock) (attr, &block) == 0)
+      if (attr->form != DW_FORM_data16
+	  && INTUSE(dwarf_formblock) (attr, &block) == 0)
 	{
 	  if (getlocation (attr->cu, &block, expr, exprlen,
 			   cu_sec_idx (attr->cu)) != 0)
@@ -952,11 +967,14 @@ dwarf_getlocations (Dwarf_Attribute *attr, ptrdiff_t offset, Dwarf_Addr *basep,
 	  return 1;
 	}
 
-      int error = INTUSE(dwarf_errno) ();
-      if (unlikely (error != DWARF_E_NO_BLOCK))
+      if (attr->form != DW_FORM_data16)
 	{
-	  __libdw_seterrno (error);
-	  return -1;
+	  int error = INTUSE(dwarf_errno) ();
+	  if (unlikely (error != DWARF_E_NO_BLOCK))
+	    {
+	      __libdw_seterrno (error);
+	      return -1;
+	    }
 	}
 
       int result = check_constant_offset (attr, expr, exprlen);
diff --git a/src/ChangeLog b/src/ChangeLog
index 8fb418a..9b119b6 100644
--- a/src/ChangeLog
+++ b/src/ChangeLog
@@ -1,3 +1,8 @@
+2018-06-15  Mark Wielaard  <mark@klomp.org>
+
+	* readelf.c (attr_callback): Only print block as expressions if it
+	isn't DW_FORM_data16.
+
 2018-06-16  Mark Wielaard  <mark@klomp.org>
 
 	* readelf.c (print_debug_loc_section): Make sure next_off doesn't
diff --git a/src/readelf.c b/src/readelf.c
index 2e7378e..4172046 100644
--- a/src/readelf.c
+++ b/src/readelf.c
@@ -7483,11 +7483,16 @@ attr_callback (Dwarf_Attribute *attrp, void *arg)
 	case DW_AT_GNU_call_site_data_value:
 	case DW_AT_GNU_call_site_target:
 	case DW_AT_GNU_call_site_target_clobbered:
-	  putchar ('\n');
-	  print_ops (cbargs->dwflmod, cbargs->dbg,
-		     12 + level * 2, 12 + level * 2,
-		     cbargs->version, cbargs->addrsize, cbargs->offset_size,
-		     attrp->cu, block.length, block.data);
+	  if (form != DW_FORM_data16)
+	    {
+	      putchar ('\n');
+	      print_ops (cbargs->dwflmod, cbargs->dbg,
+			 12 + level * 2, 12 + level * 2,
+			 cbargs->version, cbargs->addrsize, cbargs->offset_size,
+			 attrp->cu, block.length, block.data);
+	    }
+	  else
+	    print_block (block.length, block.data);
 	  break;
 	}
       break;
-- 
1.8.3.1

Re: [PATCH] libdw, readelf: Don't handle DW_FORM_data16 as expression block/location.

[Mark Wielaard]

On Fri, Jun 15, 2018 at 04:30:14PM +0200, Mark Wielaard wrote:
> Also found by afl-fuzz on the varlocs testcase.
> DW_FORM_data16 is constant from according to the DWARF5 spec.
> But since it is 128bits it isn't really representable as Dwarf_Word.
> So we treat it as block form. But we cannot treat it as an expression
> block. Make sure readelf prints it as a regular block and that
> dwarf_getlocation[s|_addr] doesn't treat it as location expression.

Pushed to master.