* Re: [PATCH] libdw: Fix __libdw_form_val_len endp pointer.
@ 2014-12-13 22:36 Josh Stone
0 siblings, 0 replies; 5+ messages in thread
From: Josh Stone @ 2014-12-13 22:36 UTC (permalink / raw)
To: elfutils-devel
[-- Attachment #1: Type: text/plain, Size: 435 bytes --]
On 12/12/2014 07:54 AM, Mark Wielaard wrote:
> @@ -513,6 +515,7 @@ __libdw_form_val_len (Dwarf *dbg, struct Dwarf_CU *cu,
> uint8_t len = form_lengths[form];
> if (len != 0)
> {
> + const const unsigned char *endp = cu->endp;
> len &= 0x7f; /* Mask to allow 0x80 -> 0. */
> if (unlikely (len > (size_t) (endp - valp)))
> {
"const const", for when you're *really* sure it won't ever change!
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] libdw: Fix __libdw_form_val_len endp pointer.
@ 2014-12-14 14:53 Mark Wielaard
0 siblings, 0 replies; 5+ messages in thread
From: Mark Wielaard @ 2014-12-14 14:53 UTC (permalink / raw)
To: elfutils-devel
[-- Attachment #1: Type: text/plain, Size: 1404 bytes --]
On Sat, Dec 13, 2014 at 02:36:08PM -0800, Josh Stone wrote:
> On 12/12/2014 07:54 AM, Mark Wielaard wrote:
> > @@ -513,6 +515,7 @@ __libdw_form_val_len (Dwarf *dbg, struct Dwarf_CU *cu,
> > uint8_t len = form_lengths[form];
> > if (len != 0)
> > {
> > + const const unsigned char *endp = cu->endp;
> > len &= 0x7f; /* Mask to allow 0x80 -> 0. */
> > if (unlikely (len > (size_t) (endp - valp)))
> > {
>
> "const const", for when you're *really* sure it won't ever change!
:) Removed the extra const.
I also made two additional changes to harden the code a bit more when
using "fake" cus. The fake empty cu isn't associated with a DWARF dbg,
which isn't a problem because it is empty, so attributes pointing to it
won't use it to resolve anything. But we might refer to it when interning
the block Dwarf_Ops. Since the block is empty no ops need to be interned
anyway. So short-circuit that path. Also in dwarf_formref_die we would
refer to the data buffer of the form with cu_data (). But that might
be the wrong buffer. Use the cu startp and endp directly and remove
cu_data () completely to prevent any future mistake like that. I haven't
actually found any such a case, because none of the attributes associated
with the fake CUs use DW_FORM_addr. So that is just a "future hardening"
change.
Update patch attached.
Cheers,
Mark
[-- Attachment #2: 0001-libdw-Make-sure-all-attributes-come-with-a-fake-CU-f.patch --]
[-- Type: text/plain, Size: 19581 bytes --]
>From 41d33ebccb7573b6ce277c05db72eb6c4e0f0a41 Mon Sep 17 00:00:00 2001
From: Mark Wielaard <mjw@redhat.com>
Date: Fri, 12 Dec 2014 16:43:04 +0100
Subject: [PATCH] libdw: Make sure all attributes come with a (fake) CU for
bound checks.
All attributes now have a reference to a (fake) CU that has startp and
endp set to the data section where the form data comes from. Use that
for bounds checking in __libdw_form_val_len and dwarf_formblock to make
sure data read doesn't overflow any data section. Remove libdwP.h cu_data
and use cu startp and endp directly where appropriate.
Signed-off-by: Mark Wielaard <mjw@redhat.com>
---
libdw/ChangeLog | 28 +++++++++++++++++++++++++++
libdw/dwarf_begin_elf.c | 22 +++++++++++++++++++++
libdw/dwarf_child.c | 4 +---
libdw/dwarf_end.c | 3 +++
libdw/dwarf_formblock.c | 21 +++++++++++++-------
libdw/dwarf_formref_die.c | 13 ++++++++-----
libdw/dwarf_getattrs.c | 4 +---
libdw/dwarf_getlocation.c | 8 ++++++++
libdw/dwarf_getlocation_attr.c | 29 +++++++++++++++++++++++-----
libdw/dwarf_getlocation_implicit_pointer.c | 8 +++++---
libdw/dwarf_getmacros.c | 5 +++--
libdw/libdwP.h | 31 ++++++++++++++----------------
libdw/libdw_findcu.c | 5 ++---
libdw/libdw_form.c | 12 ++++++------
14 files changed, 139 insertions(+), 54 deletions(-)
diff --git a/libdw/ChangeLog b/libdw/ChangeLog
index 922cc6b..6bb326d 100644
--- a/libdw/ChangeLog
+++ b/libdw/ChangeLog
@@ -1,3 +1,31 @@
+2014-12-12 Mark Wielaard <mjw@redhat.com>
+
+ * libdwP.h (struct Dwarf): Add fake_loc_cu.
+ (cu_data): Removed.
+ (DIE_OFFSET_FROM_CU_OFFSET): Don't use cu_data, use cu_sec_idx.
+ (__libdw_form_val_compute_len): Drop dbg and endp arguments.
+ (__libdw_form_val_len): Likewise.
+ * libdw_form.c (__libdw_form_val_compute_len): Likewise.
+ * libdw_findcu.c (__libdw_intern_next_unit): Don't use cu_data, use
+ the already found data buffer directly.
+ * dwarf_begin_elf.c (valid_p): Setup fake_loc_cu.
+ * dwarf_end.c (dwarf_end): Free fake_loc_cu.
+ * dwarf_child.c (__libdw_find_attr): Call __libdw_form_val_len with
+ just cu.
+ * dwarf_getattrs.c (dwarf_getattrs): Likewise.
+ * dwarf_formblock.c (dwarf_formblock): Add bounds checking.
+ * dwarf_getlocation_attr.c (attr_form_cu): New function.
+ (dwarf_getlocation_attr): Use attr_form_cu to set result->cu.
+ (getlocation): Handle empty blocks immediately.
+ * dwarf_getlocation_implicit_pointer.c (empty_cu): New static var.
+ (__libdw_empty_loc_attr): Drop cu argument, use empty_cu.
+ (dwarf_getlocation_implicit_pointer): Call __libdw_empty_loc_attr with
+ one argument.
+ * dwarf_getmacros.c (read_macros): Also setup startp and endp for
+ fake_cu. Call __libdw_form_val_len with just fake_cu.
+ * dwarf_formref_die.c (dwarf_formref_die): Don't use cu_data, get
+ datap and size directly from cu startp and endp.
+
2014-12-13 Mark Wielaard <mjw@redhat.com>
* dwarf_getaranges.c (compare_aranges): Make sure Dwarf_Addr
diff --git a/libdw/dwarf_begin_elf.c b/libdw/dwarf_begin_elf.c
index 4c6346a..4c49ce2 100644
--- a/libdw/dwarf_begin_elf.c
+++ b/libdw/dwarf_begin_elf.c
@@ -235,6 +235,28 @@ valid_p (Dwarf *result)
result = NULL;
}
+ if (result != NULL && result->sectiondata[IDX_debug_loc] != NULL)
+ {
+ result->fake_loc_cu = (Dwarf_CU *) calloc (1, sizeof (Dwarf_CU));
+ if (unlikely (result->fake_loc_cu == NULL))
+ {
+ __libdw_free_zdata (result);
+ Dwarf_Sig8_Hash_free (&result->sig8_hash);
+ __libdw_seterrno (DWARF_E_NOMEM);
+ free (result);
+ result = NULL;
+ }
+ else
+ {
+ result->fake_loc_cu->dbg = result;
+ result->fake_loc_cu->startp
+ = result->sectiondata[IDX_debug_loc]->d_buf;
+ result->fake_loc_cu->endp
+ = (result->sectiondata[IDX_debug_loc]->d_buf
+ + result->sectiondata[IDX_debug_loc]->d_size);
+ }
+ }
+
return result;
}
diff --git a/libdw/dwarf_child.c b/libdw/dwarf_child.c
index 2a5d379..96d8e23 100644
--- a/libdw/dwarf_child.c
+++ b/libdw/dwarf_child.c
@@ -94,9 +94,7 @@ __libdw_find_attr (Dwarf_Die *die, unsigned int search_name,
/* Skip over the rest of this attribute (if there is any). */
if (attr_form != 0)
{
- size_t len = __libdw_form_val_len (dbg, die->cu, attr_form, readp,
- endp);
-
+ size_t len = __libdw_form_val_len (die->cu, attr_form, readp);
if (unlikely (len == (size_t) -1l))
{
readp = NULL;
diff --git a/libdw/dwarf_end.c b/libdw/dwarf_end.c
index 647a1b8..922dc8f 100644
--- a/libdw/dwarf_end.c
+++ b/libdw/dwarf_end.c
@@ -117,6 +117,9 @@ dwarf_end (dwarf)
if (dwarf->free_elf)
elf_end (dwarf->elf);
+ /* Free the fake location list CU. */
+ free (dwarf->fake_loc_cu);
+
/* Free the context descriptor. */
free (dwarf);
}
diff --git a/libdw/dwarf_formblock.c b/libdw/dwarf_formblock.c
index 799d877..980bc10 100644
--- a/libdw/dwarf_formblock.c
+++ b/libdw/dwarf_formblock.c
@@ -1,5 +1,5 @@
/* Return block represented by attribute.
- Copyright (C) 2004-2010 Red Hat, Inc.
+ Copyright (C) 2004-2010, 2014 Red Hat, Inc.
This file is part of elfutils.
Written by Ulrich Drepper <drepper@redhat.com>, 2004.
@@ -43,28 +43,37 @@ dwarf_formblock (attr, return_block)
if (attr == NULL)
return -1;
- const unsigned char *datap;
+ const unsigned char *datap = attr->valp;
+ const unsigned char *endp = attr->cu->endp;
switch (attr->form)
{
case DW_FORM_block1:
+ if (unlikely (endp - datap < 1))
+ goto invalid;
return_block->length = *(uint8_t *) attr->valp;
return_block->data = attr->valp + 1;
break;
case DW_FORM_block2:
+ if (unlikely (endp - datap < 2))
+ goto invalid;
return_block->length = read_2ubyte_unaligned (attr->cu->dbg, attr->valp);
return_block->data = attr->valp + 2;
break;
case DW_FORM_block4:
+ if (unlikely (endp - datap < 4))
+ goto invalid;
return_block->length = read_4ubyte_unaligned (attr->cu->dbg, attr->valp);
return_block->data = attr->valp + 4;
break;
case DW_FORM_block:
case DW_FORM_exprloc:
- datap = attr->valp;
+ if (unlikely (endp - datap < 1))
+ goto invalid;
+ // XXX bounds check
get_uleb128 (return_block->length, datap);
return_block->data = (unsigned char *) datap;
break;
@@ -74,12 +83,10 @@ dwarf_formblock (attr, return_block)
return -1;
}
- if (unlikely (cu_data (attr->cu)->d_size
- - (return_block->data
- - (unsigned char *) cu_data (attr->cu)->d_buf)
- < return_block->length))
+ if (unlikely (return_block->length > (size_t) (endp - return_block->data)))
{
/* Block does not fit. */
+ invalid:
__libdw_seterrno (DWARF_E_INVALID_DWARF);
return -1;
}
diff --git a/libdw/dwarf_formref_die.c b/libdw/dwarf_formref_die.c
index b54e216..63f6697 100644
--- a/libdw/dwarf_formref_die.c
+++ b/libdw/dwarf_formref_die.c
@@ -70,7 +70,8 @@ dwarf_formref_die (attr, result)
return INTUSE(dwarf_offdie) (dbg_ret, offset, result);
}
- Elf_Data *data;
+ const unsigned char *datap;
+ size_t size;
if (attr->form == DW_FORM_ref_sig8)
{
/* This doesn't have an offset, but instead a value we
@@ -92,7 +93,8 @@ dwarf_formref_die (attr, result)
}
while (cu->type_sig8 != sig);
- data = cu->dbg->sectiondata[IDX_debug_types];
+ datap = cu->dbg->sectiondata[IDX_debug_types]->d_buf;
+ size = cu->dbg->sectiondata[IDX_debug_types]->d_size;
offset = cu->type_offset;
}
else
@@ -101,17 +103,18 @@ dwarf_formref_die (attr, result)
if (unlikely (__libdw_formref (attr, &offset) != 0))
return NULL;
- data = cu_data (cu);
+ datap = cu->startp;
+ size = cu->endp - cu->startp;
}
- if (unlikely (data->d_size - cu->start <= offset))
+ if (unlikely (offset >= size))
{
__libdw_seterrno (DWARF_E_INVALID_DWARF);
return NULL;
}
memset (result, '\0', sizeof (Dwarf_Die));
- result->addr = (char *) data->d_buf + cu->start + offset;
+ result->addr = (char *) datap + offset;
result->cu = cu;
return result;
}
diff --git a/libdw/dwarf_getattrs.c b/libdw/dwarf_getattrs.c
index 9ea70fc..0c54e5d 100644
--- a/libdw/dwarf_getattrs.c
+++ b/libdw/dwarf_getattrs.c
@@ -106,9 +106,7 @@ dwarf_getattrs (Dwarf_Die *die, int (*callback) (Dwarf_Attribute *, void *),
/* Skip over the rest of this attribute (if there is any). */
if (attr.form != 0)
{
- size_t len = __libdw_form_val_len (dbg, die->cu, attr.form,
- die_addr, endp);
-
+ size_t len = __libdw_form_val_len (die->cu, attr.form, die_addr);
if (unlikely (len == (size_t) -1l))
/* Something wrong with the file. */
return -1l;
diff --git a/libdw/dwarf_getlocation.c b/libdw/dwarf_getlocation.c
index 2a4c890..38e93e6 100644
--- a/libdw/dwarf_getlocation.c
+++ b/libdw/dwarf_getlocation.c
@@ -555,6 +555,14 @@ static int
getlocation (struct Dwarf_CU *cu, const Dwarf_Block *block,
Dwarf_Op **llbuf, size_t *listlen, int sec_index)
{
+ /* Empty location expressions don't have any ops to intern.
+ Note that synthetic empty_cu doesn't have an associated DWARF dbg. */
+ if (block->length == 0)
+ {
+ *listlen = 0;
+ return 0;
+ }
+
return __libdw_intern_expression (cu->dbg, cu->dbg->other_byte_order,
cu->address_size, (cu->version == 2
? cu->address_size
diff --git a/libdw/dwarf_getlocation_attr.c b/libdw/dwarf_getlocation_attr.c
index cb29045..3229baf 100644
--- a/libdw/dwarf_getlocation_attr.c
+++ b/libdw/dwarf_getlocation_attr.c
@@ -1,5 +1,5 @@
/* Return DWARF attribute associated with a location expression op.
- Copyright (C) 2013 Red Hat, Inc.
+ Copyright (C) 2013, 2014 Red Hat, Inc.
This file is part of elfutils.
This file is free software; you can redistribute it and/or modify
@@ -33,6 +33,24 @@
#include <dwarf.h>
#include <libdwP.h>
+static Dwarf_CU *
+attr_form_cu (Dwarf_Attribute *attr)
+{
+ /* If the attribute has block/expr form the data comes from the
+ .debug_info from the same cu as the attr. Otherwise it comes from
+ the .debug_loc data section. */
+ switch (attr->form)
+ {
+ case DW_FORM_block1:
+ case DW_FORM_block2:
+ case DW_FORM_block4:
+ case DW_FORM_block:
+ case DW_FORM_exprloc:
+ return attr->cu;
+ default:
+ return attr->cu->dbg->fake_loc_cu;
+ }
+}
int
dwarf_getlocation_attr (attr, op, result)
@@ -43,26 +61,27 @@ dwarf_getlocation_attr (attr, op, result)
if (attr == NULL)
return -1;
- result->cu = attr->cu;
-
switch (op->atom)
{
case DW_OP_implicit_value:
result->code = DW_AT_const_value;
result->form = DW_FORM_block;
result->valp = (unsigned char *) (uintptr_t) op->number2;
+ result->cu = attr_form_cu (attr);
break;
case DW_OP_GNU_entry_value:
result->code = DW_AT_location;
result->form = DW_FORM_exprloc;
result->valp = (unsigned char *) (uintptr_t) op->number2;
+ result->cu = attr_form_cu (attr);
break;
case DW_OP_GNU_const_type:
result->code = DW_AT_const_value;
result->form = DW_FORM_block1;
result->valp = (unsigned char *) (uintptr_t) op->number2;
+ result->cu = attr_form_cu (attr);
break;
case DW_OP_call2:
@@ -74,7 +93,7 @@ dwarf_getlocation_attr (attr, op, result)
return -1;
if (INTUSE(dwarf_attr) (&die, DW_AT_location, result) == NULL)
{
- __libdw_empty_loc_attr (result, attr->cu);
+ __libdw_empty_loc_attr (result);
return 0;
}
}
@@ -88,7 +107,7 @@ dwarf_getlocation_attr (attr, op, result)
if (INTUSE(dwarf_attr) (&die, DW_AT_location, result) == NULL
&& INTUSE(dwarf_attr) (&die, DW_AT_const_value, result) == NULL)
{
- __libdw_empty_loc_attr (result, attr->cu);
+ __libdw_empty_loc_attr (result);
return 0;
}
}
diff --git a/libdw/dwarf_getlocation_implicit_pointer.c b/libdw/dwarf_getlocation_implicit_pointer.c
index f93d17e..f1c16be 100644
--- a/libdw/dwarf_getlocation_implicit_pointer.c
+++ b/libdw/dwarf_getlocation_implicit_pointer.c
@@ -35,15 +35,17 @@
static unsigned char empty_exprloc = 0;
+static Dwarf_CU empty_cu = { .startp = &empty_exprloc,
+ .endp = &empty_exprloc + 1 };
void
internal_function
-__libdw_empty_loc_attr (Dwarf_Attribute *attr, struct Dwarf_CU *cu )
+__libdw_empty_loc_attr (Dwarf_Attribute *attr)
{
attr->code = DW_AT_location;
attr->form = DW_FORM_exprloc;
attr->valp = &empty_exprloc;
- attr->cu = cu;
+ attr->cu = &empty_cu;
}
int
@@ -69,7 +71,7 @@ dwarf_getlocation_implicit_pointer (attr, op, result)
if (INTUSE(dwarf_attr) (&die, DW_AT_location, result) == NULL
&& INTUSE(dwarf_attr) (&die, DW_AT_const_value, result) == NULL)
{
- __libdw_empty_loc_attr (result, attr->cu);
+ __libdw_empty_loc_attr (result);
return 0;
}
diff --git a/libdw/dwarf_getmacros.c b/libdw/dwarf_getmacros.c
index 737dc5d..848128e 100644
--- a/libdw/dwarf_getmacros.c
+++ b/libdw/dwarf_getmacros.c
@@ -354,6 +354,8 @@ read_macros (Dwarf *dbg, int sec_index,
.dbg = dbg,
.version = 4,
.offset_size = table->is_64bit ? 8 : 4,
+ .startp = (void *) startp + offset,
+ .endp = (void *) endp,
};
Dwarf_Attribute attributes[proto->nforms];
@@ -367,8 +369,7 @@ read_macros (Dwarf *dbg, int sec_index,
attributes[i].valp = (void *) readp;
attributes[i].cu = &fake_cu;
- size_t len = __libdw_form_val_len (dbg, &fake_cu,
- proto->forms[i], readp, endp);
+ size_t len = __libdw_form_val_len (&fake_cu, proto->forms[i], readp);
if (len == (size_t) -1)
return -1;
diff --git a/libdw/libdwP.h b/libdw/libdwP.h
index edceb59..eda61a9 100644
--- a/libdw/libdwP.h
+++ b/libdw/libdwP.h
@@ -188,6 +188,10 @@ struct Dwarf
/* Cached info from the CFI section. */
struct Dwarf_CFI_s *cfi;
+ /* Fake loc CU. Used when synthesizing attributes for Dwarf_Ops that
+ came from a location list entry in dwarf_getlocation_attr. */
+ struct Dwarf_CU *fake_loc_cu;
+
/* Internal memory handling. This is basically a simplified
reimplementation of obstacks. Unfortunately the standard obstack
implementation is not usable in libraries. */
@@ -337,7 +341,7 @@ struct Dwarf_CU
((Dwarf_Die) \
{ \
.cu = (fromcu), \
- .addr = ((char *) cu_data (fromcu)->d_buf \
+ .addr = ((char *) fromcu->dbg->sectiondata[cu_sec_idx (fromcu)]->d_buf \
+ DIE_OFFSET_FROM_CU_OFFSET ((fromcu)->start, \
(fromcu)->offset_size, \
(fromcu)->type_offset != 0)) \
@@ -483,18 +487,16 @@ __libdw_dieabbrev (Dwarf_Die *die, const unsigned char **readp)
}
/* Helper functions for form handling. */
-extern size_t __libdw_form_val_compute_len (Dwarf *dbg, struct Dwarf_CU *cu,
+extern size_t __libdw_form_val_compute_len (struct Dwarf_CU *cu,
unsigned int form,
- const unsigned char *valp,
- const unsigned char *endp)
- __nonnull_attribute__ (1, 2, 4, 5) internal_function;
+ const unsigned char *valp)
+ __nonnull_attribute__ (1, 3) internal_function;
/* Find the length of a form attribute. */
static inline size_t
-__nonnull_attribute__ (1, 2, 4, 5)
-__libdw_form_val_len (Dwarf *dbg, struct Dwarf_CU *cu,
- unsigned int form, const unsigned char *valp,
- const unsigned char *endp)
+__nonnull_attribute__ (1, 3)
+__libdw_form_val_len (struct Dwarf_CU *cu, unsigned int form,
+ const unsigned char *valp)
{
/* Small lookup table of forms with fixed lengths. Absent indexes are
initialized 0, so any truly desired 0 is set to 0x80 and masked. */
@@ -513,6 +515,7 @@ __libdw_form_val_len (Dwarf *dbg, struct Dwarf_CU *cu,
uint8_t len = form_lengths[form];
if (len != 0)
{
+ const unsigned char *endp = cu->endp;
len &= 0x7f; /* Mask to allow 0x80 -> 0. */
if (unlikely (len > (size_t) (endp - valp)))
{
@@ -524,7 +527,7 @@ __libdw_form_val_len (Dwarf *dbg, struct Dwarf_CU *cu,
}
/* Other forms require some computation. */
- return __libdw_form_val_compute_len (dbg, cu, form, valp, endp);
+ return __libdw_form_val_compute_len (cu, form, valp);
}
/* Helper function for DW_FORM_ref* handling. */
@@ -721,12 +724,6 @@ cu_sec_idx (struct Dwarf_CU *cu)
return cu->type_offset == 0 ? IDX_debug_info : IDX_debug_types;
}
-static inline Elf_Data *
-cu_data (struct Dwarf_CU *cu)
-{
- return cu->dbg->sectiondata[cu_sec_idx (cu)];
-}
-
/* Read up begin/end pair and increment read pointer.
- If it's normal range record, set up *BEGINP and *ENDP and return 0.
- If it's base address selection record, set up *BASEP and return 1.
@@ -744,7 +741,7 @@ unsigned char * __libdw_formptr (Dwarf_Attribute *attr, int sec_index,
internal_function;
/* Fills in the given attribute to point at an empty location expression. */
-void __libdw_empty_loc_attr (Dwarf_Attribute *attr, struct Dwarf_CU *cu)
+void __libdw_empty_loc_attr (Dwarf_Attribute *attr)
internal_function;
/* Load .debug_line unit at DEBUG_LINE_OFFSET. COMP_DIR is a value of
diff --git a/libdw/libdw_findcu.c b/libdw/libdw_findcu.c
index c783ff8..d8da2e3 100644
--- a/libdw/libdw_findcu.c
+++ b/libdw/libdw_findcu.c
@@ -119,9 +119,8 @@ __libdw_intern_next_unit (dbg, debug_types)
if (debug_types)
Dwarf_Sig8_Hash_insert (&dbg->sig8_hash, type_sig8, newp);
- void *buf = cu_data (newp)->d_buf;
- newp->startp = buf + newp->start;
- newp->endp = buf + newp->end;
+ newp->startp = data->d_buf + newp->start;
+ newp->endp = data->d_buf + newp->end;
/* Add the new entry to the search tree. */
if (tsearch (newp, tree, findcu_cb) == NULL)
diff --git a/libdw/libdw_form.c b/libdw/libdw_form.c
index 65ac7de..379ede5 100644
--- a/libdw/libdw_form.c
+++ b/libdw/libdw_form.c
@@ -39,11 +39,11 @@
size_t
internal_function
-__libdw_form_val_compute_len (Dwarf *dbg, struct Dwarf_CU *cu,
- unsigned int form, const unsigned char *valp,
- const unsigned char *endp)
+__libdw_form_val_compute_len (struct Dwarf_CU *cu, unsigned int form,
+ const unsigned char *valp)
{
const unsigned char *startp = valp;
+ const unsigned char *endp = cu->endp;
Dwarf_Word u128;
size_t result;
@@ -75,13 +75,13 @@ __libdw_form_val_compute_len (Dwarf *dbg, struct Dwarf_CU *cu,
case DW_FORM_block2:
if (unlikely ((size_t) (endp - startp) < 2))
goto invalid;
- result = read_2ubyte_unaligned (dbg, valp) + 2;
+ result = read_2ubyte_unaligned (cu->dbg, valp) + 2;
break;
case DW_FORM_block4:
if (unlikely ((size_t) (endp - startp) < 4))
goto invalid;
- result = read_4ubyte_unaligned (dbg, valp) + 4;
+ result = read_4ubyte_unaligned (cu->dbg, valp) + 4;
break;
case DW_FORM_block:
@@ -112,7 +112,7 @@ __libdw_form_val_compute_len (Dwarf *dbg, struct Dwarf_CU *cu,
case DW_FORM_indirect:
get_uleb128 (u128, valp);
// XXX Is this really correct?
- result = __libdw_form_val_len (dbg, cu, u128, valp, endp);
+ result = __libdw_form_val_len (cu, u128, valp);
if (result != (size_t) -1)
result += valp - startp;
else
--
2.1.0
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] libdw: Fix __libdw_form_val_len endp pointer.
@ 2014-12-12 15:54 Mark Wielaard
0 siblings, 0 replies; 5+ messages in thread
From: Mark Wielaard @ 2014-12-12 15:54 UTC (permalink / raw)
To: elfutils-devel
[-- Attachment #1: Type: text/plain, Size: 2350 bytes --]
On Thu, 2014-12-11 at 15:34 -0800, Josh Stone wrote:
> On 12/11/2014 03:05 PM, Mark Wielaard wrote:
> > While rebasing my work on top of Josh attrabbrev reading speedups
> > I noticed a typo/thinko in my form_val_len bound checking patch.
> >
> > We should check against the die->cu->endp, not the abbrev endp.
>
> In that case, I don't think you need endp as a parameter at all. Just
> use cu->endp as needed, and prevent this mistake from ever happening.
Yes we can. But it is slightly tricky since we have a couple of places
where we have "fake" attributes that get their form data not from
the .debug_info CU data. So we need to make sure those attributes have a
fake cu with the data pointers properly initialized.
We have one "static" empty location expression that we have to give a
static "one byte" fake CU. The new debug macros also have form data,
which comes from a fake debug_macro CU. The somewhat tricky one is
dwarf_getlocation_attr, which gives a fake attribute that represents a
location given an Dwarf_Op. In the case of DW_OP_implicit_value,
DW_OP_GNU_entry_value and DW_OP_GNU_const_type this returns an attribute
that represents a (expression) block. The block data can come from
either the .debug_info cu data or from the location list data
in .debug_loc. For the last case we need a fake loc CU for each Dwarf
that covers the .debug_loc data.
The attached patch implements this. It also adds bounds checking for
dwarf_form_block. Which did actually try to do some checking to see
whether the returned block was completely in the CU. Luckily this check
was wrong and for the above tricky cases not notice the block data was
beyond the .debug_info data. Correcting the check without the proper
fake CUs in place would make some testcases fail.
> Maybe you could also check valp >= cu->startp while you're at it, but
> that's probably an ok invariant to just assume.
I think that is redundant checking in a place where we want to do as
little work as possible. Also that is checked earlier in the code.
> Also along these lines, the dbg parameter could just use cu->dbg as
> needed. Saving parameters probably won't do much for performance in
> this case, nor is changing to a pointer read likely to hurt. It does
> help code clarity though, IMO.
Yes, much nicer to read.
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-libdw-Make-sure-all-attributes-come-with-a-fake-CU-f.patch --]
[-- Type: text/x-patch, Size: 15915 bytes --]
From c93108be2a0257976a16a56a6acac22248efcbfc Mon Sep 17 00:00:00 2001
From: Mark Wielaard <mjw@redhat.com>
Date: Fri, 12 Dec 2014 16:43:04 +0100
Subject: [PATCH] libdw: Make sure all attributes come with a (fake) CU for
bound checks.
All attributes now have a reference to a (fake) CU that has startp and
endp set to the data section where the form data comes from. Use that
for bounds checking in __libdw_form_val_len and dwarf_formblock to make
sure data read doesn't overflow any data section.
Signed-off-by: Mark Wielaard <mjw@redhat.com>
---
libdw/ChangeLog | 21 +++++++++++++++++++++
libdw/dwarf_begin_elf.c | 22 ++++++++++++++++++++++
libdw/dwarf_child.c | 4 +---
libdw/dwarf_end.c | 3 +++
libdw/dwarf_formblock.c | 21 ++++++++++++++-------
libdw/dwarf_getattrs.c | 4 +---
libdw/dwarf_getlocation_attr.c | 29 ++++++++++++++++++++++++-----
libdw/dwarf_getlocation_implicit_pointer.c | 8 +++++---
libdw/dwarf_getmacros.c | 5 +++--
libdw/libdwP.h | 23 +++++++++++++----------
libdw/libdw_form.c | 12 ++++++------
11 files changed, 113 insertions(+), 39 deletions(-)
diff --git a/libdw/ChangeLog b/libdw/ChangeLog
index db4995f..2fe5454 100644
--- a/libdw/ChangeLog
+++ b/libdw/ChangeLog
@@ -1,3 +1,24 @@
+2014-12-12 Mark Wielaard <mjw@redhat.com>
+
+ * libdwP.h (struct Dwarf): Add fake_loc_cu.
+ (__libdw_form_val_compute_len): Drop dbg and endp arguments.
+ (__libdw_form_val_len): Likewise.
+ * libdw_form.c (__libdw_form_val_compute_len): Likewise.
+ * dwarf_begin_elf.c (valid_p): Setup fake_loc_cu.
+ * dwarf_end.c (dwarf_end): Free fake_loc_cu.
+ * dwarf_child.c (__libdw_find_attr): Call __libdw_form_val_len with
+ just cu.
+ * dwarf_getattrs.c (dwarf_getattrs): Likewise.
+ * dwarf_formblock.c (dwarf_formblock): Add bounds checking.
+ * dwarf_getlocation_attr.c (attr_form_cu): New function.
+ (dwarf_getlocation_attr): Use attr_form_cu to set result->cu.
+ * dwarf_getlocation_implicit_pointer.c (empty_cu): New static var.
+ (__libdw_empty_loc_attr): Drop cu argument, use empty_cu.
+ (dwarf_getlocation_implicit_pointer): Call __libdw_empty_loc_attr with
+ one argument.
+ * dwarf_getmacros.c (read_macros): Also setup startp and endp for
+ fake_cu. Call __libdw_form_val_len with just fake_cu.
+
2014-12-11 Mark Wielaard <mjw@redhat.com>
* libdw_findcu.c (__libdw_intern_next_unit): Sanity check offset.
diff --git a/libdw/dwarf_begin_elf.c b/libdw/dwarf_begin_elf.c
index 4c6346a..4c49ce2 100644
--- a/libdw/dwarf_begin_elf.c
+++ b/libdw/dwarf_begin_elf.c
@@ -235,6 +235,28 @@ valid_p (Dwarf *result)
result = NULL;
}
+ if (result != NULL && result->sectiondata[IDX_debug_loc] != NULL)
+ {
+ result->fake_loc_cu = (Dwarf_CU *) calloc (1, sizeof (Dwarf_CU));
+ if (unlikely (result->fake_loc_cu == NULL))
+ {
+ __libdw_free_zdata (result);
+ Dwarf_Sig8_Hash_free (&result->sig8_hash);
+ __libdw_seterrno (DWARF_E_NOMEM);
+ free (result);
+ result = NULL;
+ }
+ else
+ {
+ result->fake_loc_cu->dbg = result;
+ result->fake_loc_cu->startp
+ = result->sectiondata[IDX_debug_loc]->d_buf;
+ result->fake_loc_cu->endp
+ = (result->sectiondata[IDX_debug_loc]->d_buf
+ + result->sectiondata[IDX_debug_loc]->d_size);
+ }
+ }
+
return result;
}
diff --git a/libdw/dwarf_child.c b/libdw/dwarf_child.c
index 2a5d379..96d8e23 100644
--- a/libdw/dwarf_child.c
+++ b/libdw/dwarf_child.c
@@ -94,9 +94,7 @@ __libdw_find_attr (Dwarf_Die *die, unsigned int search_name,
/* Skip over the rest of this attribute (if there is any). */
if (attr_form != 0)
{
- size_t len = __libdw_form_val_len (dbg, die->cu, attr_form, readp,
- endp);
-
+ size_t len = __libdw_form_val_len (die->cu, attr_form, readp);
if (unlikely (len == (size_t) -1l))
{
readp = NULL;
diff --git a/libdw/dwarf_end.c b/libdw/dwarf_end.c
index 647a1b8..922dc8f 100644
--- a/libdw/dwarf_end.c
+++ b/libdw/dwarf_end.c
@@ -117,6 +117,9 @@ dwarf_end (dwarf)
if (dwarf->free_elf)
elf_end (dwarf->elf);
+ /* Free the fake location list CU. */
+ free (dwarf->fake_loc_cu);
+
/* Free the context descriptor. */
free (dwarf);
}
diff --git a/libdw/dwarf_formblock.c b/libdw/dwarf_formblock.c
index 799d877..980bc10 100644
--- a/libdw/dwarf_formblock.c
+++ b/libdw/dwarf_formblock.c
@@ -1,5 +1,5 @@
/* Return block represented by attribute.
- Copyright (C) 2004-2010 Red Hat, Inc.
+ Copyright (C) 2004-2010, 2014 Red Hat, Inc.
This file is part of elfutils.
Written by Ulrich Drepper <drepper@redhat.com>, 2004.
@@ -43,28 +43,37 @@ dwarf_formblock (attr, return_block)
if (attr == NULL)
return -1;
- const unsigned char *datap;
+ const unsigned char *datap = attr->valp;
+ const unsigned char *endp = attr->cu->endp;
switch (attr->form)
{
case DW_FORM_block1:
+ if (unlikely (endp - datap < 1))
+ goto invalid;
return_block->length = *(uint8_t *) attr->valp;
return_block->data = attr->valp + 1;
break;
case DW_FORM_block2:
+ if (unlikely (endp - datap < 2))
+ goto invalid;
return_block->length = read_2ubyte_unaligned (attr->cu->dbg, attr->valp);
return_block->data = attr->valp + 2;
break;
case DW_FORM_block4:
+ if (unlikely (endp - datap < 4))
+ goto invalid;
return_block->length = read_4ubyte_unaligned (attr->cu->dbg, attr->valp);
return_block->data = attr->valp + 4;
break;
case DW_FORM_block:
case DW_FORM_exprloc:
- datap = attr->valp;
+ if (unlikely (endp - datap < 1))
+ goto invalid;
+ // XXX bounds check
get_uleb128 (return_block->length, datap);
return_block->data = (unsigned char *) datap;
break;
@@ -74,12 +83,10 @@ dwarf_formblock (attr, return_block)
return -1;
}
- if (unlikely (cu_data (attr->cu)->d_size
- - (return_block->data
- - (unsigned char *) cu_data (attr->cu)->d_buf)
- < return_block->length))
+ if (unlikely (return_block->length > (size_t) (endp - return_block->data)))
{
/* Block does not fit. */
+ invalid:
__libdw_seterrno (DWARF_E_INVALID_DWARF);
return -1;
}
diff --git a/libdw/dwarf_getattrs.c b/libdw/dwarf_getattrs.c
index 9ea70fc..0c54e5d 100644
--- a/libdw/dwarf_getattrs.c
+++ b/libdw/dwarf_getattrs.c
@@ -106,9 +106,7 @@ dwarf_getattrs (Dwarf_Die *die, int (*callback) (Dwarf_Attribute *, void *),
/* Skip over the rest of this attribute (if there is any). */
if (attr.form != 0)
{
- size_t len = __libdw_form_val_len (dbg, die->cu, attr.form,
- die_addr, endp);
-
+ size_t len = __libdw_form_val_len (die->cu, attr.form, die_addr);
if (unlikely (len == (size_t) -1l))
/* Something wrong with the file. */
return -1l;
diff --git a/libdw/dwarf_getlocation_attr.c b/libdw/dwarf_getlocation_attr.c
index cb29045..3229baf 100644
--- a/libdw/dwarf_getlocation_attr.c
+++ b/libdw/dwarf_getlocation_attr.c
@@ -1,5 +1,5 @@
/* Return DWARF attribute associated with a location expression op.
- Copyright (C) 2013 Red Hat, Inc.
+ Copyright (C) 2013, 2014 Red Hat, Inc.
This file is part of elfutils.
This file is free software; you can redistribute it and/or modify
@@ -33,6 +33,24 @@
#include <dwarf.h>
#include <libdwP.h>
+static Dwarf_CU *
+attr_form_cu (Dwarf_Attribute *attr)
+{
+ /* If the attribute has block/expr form the data comes from the
+ .debug_info from the same cu as the attr. Otherwise it comes from
+ the .debug_loc data section. */
+ switch (attr->form)
+ {
+ case DW_FORM_block1:
+ case DW_FORM_block2:
+ case DW_FORM_block4:
+ case DW_FORM_block:
+ case DW_FORM_exprloc:
+ return attr->cu;
+ default:
+ return attr->cu->dbg->fake_loc_cu;
+ }
+}
int
dwarf_getlocation_attr (attr, op, result)
@@ -43,26 +61,27 @@ dwarf_getlocation_attr (attr, op, result)
if (attr == NULL)
return -1;
- result->cu = attr->cu;
-
switch (op->atom)
{
case DW_OP_implicit_value:
result->code = DW_AT_const_value;
result->form = DW_FORM_block;
result->valp = (unsigned char *) (uintptr_t) op->number2;
+ result->cu = attr_form_cu (attr);
break;
case DW_OP_GNU_entry_value:
result->code = DW_AT_location;
result->form = DW_FORM_exprloc;
result->valp = (unsigned char *) (uintptr_t) op->number2;
+ result->cu = attr_form_cu (attr);
break;
case DW_OP_GNU_const_type:
result->code = DW_AT_const_value;
result->form = DW_FORM_block1;
result->valp = (unsigned char *) (uintptr_t) op->number2;
+ result->cu = attr_form_cu (attr);
break;
case DW_OP_call2:
@@ -74,7 +93,7 @@ dwarf_getlocation_attr (attr, op, result)
return -1;
if (INTUSE(dwarf_attr) (&die, DW_AT_location, result) == NULL)
{
- __libdw_empty_loc_attr (result, attr->cu);
+ __libdw_empty_loc_attr (result);
return 0;
}
}
@@ -88,7 +107,7 @@ dwarf_getlocation_attr (attr, op, result)
if (INTUSE(dwarf_attr) (&die, DW_AT_location, result) == NULL
&& INTUSE(dwarf_attr) (&die, DW_AT_const_value, result) == NULL)
{
- __libdw_empty_loc_attr (result, attr->cu);
+ __libdw_empty_loc_attr (result);
return 0;
}
}
diff --git a/libdw/dwarf_getlocation_implicit_pointer.c b/libdw/dwarf_getlocation_implicit_pointer.c
index f93d17e..f1c16be 100644
--- a/libdw/dwarf_getlocation_implicit_pointer.c
+++ b/libdw/dwarf_getlocation_implicit_pointer.c
@@ -35,15 +35,17 @@
static unsigned char empty_exprloc = 0;
+static Dwarf_CU empty_cu = { .startp = &empty_exprloc,
+ .endp = &empty_exprloc + 1 };
void
internal_function
-__libdw_empty_loc_attr (Dwarf_Attribute *attr, struct Dwarf_CU *cu )
+__libdw_empty_loc_attr (Dwarf_Attribute *attr)
{
attr->code = DW_AT_location;
attr->form = DW_FORM_exprloc;
attr->valp = &empty_exprloc;
- attr->cu = cu;
+ attr->cu = &empty_cu;
}
int
@@ -69,7 +71,7 @@ dwarf_getlocation_implicit_pointer (attr, op, result)
if (INTUSE(dwarf_attr) (&die, DW_AT_location, result) == NULL
&& INTUSE(dwarf_attr) (&die, DW_AT_const_value, result) == NULL)
{
- __libdw_empty_loc_attr (result, attr->cu);
+ __libdw_empty_loc_attr (result);
return 0;
}
diff --git a/libdw/dwarf_getmacros.c b/libdw/dwarf_getmacros.c
index 737dc5d..848128e 100644
--- a/libdw/dwarf_getmacros.c
+++ b/libdw/dwarf_getmacros.c
@@ -354,6 +354,8 @@ read_macros (Dwarf *dbg, int sec_index,
.dbg = dbg,
.version = 4,
.offset_size = table->is_64bit ? 8 : 4,
+ .startp = (void *) startp + offset,
+ .endp = (void *) endp,
};
Dwarf_Attribute attributes[proto->nforms];
@@ -367,8 +369,7 @@ read_macros (Dwarf *dbg, int sec_index,
attributes[i].valp = (void *) readp;
attributes[i].cu = &fake_cu;
- size_t len = __libdw_form_val_len (dbg, &fake_cu,
- proto->forms[i], readp, endp);
+ size_t len = __libdw_form_val_len (&fake_cu, proto->forms[i], readp);
if (len == (size_t) -1)
return -1;
diff --git a/libdw/libdwP.h b/libdw/libdwP.h
index edceb59..fb922f5 100644
--- a/libdw/libdwP.h
+++ b/libdw/libdwP.h
@@ -188,6 +188,10 @@ struct Dwarf
/* Cached info from the CFI section. */
struct Dwarf_CFI_s *cfi;
+ /* Fake loc CU. Used when synthesizing attributes for Dwarf_Ops that
+ came from a location list entry in dwarf_getlocation_attr. */
+ struct Dwarf_CU *fake_loc_cu;
+
/* Internal memory handling. This is basically a simplified
reimplementation of obstacks. Unfortunately the standard obstack
implementation is not usable in libraries. */
@@ -483,18 +487,16 @@ __libdw_dieabbrev (Dwarf_Die *die, const unsigned char **readp)
}
/* Helper functions for form handling. */
-extern size_t __libdw_form_val_compute_len (Dwarf *dbg, struct Dwarf_CU *cu,
+extern size_t __libdw_form_val_compute_len (struct Dwarf_CU *cu,
unsigned int form,
- const unsigned char *valp,
- const unsigned char *endp)
- __nonnull_attribute__ (1, 2, 4, 5) internal_function;
+ const unsigned char *valp)
+ __nonnull_attribute__ (1, 3) internal_function;
/* Find the length of a form attribute. */
static inline size_t
-__nonnull_attribute__ (1, 2, 4, 5)
-__libdw_form_val_len (Dwarf *dbg, struct Dwarf_CU *cu,
- unsigned int form, const unsigned char *valp,
- const unsigned char *endp)
+__nonnull_attribute__ (1, 3)
+__libdw_form_val_len (struct Dwarf_CU *cu, unsigned int form,
+ const unsigned char *valp)
{
/* Small lookup table of forms with fixed lengths. Absent indexes are
initialized 0, so any truly desired 0 is set to 0x80 and masked. */
@@ -513,6 +515,7 @@ __libdw_form_val_len (Dwarf *dbg, struct Dwarf_CU *cu,
uint8_t len = form_lengths[form];
if (len != 0)
{
+ const const unsigned char *endp = cu->endp;
len &= 0x7f; /* Mask to allow 0x80 -> 0. */
if (unlikely (len > (size_t) (endp - valp)))
{
@@ -524,7 +527,7 @@ __libdw_form_val_len (Dwarf *dbg, struct Dwarf_CU *cu,
}
/* Other forms require some computation. */
- return __libdw_form_val_compute_len (dbg, cu, form, valp, endp);
+ return __libdw_form_val_compute_len (cu, form, valp);
}
/* Helper function for DW_FORM_ref* handling. */
@@ -744,7 +747,7 @@ unsigned char * __libdw_formptr (Dwarf_Attribute *attr, int sec_index,
internal_function;
/* Fills in the given attribute to point at an empty location expression. */
-void __libdw_empty_loc_attr (Dwarf_Attribute *attr, struct Dwarf_CU *cu)
+void __libdw_empty_loc_attr (Dwarf_Attribute *attr)
internal_function;
/* Load .debug_line unit at DEBUG_LINE_OFFSET. COMP_DIR is a value of
diff --git a/libdw/libdw_form.c b/libdw/libdw_form.c
index 65ac7de..379ede5 100644
--- a/libdw/libdw_form.c
+++ b/libdw/libdw_form.c
@@ -39,11 +39,11 @@
size_t
internal_function
-__libdw_form_val_compute_len (Dwarf *dbg, struct Dwarf_CU *cu,
- unsigned int form, const unsigned char *valp,
- const unsigned char *endp)
+__libdw_form_val_compute_len (struct Dwarf_CU *cu, unsigned int form,
+ const unsigned char *valp)
{
const unsigned char *startp = valp;
+ const unsigned char *endp = cu->endp;
Dwarf_Word u128;
size_t result;
@@ -75,13 +75,13 @@ __libdw_form_val_compute_len (Dwarf *dbg, struct Dwarf_CU *cu,
case DW_FORM_block2:
if (unlikely ((size_t) (endp - startp) < 2))
goto invalid;
- result = read_2ubyte_unaligned (dbg, valp) + 2;
+ result = read_2ubyte_unaligned (cu->dbg, valp) + 2;
break;
case DW_FORM_block4:
if (unlikely ((size_t) (endp - startp) < 4))
goto invalid;
- result = read_4ubyte_unaligned (dbg, valp) + 4;
+ result = read_4ubyte_unaligned (cu->dbg, valp) + 4;
break;
case DW_FORM_block:
@@ -112,7 +112,7 @@ __libdw_form_val_compute_len (Dwarf *dbg, struct Dwarf_CU *cu,
case DW_FORM_indirect:
get_uleb128 (u128, valp);
// XXX Is this really correct?
- result = __libdw_form_val_len (dbg, cu, u128, valp, endp);
+ result = __libdw_form_val_len (cu, u128, valp);
if (result != (size_t) -1)
result += valp - startp;
else
--
1.8.3.1
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] libdw: Fix __libdw_form_val_len endp pointer.
@ 2014-12-11 23:34 Josh Stone
0 siblings, 0 replies; 5+ messages in thread
From: Josh Stone @ 2014-12-11 23:34 UTC (permalink / raw)
To: elfutils-devel
[-- Attachment #1: Type: text/plain, Size: 2469 bytes --]
On 12/11/2014 03:05 PM, Mark Wielaard wrote:
> While rebasing my work on top of Josh attrabbrev reading speedups
> I noticed a typo/thinko in my form_val_len bound checking patch.
>
> We should check against the die->cu->endp, not the abbrev endp.
In that case, I don't think you need endp as a parameter at all. Just
use cu->endp as needed, and prevent this mistake from ever happening.
Maybe you could also check valp >= cu->startp while you're at it, but
that's probably an ok invariant to just assume.
Also along these lines, the dbg parameter could just use cu->dbg as
needed. Saving parameters probably won't do much for performance in
this case, nor is changing to a pointer read likely to hurt. It does
help code clarity though, IMO.
>
> Signed-off-by: Mark Wielaard <mjw@redhat.com>
> ---
> libdw/ChangeLog | 6 ++++++
> libdw/dwarf_child.c | 2 +-
> libdw/dwarf_getattrs.c | 2 +-
> 3 files changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/libdw/ChangeLog b/libdw/ChangeLog
> index 69592a7..8b00970 100644
> --- a/libdw/ChangeLog
> +++ b/libdw/ChangeLog
> @@ -1,3 +1,9 @@
> +2014-12-11 Mark Wielaard <mjw@redhat.com>
> +
> + * dwarf_child.c (__libdw_find_attr): Call __libdw_form_val_len with
> + die->cu->endp, not the abbrev endp.
> + * dwarf_getattrs.c (dwarf_getattrs): Likewise.
> +
> 2014-12-10 Josh Stone <jistone@redhat.com>
>
> * libdwP.h (Dwarf_CU): Add startp and endp boundaries.
> diff --git a/libdw/dwarf_child.c b/libdw/dwarf_child.c
> index 2a5d379..46d010d 100644
> --- a/libdw/dwarf_child.c
> +++ b/libdw/dwarf_child.c
> @@ -95,7 +95,7 @@ __libdw_find_attr (Dwarf_Die *die, unsigned int search_name,
> if (attr_form != 0)
> {
> size_t len = __libdw_form_val_len (dbg, die->cu, attr_form, readp,
> - endp);
> + die->cu->endp);
>
> if (unlikely (len == (size_t) -1l))
> {
> diff --git a/libdw/dwarf_getattrs.c b/libdw/dwarf_getattrs.c
> index 9ea70fc..f6453c7 100644
> --- a/libdw/dwarf_getattrs.c
> +++ b/libdw/dwarf_getattrs.c
> @@ -107,7 +107,7 @@ dwarf_getattrs (Dwarf_Die *die, int (*callback) (Dwarf_Attribute *, void *),
> if (attr.form != 0)
> {
> size_t len = __libdw_form_val_len (dbg, die->cu, attr.form,
> - die_addr, endp);
> + die_addr, die->cu->endp);
>
> if (unlikely (len == (size_t) -1l))
> /* Something wrong with the file. */
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH] libdw: Fix __libdw_form_val_len endp pointer.
@ 2014-12-11 23:05 Mark Wielaard
0 siblings, 0 replies; 5+ messages in thread
From: Mark Wielaard @ 2014-12-11 23:05 UTC (permalink / raw)
To: elfutils-devel
[-- Attachment #1: Type: text/plain, Size: 1814 bytes --]
While rebasing my work on top of Josh attrabbrev reading speedups
I noticed a typo/thinko in my form_val_len bound checking patch.
We should check against the die->cu->endp, not the abbrev endp.
Signed-off-by: Mark Wielaard <mjw@redhat.com>
---
libdw/ChangeLog | 6 ++++++
libdw/dwarf_child.c | 2 +-
libdw/dwarf_getattrs.c | 2 +-
3 files changed, 8 insertions(+), 2 deletions(-)
diff --git a/libdw/ChangeLog b/libdw/ChangeLog
index 69592a7..8b00970 100644
--- a/libdw/ChangeLog
+++ b/libdw/ChangeLog
@@ -1,3 +1,9 @@
+2014-12-11 Mark Wielaard <mjw@redhat.com>
+
+ * dwarf_child.c (__libdw_find_attr): Call __libdw_form_val_len with
+ die->cu->endp, not the abbrev endp.
+ * dwarf_getattrs.c (dwarf_getattrs): Likewise.
+
2014-12-10 Josh Stone <jistone@redhat.com>
* libdwP.h (Dwarf_CU): Add startp and endp boundaries.
diff --git a/libdw/dwarf_child.c b/libdw/dwarf_child.c
index 2a5d379..46d010d 100644
--- a/libdw/dwarf_child.c
+++ b/libdw/dwarf_child.c
@@ -95,7 +95,7 @@ __libdw_find_attr (Dwarf_Die *die, unsigned int search_name,
if (attr_form != 0)
{
size_t len = __libdw_form_val_len (dbg, die->cu, attr_form, readp,
- endp);
+ die->cu->endp);
if (unlikely (len == (size_t) -1l))
{
diff --git a/libdw/dwarf_getattrs.c b/libdw/dwarf_getattrs.c
index 9ea70fc..f6453c7 100644
--- a/libdw/dwarf_getattrs.c
+++ b/libdw/dwarf_getattrs.c
@@ -107,7 +107,7 @@ dwarf_getattrs (Dwarf_Die *die, int (*callback) (Dwarf_Attribute *, void *),
if (attr.form != 0)
{
size_t len = __libdw_form_val_len (dbg, die->cu, attr.form,
- die_addr, endp);
+ die_addr, die->cu->endp);
if (unlikely (len == (size_t) -1l))
/* Something wrong with the file. */
--
2.1.0
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2014-12-14 14:53 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-12-13 22:36 [PATCH] libdw: Fix __libdw_form_val_len endp pointer Josh Stone
-- strict thread matches above, loose matches on Subject: below --
2014-12-14 14:53 Mark Wielaard
2014-12-12 15:54 Mark Wielaard
2014-12-11 23:34 Josh Stone
2014-12-11 23:05 Mark Wielaard
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).