This replaces the two previously proposed patches for libdwfl/cu.c: libdwfl: Sanity check cu offset before trying to intern. libdwfl: Arange CU cannot point to the EOF marker.

The issue is kind of the same and still didn't fully solve the root cause. This patch makes sure we never insert a bogus DIE nor an EOF marker. It also makes sure that we only tdestroy the lazy_cu_root when we really don't need it anymore.

The attached patch looks big, but ignoring whitespace/indenting changes it is simply:

--- a/libdwfl/cu.c +++ b/libdwfl/cu.c @@ -171,6 +171,29 @@ compare_cukey (const void *a, const void *b) static Dwfl_Error intern_cu (Dwfl_Module *mod, Dwarf_Off cuoff, struct dwfl_cu **result) { + if (unlikely (cuoff + 4 >= mod->dw->sectiondata[IDX_debug_info]->d_size)) + { + if (mod->lazycu == 1) + { + /* This is the EOF marker. Now we have interned all the CUs. + One increment in MOD->lazycu counts not having hit EOF yet. */ + *result = (void *) -1; + less_lazy (mod); + return DWFL_E_NOERROR; + } + else + { + /* Unexpected EOF, most likely a bogus aranges. */ + return (DWFL_E (LIBDW, DWARF_E_INVALID_DWARF)); + } + } + + /* Make sure the cuoff points to a real DIE. */ + Dwarf_Die cudie; + Dwarf_Die *die = INTUSE(dwarf_offdie) (mod->dw, cuoff, &cudie); + if (die == NULL) + return DWFL_E_LIBDW; + struct Dwarf_CU dwkey; struct dwfl_cu key; key.die.cu = &dwkey; @@ -182,16 +205,6 @@ intern_cu (Dwfl_Module *mod, Dwarf_Off cuoff, struct dwfl_cu **result)

if (*found == &key || *found == NULL) { - if (unlikely (cuoff + 4 >= mod->dw->sectiondata[IDX_debug_info]->d_size)) - { - /* This is the EOF marker. Now we have interned all the CUs. - One increment in MOD->lazycu counts not having hit EOF yet. */ - *found = *result = (void *) -1; - less_lazy (mod); - return DWFL_E_NOERROR; - } - else - { /* This is a new entry, meaning we haven't looked at this CU. */

*found = NULL; @@ -203,15 +216,7 @@ intern_cu (Dwfl_Module *mod, Dwarf_Off cuoff, struct dwfl_cu **result) cu->mod = mod; cu->next = NULL; cu->lines = NULL; - - /* XXX use non-searching lookup */ - Dwarf_Die *die = INTUSE(dwarf_offdie) (mod->dw, cuoff, &cu->die); - if (die == NULL) - { - free (cu); - return DWFL_E_LIBDW; - } - assert (die == &cu->die); + cu->die = cudie;

struct dwfl_cu **newvec = realloc (mod->cu, ((mod->ncu + 1) * sizeof (mod->cu[0]))); @@ -228,7 +233,6 @@ intern_cu (Dwfl_Module *mod, Dwarf_Off cuoff, struct dwfl_cu **result)

*found = cu; } - }

*result = *found; return DWFL_E_NOERROR;

<--->

We need to check the cuoff points to a real Dwarf_Die before trying to intern the cu with tsearch. Otherwise bogus keys might end up in the search tree with NULL cus. That will cause crashes in compare_cukey during next insertion or deletion of cus. We also don't want to insert the EOF marker and unconditionally tdestroy the lazy_cu_root. The EOF could be caused by bad DWARF from a bogus agranges entry.

https://bugzilla.redhat.com/show_bug.cgi?id=1170810#c30

Signed-off-by: Mark Wielaard mjw@redhat.com --- libdwfl/ChangeLog | 5 ++++ libdwfl/cu.c | 84 +++++++++++++++++++++++++++++-------------------------- 2 files changed, 49 insertions(+), 40 deletions(-)

diff --git a/libdwfl/ChangeLog b/libdwfl/ChangeLog index de76378..47f3854 100644 --- a/libdwfl/ChangeLog +++ b/libdwfl/ChangeLog @@ -1,3 +1,8 @@ +2015-05-07 Mark Wielaard mjw@redhat.com + + * cu.c (intern_cu): Check for EOF and check cuoff points to a real + Dwarf_Die before interning. Explicitly check EOF is expected. + 2015-05-05 Mark Wielaard mjw@redhat.com

* dwfl_lineinfo.c (dwfl_lineinfo): Check info->file is valid. diff --git a/libdwfl/cu.c b/libdwfl/cu.c index 3ac341e..8f35b6b 100644 --- a/libdwfl/cu.c +++ b/libdwfl/cu.c @@ -171,63 +171,67 @@ compare_cukey (const void *a, const void *b) static Dwfl_Error intern_cu (Dwfl_Module *mod, Dwarf_Off cuoff, struct dwfl_cu **result) { - struct Dwarf_CU dwkey; - struct dwfl_cu key; - key.die.cu = &dwkey; - dwkey.offset_size = 0; - dwkey.start = cuoff - (3 * 0 - 4 + 3); - struct dwfl_cu **found = tsearch (&key, &mod->lazy_cu_root, &compare_cukey); - if (unlikely (found == NULL)) - return DWFL_E_NOMEM; - - if (*found == &key || *found == NULL) + if (unlikely (cuoff + 4 >= mod->dw->sectiondata[IDX_debug_info]->d_size)) { - if (unlikely (cuoff + 4 >= mod->dw->sectiondata[IDX_debug_info]->d_size)) + if (mod->lazycu == 1) { /* This is the EOF marker. Now we have interned all the CUs. One increment in MOD->lazycu counts not having hit EOF yet. */ - *found = *result = (void *) -1; + *result = (void *) -1; less_lazy (mod); return DWFL_E_NOERROR; } else { - /* This is a new entry, meaning we haven't looked at this CU. */ + /* Unexpected EOF, most likely a bogus aranges. */ + return (DWFL_E (LIBDW, DWARF_E_INVALID_DWARF)); + } + }

- *found = NULL; + /* Make sure the cuoff points to a real DIE. */ + Dwarf_Die cudie; + Dwarf_Die *die = INTUSE(dwarf_offdie) (mod->dw, cuoff, &cudie); + if (die == NULL) + return DWFL_E_LIBDW;

- struct dwfl_cu *cu = malloc (sizeof *cu); - if (unlikely (cu == NULL)) - return DWFL_E_NOMEM; + struct Dwarf_CU dwkey; + struct dwfl_cu key; + key.die.cu = &dwkey; + dwkey.offset_size = 0; + dwkey.start = cuoff - (3 * 0 - 4 + 3); + struct dwfl_cu **found = tsearch (&key, &mod->lazy_cu_root, &compare_cukey); + if (unlikely (found == NULL)) + return DWFL_E_NOMEM;

- cu->mod = mod; - cu->next = NULL; - cu->lines = NULL; + if (*found == &key || *found == NULL) + { + /* This is a new entry, meaning we haven't looked at this CU. */

- /* XXX use non-searching lookup */ - Dwarf_Die *die = INTUSE(dwarf_offdie) (mod->dw, cuoff, &cu->die); - if (die == NULL) - { - free (cu); - return DWFL_E_LIBDW; - } - assert (die == &cu->die); + *found = NULL;

- struct dwfl_cu **newvec = realloc (mod->cu, ((mod->ncu + 1) - * sizeof (mod->cu[0]))); - if (newvec == NULL) - { - free (cu); - return DWFL_E_NOMEM; - } - mod->cu = newvec; + struct dwfl_cu *cu = malloc (sizeof *cu); + if (unlikely (cu == NULL)) + return DWFL_E_NOMEM;

- mod->cu[mod->ncu++] = cu; - if (cu->die.cu->start == 0) - mod->first_cu = cu; + cu->mod = mod; + cu->next = NULL; + cu->lines = NULL; + cu->die = cudie;

- *found = cu; + struct dwfl_cu **newvec = realloc (mod->cu, ((mod->ncu + 1) + * sizeof (mod->cu[0]))); + if (newvec == NULL) + { + free (cu); + return DWFL_E_NOMEM; } + mod->cu = newvec; + + mod->cu[mod->ncu++] = cu; + if (cu->die.cu->start == 0) + mod->first_cu = cu; + + *found = cu; }

*result = *found;