[PATCH] gdb/dwarf2read.c: Sanity check DW_AT_sibling values.

[PATCH] gdb/dwarf2read.c: Sanity check DW_AT_sibling values.

[Will Newton]

When reading objects with corrupt debug information it is possible that
the sibling chain can form a loop, which leads to an infinite loop and
memory exhaustion.

Avoid this situation by disregarding and DW_AT_sibling values that point
to a lower address than the current entry.

gdb/ChangeLog:

2013-11-01  Will Newton  <will.newton@linaro.org>

	PR gdb/12866
	* dwarf2read.c (skip_one_die): Sanity check DW_AT_sibling
	values.  (read_partial_die): Likewise.
---
 gdb/dwarf2read.c | 18 ++++++++++++++++--
 1 file changed, 16 insertions(+), 2 deletions(-)

diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
index 3974d0b..d4dfd45 100644
--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -7016,7 +7016,14 @@ skip_one_die (const struct die_reader_specs *reader, const gdb_byte *info_ptr,
 	    complaint (&symfile_complaints,
 		       _("ignoring absolute DW_AT_sibling"));
 	  else
-	    return buffer + dwarf2_get_ref_die_offset (&attr).sect_off;
+	    {
+	      const gdb_byte *sibling_ptr = buffer + dwarf2_get_ref_die_offset (&attr).sect_off;
+	      if (sibling_ptr < info_ptr)
+		complaint (&symfile_complaints,
+			   _("DW_AT_sibling points backwards"));
+	      else
+		return buffer + dwarf2_get_ref_die_offset (&attr).sect_off;
+	    }
 	}

       /* If it isn't DW_AT_sibling, skip this attribute.  */
@@ -15134,7 +15141,14 @@ read_partial_die (const struct die_reader_specs *reader,
 	    complaint (&symfile_complaints,
 		       _("ignoring absolute DW_AT_sibling"));
 	  else
-	    part_die->sibling = buffer + dwarf2_get_ref_die_offset (&attr).sect_off;
+	    {
+	      const gdb_byte *sibling_ptr = buffer + dwarf2_get_ref_die_offset (&attr).sect_off;
+	      if (sibling_ptr < info_ptr)
+		complaint (&symfile_complaints,
+			   _("DW_AT_sibling points backwards"));
+	      else
+		part_die->sibling = sibling_ptr;
+	    }
 	  break;
         case DW_AT_byte_size:
           part_die->has_byte_size = 1;
-- 
1.8.1.4

Re: [PATCH] gdb/dwarf2read.c: Sanity check DW_AT_sibling values.

[Tom Tromey]

>>>>> "Will" == Will Newton <will.newton@linaro.org> writes:

Will> When reading objects with corrupt debug information it is possible that
Will> the sibling chain can form a loop, which leads to an infinite loop and
Will> memory exhaustion.

Will> Avoid this situation by disregarding and DW_AT_sibling values that point
Will> to a lower address than the current entry.

Thanks for doing this.

Will> +	      const gdb_byte *sibling_ptr = buffer + dwarf2_get_ref_die_offset (&attr).sect_off;

This line is too long, it should be split somewhere.

Will> +	      if (sibling_ptr < info_ptr)
Will> +		complaint (&symfile_complaints,
Will> +			   _("DW_AT_sibling points backwards"));

I wonder whether the check should be "<=".

Will> +	      const gdb_byte *sibling_ptr = buffer + dwarf2_get_ref_die_offset (&attr).sect_off;

Also too long.

Tom

Re: [PATCH] gdb/dwarf2read.c: Sanity check DW_AT_sibling values.

[Will Newton]

On 4 November 2013 15:57, Tom Tromey <tromey@redhat.com> wrote:
>>>>>> "Will" == Will Newton <will.newton@linaro.org> writes:
>
> Will> When reading objects with corrupt debug information it is possible that
> Will> the sibling chain can form a loop, which leads to an infinite loop and
> Will> memory exhaustion.
>
> Will> Avoid this situation by disregarding and DW_AT_sibling values that point
> Will> to a lower address than the current entry.
>
> Thanks for doing this.
>
> Will> +       const gdb_byte *sibling_ptr = buffer + dwarf2_get_ref_die_offset (&attr).sect_off;
>
> This line is too long, it should be split somewhere.

Thanks, I'll fix these.

> Will> +       if (sibling_ptr < info_ptr)
> Will> +         complaint (&symfile_complaints,
> Will> +                    _("DW_AT_sibling points backwards"));
>
> I wonder whether the check should be "<=".

I'm not sure. It looks to me that info_ptr at this point will point to
the next attribute/DIE which could be a valid sibling?

-- 
Will Newton
Toolchain Working Group, Linaro

Re: [PATCH] gdb/dwarf2read.c: Sanity check DW_AT_sibling values.

[Tom Tromey]

>>>>> "Will" == Will Newton <will.newton@linaro.org> writes:

Tom> I wonder whether the check should be "<=".

Will> I'm not sure. It looks to me that info_ptr at this point will point to
Will> the next attribute/DIE which could be a valid sibling?

Yeah, you're right.
Thanks.

Tom