* [RFA] Fix gdb/15827 (crash w/corrupt DWARF)
@ 2014-03-21 16:48 Keith Seitz
2014-03-21 17:14 ` Joel Brobecker
0 siblings, 1 reply; 6+ messages in thread
From: Keith Seitz @ 2014-03-21 16:48 UTC (permalink / raw)
To: gdb-patches@sourceware.org ml
[-- Attachment #1: Type: text/plain, Size: 1961 bytes --]
Hi,
I think most of us know that the dwarf2read.c pretty much assumes that
the DWARF input is okay. The reporter of this bug used a binary fuzzer
to corrupt the DWARF info:
<1><569>: Abbrev Number: 16 (DW_TAG_enumeration_type)
<56a> DW_AT_byte_size : 4
<56b> DW_AT_decl_file : 17
<56c> DW_AT_decl_line : 73
<56d> DW_AT_sibling : <0xb12>
<2><571>: Abbrev Number: 17 (DW_TAG_enumerator)
<572> DW_AT_name : (indirect string, offset: 0xf14):
_SC_ARG_MAX
<576> DW_AT_const_value : 0
[snip]
<2><a8c>: Abbrev Number: 17 (DW_TAG_enumerator)
<a8d> DW_AT_name : (indirect string, offset: 0x9b7):
_SC_LEVEL3_CACHE_ASSOC
<a91> DW_AT_const_value : 2243
<2><a93>: Abbrev Number: 12 (DW_TAG_array_type)
<a94> DW_AT_type : <0x114>
<a98> DW_AT_sibling : <0x761101c4>
<3><a9c>: Abbrev Number: 10 (DW_TAG_member)
<a9d> DW_AT_name : (indirect string, offset: 0x1c50000):
<offset is
too big>
<aa1> DW_AT_decl_file : 17
<aa2> DW_AT_decl_line : 2539
<aa4> DW_AT_type : <0x1c60000>
<aa8> DW_AT_data_member_location: 17
As you can see, the sibling for DIE 0xa93 points of to la-la land. This
causes skip_one_die to crash, since it never validates whether the
offset of the sibling is contained within the current readers's input
buffer. This function does check, however, whether the offset is negative.
This patch essentially adds the counterpoint check and a beginning at
some tests for catch-all "corrupt" DWARF.
Keith
ChangeLog
2014-03-20 Keith Seitz <keiths@redhat.com>
PR gdb/15827
* dwarf2read.c (skip_one_die): Check that all relative-offset
sibling DIEs fall within range of the current reader's buffer.
(read_partial_die): Likewise.
testsuite/ChangeLog
2014-03-20 Keith Seitz <keiths@redhat.com>
PR gdb/15827
* gdb.dwarf2/corrupt.c: New file.
* gdb.dwarf2/corrupt.exp: New file.
[-- Attachment #2: 15827.patch --]
[-- Type: text/x-patch, Size: 4037 bytes --]
diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
index 705dc2d..c30b1b3 100644
--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -7103,6 +7103,8 @@ skip_one_die (const struct die_reader_specs *reader, const gdb_byte *info_ptr,
if (sibling_ptr < info_ptr)
complaint (&symfile_complaints,
_("DW_AT_sibling points backwards"));
+ else if (sibling_ptr > reader->buffer_end)
+ dwarf2_section_buffer_overflow_complaint (reader->die_section);
else
return sibling_ptr;
}
@@ -15416,6 +15418,8 @@ read_partial_die (const struct die_reader_specs *reader,
if (sibling_ptr < info_ptr)
complaint (&symfile_complaints,
_("DW_AT_sibling points backwards"));
+ else if (sibling_ptr > reader->buffer_end)
+ dwarf2_section_buffer_overflow_complaint (reader->die_section);
else
part_die->sibling = sibling_ptr;
}
diff --git a/gdb/testsuite/gdb.dwarf2/corrupt.c b/gdb/testsuite/gdb.dwarf2/corrupt.c
new file mode 100644
index 0000000..4120f3d
--- /dev/null
+++ b/gdb/testsuite/gdb.dwarf2/corrupt.c
@@ -0,0 +1,24 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+ Copyright 2014 Free Software Foundation, Inc.
+
+ This program is free software; you can redistribute it and/or modify
+ it under the terms of the GNU General Public License as published by
+ the Free Software Foundation; either version 3 of the License, or
+ (at your option) any later version.
+
+ This program is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ GNU General Public License for more details.
+
+ You should have received a copy of the GNU General Public License
+ along with this program. If not, see <http://www.gnu.org/licenses/>. */
+
+/* Dummy main function. */
+
+int
+main()
+{
+ return 0;
+}
diff --git a/gdb/testsuite/gdb.dwarf2/corrupt.exp b/gdb/testsuite/gdb.dwarf2/corrupt.exp
new file mode 100644
index 0000000..ba5e551
--- /dev/null
+++ b/gdb/testsuite/gdb.dwarf2/corrupt.exp
@@ -0,0 +1,78 @@
+# Copyright 2014 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program. If not, see <http://www.gnu.org/licenses/>.
+
+# Test corrupt DWARF input
+# PR gdb/15827
+
+load_lib dwarf.exp
+
+if {![dwarf2_support]} {
+ return 0
+}
+
+standard_testfile corrupt.c corrupt.S
+
+# Make the DWARF used for the test.
+#
+# Here we put DW_AT_sibling DIEs into the output which
+# point off into la-la land. The whole purpose is to simulate
+# corrupt DWARF information and make sure that GDB can handle it
+# without crashing.
+
+set asm_file [standard_output_file $srcfile2]
+Dwarf::assemble $asm_file {
+ cu {} {
+ compile_unit {} {
+ declare_labels int_label
+
+ int_label: base_type {
+ {byte_size 4}
+ {name "int"}
+ }
+
+ enumeration_type {
+ {name "ENUM"}
+ {byte_size 4}
+ } {
+ enumerator {
+ {name "A"}
+ {const_value 0}
+ }
+ enumerator {
+ {name "B"}
+ {const_value 1}
+ {sibling 12345678 DW_FORM_ref4}
+ } {
+ base_type {
+ {byte_size 1}
+ {name "char"}
+ }
+ }
+ array_type {
+ {type :$int_label}
+ {sibling 12345678 DW_FORM_ref4}
+ }
+ }
+ }
+ }
+}
+
+if {[prepare_for_testing $testfile.exp $testfile \
+ [list $srcfile $asm_file] {nodebug}]} {
+ return -1
+}
+
+# If we get here and gdb hasn't crashed, the tests pass.
+pass "corrupt DWARF"
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RFA] Fix gdb/15827 (crash w/corrupt DWARF)
2014-03-21 16:48 [RFA] Fix gdb/15827 (crash w/corrupt DWARF) Keith Seitz
@ 2014-03-21 17:14 ` Joel Brobecker
2014-03-21 17:26 ` Keith Seitz
2014-03-21 19:27 ` Doug Evans
0 siblings, 2 replies; 6+ messages in thread
From: Joel Brobecker @ 2014-03-21 17:14 UTC (permalink / raw)
To: Keith Seitz; +Cc: gdb-patches@sourceware.org ml
Hi Keith,
> ChangeLog
> 2014-03-20 Keith Seitz <keiths@redhat.com>
>
> PR gdb/15827
> * dwarf2read.c (skip_one_die): Check that all relative-offset
> sibling DIEs fall within range of the current reader's buffer.
> (read_partial_die): Likewise.
>
> testsuite/ChangeLog
> 2014-03-20 Keith Seitz <keiths@redhat.com>
>
> PR gdb/15827
> * gdb.dwarf2/corrupt.c: New file.
> * gdb.dwarf2/corrupt.exp: New file.
LGTM! Just one tiny nit...
> +/* Dummy main function. */
> +
> +int
> +main()
Use "(void)" instead of "()". There was a recent policy clarification
regarding the CS to be using with testcases, and basically we decided
to try to follow the GCS as much as we reasonably could.
> +}
> +
> +# If we get here and gdb hasn't crashed, the tests pass.
> +pass "corrupt DWARF"
That's just me but I usually do a "print 1" test, just to make sure
that even if the testing framework did not detect the GDB process
dying, the "print 1" test definitely will. Not important on most,
if not all platforms, but ISTR some odd platforms where this helped.
That's just a suggestion, you don't have to follow it.
--
Joel
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RFA] Fix gdb/15827 (crash w/corrupt DWARF)
2014-03-21 17:14 ` Joel Brobecker
@ 2014-03-21 17:26 ` Keith Seitz
2014-03-21 17:54 ` Joel Brobecker
2014-03-21 19:27 ` Doug Evans
1 sibling, 1 reply; 6+ messages in thread
From: Keith Seitz @ 2014-03-21 17:26 UTC (permalink / raw)
To: Joel Brobecker; +Cc: gdb-patches@sourceware.org ml
[-- Attachment #1: Type: text/plain, Size: 1509 bytes --]
Hi, Joel!
Thank you for having a look at this.
On 03/21/2014 10:14 AM, Joel Brobecker wrote:
> Use "(void)" instead of "()". There was a recent policy clarification
> regarding the CS to be using with testcases, and basically we decided
> to try to follow the GCS as much as we reasonably could.
Cut-n-paste-o. Fixed.
>> +# If we get here and gdb hasn't crashed, the tests pass.
>> +pass "corrupt DWARF"
>
> That's just me but I usually do a "print 1" test, just to make sure
> that even if the testing framework did not detect the GDB process
> dying, the "print 1" test definitely will. Not important on most,
> if not all platforms, but ISTR some odd platforms where this helped.
> That's just a suggestion, you don't have to follow it.
Actually, I think that's a very good idea (which did not occur to me).
My big hesitation with this is that this failure gets reported as
UNRESOLVED. While I try to be studious about checking
XFAIL/UNRESOLVED/ERROR, I sometimes overlook these in favor of a raw
PASS/FAIL check in gdb.sum.
I've attached a revision with those two changes (ChangeLog remains
unchanged).
Keith
ChangeLog
2014-03-20 Keith Seitz <keiths@redhat.com>
PR gdb/15827
* dwarf2read.c (skip_one_die): Check that all relative-offset
sibling DIEs fall within range of the current reader's buffer.
(read_partial_die): Likewise.
testsuite/ChangeLog
2014-03-20 Keith Seitz <keiths@redhat.com>
PR gdb/15827
* gdb.dwarf2/corrupt.c: New file.
* gdb.dwarf2/corrupt.exp: New file.
[-- Attachment #2: 15827-2.patch --]
[-- Type: text/x-patch, Size: 4017 bytes --]
diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
index 705dc2d..c30b1b3 100644
--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -7103,6 +7103,8 @@ skip_one_die (const struct die_reader_specs *reader, const gdb_byte *info_ptr,
if (sibling_ptr < info_ptr)
complaint (&symfile_complaints,
_("DW_AT_sibling points backwards"));
+ else if (sibling_ptr > reader->buffer_end)
+ dwarf2_section_buffer_overflow_complaint (reader->die_section);
else
return sibling_ptr;
}
@@ -15416,6 +15418,8 @@ read_partial_die (const struct die_reader_specs *reader,
if (sibling_ptr < info_ptr)
complaint (&symfile_complaints,
_("DW_AT_sibling points backwards"));
+ else if (sibling_ptr > reader->buffer_end)
+ dwarf2_section_buffer_overflow_complaint (reader->die_section);
else
part_die->sibling = sibling_ptr;
}
diff --git a/gdb/testsuite/gdb.dwarf2/corrupt.c b/gdb/testsuite/gdb.dwarf2/corrupt.c
new file mode 100644
index 0000000..bcd5fd8
--- /dev/null
+++ b/gdb/testsuite/gdb.dwarf2/corrupt.c
@@ -0,0 +1,24 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+ Copyright 2014 Free Software Foundation, Inc.
+
+ This program is free software; you can redistribute it and/or modify
+ it under the terms of the GNU General Public License as published by
+ the Free Software Foundation; either version 3 of the License, or
+ (at your option) any later version.
+
+ This program is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ GNU General Public License for more details.
+
+ You should have received a copy of the GNU General Public License
+ along with this program. If not, see <http://www.gnu.org/licenses/>. */
+
+/* Dummy main function. */
+
+int
+main (void)
+{
+ return 0;
+}
diff --git a/gdb/testsuite/gdb.dwarf2/corrupt.exp b/gdb/testsuite/gdb.dwarf2/corrupt.exp
new file mode 100644
index 0000000..048ae0c
--- /dev/null
+++ b/gdb/testsuite/gdb.dwarf2/corrupt.exp
@@ -0,0 +1,77 @@
+# Copyright 2014 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program. If not, see <http://www.gnu.org/licenses/>.
+
+# Test corrupt DWARF input
+# PR gdb/15827
+
+load_lib dwarf.exp
+
+if {![dwarf2_support]} {
+ return 0
+}
+
+standard_testfile corrupt.c corrupt.S
+
+# Make the DWARF used for the test.
+#
+# Here we put DW_AT_sibling DIEs into the output which
+# point off into la-la land. The whole purpose is to simulate
+# corrupt DWARF information and make sure that GDB can handle it
+# without crashing.
+
+set asm_file [standard_output_file $srcfile2]
+Dwarf::assemble $asm_file {
+ cu {} {
+ compile_unit {} {
+ declare_labels int_label
+
+ int_label: base_type {
+ {byte_size 4}
+ {name "int"}
+ }
+
+ enumeration_type {
+ {name "ENUM"}
+ {byte_size 4}
+ } {
+ enumerator {
+ {name "A"}
+ {const_value 0}
+ }
+ enumerator {
+ {name "B"}
+ {const_value 1}
+ {sibling 12345678 DW_FORM_ref4}
+ } {
+ base_type {
+ {byte_size 1}
+ {name "char"}
+ }
+ }
+ array_type {
+ {type :$int_label}
+ {sibling 12345678 DW_FORM_ref4}
+ }
+ }
+ }
+ }
+}
+
+if {[prepare_for_testing $testfile.exp $testfile \
+ [list $srcfile $asm_file] {nodebug}]} {
+ return -1
+}
+
+gdb_test "print 1" "= 1" "recover from corrupt DWARF"
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RFA] Fix gdb/15827 (crash w/corrupt DWARF)
2014-03-21 17:26 ` Keith Seitz
@ 2014-03-21 17:54 ` Joel Brobecker
2014-04-16 21:50 ` Keith Seitz
0 siblings, 1 reply; 6+ messages in thread
From: Joel Brobecker @ 2014-03-21 17:54 UTC (permalink / raw)
To: Keith Seitz; +Cc: gdb-patches@sourceware.org ml
> ChangeLog
> 2014-03-20 Keith Seitz <keiths@redhat.com>
>
> PR gdb/15827
> * dwarf2read.c (skip_one_die): Check that all relative-offset
> sibling DIEs fall within range of the current reader's buffer.
> (read_partial_die): Likewise.
>
> testsuite/ChangeLog
> 2014-03-20 Keith Seitz <keiths@redhat.com>
>
> PR gdb/15827
> * gdb.dwarf2/corrupt.c: New file.
> * gdb.dwarf2/corrupt.exp: New file.
FAOD, this version looks good to me.
Thanks!
--
Joel
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RFA] Fix gdb/15827 (crash w/corrupt DWARF)
2014-03-21 17:14 ` Joel Brobecker
2014-03-21 17:26 ` Keith Seitz
@ 2014-03-21 19:27 ` Doug Evans
1 sibling, 0 replies; 6+ messages in thread
From: Doug Evans @ 2014-03-21 19:27 UTC (permalink / raw)
To: Joel Brobecker; +Cc: Keith Seitz, gdb-patches@sourceware.org ml
On Fri, Mar 21, 2014 at 10:14 AM, Joel Brobecker <brobecker@adacore.com> wrote:
>> +# If we get here and gdb hasn't crashed, the tests pass.
>> +pass "corrupt DWARF"
>
> That's just me but I usually do a "print 1" test, just to make sure
> that even if the testing framework did not detect the GDB process
> dying, the "print 1" test definitely will. Not important on most,
> if not all platforms, but ISTR some odd platforms where this helped.
> That's just a suggestion, you don't have to follow it.
Not that this needs to be added to this patch, but IWBN to codify this
in a utility routine, some tcl proc that does "print 1" or whatever
and verifies the expected result. The point is to encapsulate the
intent in a suitably named function, verify_gdb_alive or some such.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RFA] Fix gdb/15827 (crash w/corrupt DWARF)
2014-03-21 17:54 ` Joel Brobecker
@ 2014-04-16 21:50 ` Keith Seitz
0 siblings, 0 replies; 6+ messages in thread
From: Keith Seitz @ 2014-04-16 21:50 UTC (permalink / raw)
To: gdb-patches@sourceware.org ml
On 03/21/2014 10:54 AM, Joel Brobecker wrote:
>> ChangeLog
>> 2014-03-20 Keith Seitz <keiths@redhat.com>
>>
>> PR gdb/15827
>> * dwarf2read.c (skip_one_die): Check that all relative-offset
>> sibling DIEs fall within range of the current reader's buffer.
>> (read_partial_die): Likewise.
>>
>> testsuite/ChangeLog
>> 2014-03-20 Keith Seitz <keiths@redhat.com>
>>
>> PR gdb/15827
>> * gdb.dwarf2/corrupt.c: New file.
>> * gdb.dwarf2/corrupt.exp: New file.
>
> FAOD, this version looks good to me.
>
I don't know why I didn't see this response, but, well, I didn't.
Sorry about the delay -- I've now pushed this patch in.
Thank you for the review.
Keith
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2014-04-16 21:50 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-03-21 16:48 [RFA] Fix gdb/15827 (crash w/corrupt DWARF) Keith Seitz
2014-03-21 17:14 ` Joel Brobecker
2014-03-21 17:26 ` Keith Seitz
2014-03-21 17:54 ` Joel Brobecker
2014-04-16 21:50 ` Keith Seitz
2014-03-21 19:27 ` Doug Evans
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).