* Re: [PATCH] libdwfl: Only intern CU when not EOF marker and cuoff points to a DIE.
@ 2015-05-12 14:55 Mark Wielaard
0 siblings, 0 replies; 2+ messages in thread
From: Mark Wielaard @ 2015-05-12 14:55 UTC (permalink / raw)
To: elfutils-devel
[-- Attachment #1: Type: text/plain, Size: 752 bytes --]
On Thu, 2015-05-07 at 19:16 +0200, Mark Wielaard wrote:
> 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.
>
> <--->
>
> 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.
Pushed to master, wrapping the new tests in likely/unlikely.
^ permalink raw reply [flat|nested] 2+ messages in thread
* [PATCH] libdwfl: Only intern CU when not EOF marker and cuoff points to a DIE.
@ 2015-05-07 17:16 Mark Wielaard
0 siblings, 0 replies; 2+ messages in thread
From: Mark Wielaard @ 2015-05-07 17:16 UTC (permalink / raw)
To: elfutils-devel
[-- Attachment #1: Type: text/plain, Size: 7408 bytes --]
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;
--
2.1.0
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2015-05-12 14:55 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-05-12 14:55 [PATCH] libdwfl: Only intern CU when not EOF marker and cuoff points to a DIE Mark Wielaard
-- strict thread matches above, loose matches on Subject: below --
2015-05-07 17:16 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).