public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [PATCH] gdb: fix buffer overflow in DWARF reader
@ 2023-09-14 15:48 Andrew Burgess
  2023-09-14 16:03 ` Tom Tromey
  0 siblings, 1 reply; 2+ messages in thread
From: Andrew Burgess @ 2023-09-14 15:48 UTC (permalink / raw)
  To: gdb-patches; +Cc: Tom Tromey, Andrew Burgess

In this commit:

  commit 48ac197b0c209ccf1f2de9704eb6cdf7c5c73a8e
  Date:   Fri Nov 19 10:12:44 2021 -0700

      Handle multiple addresses in call_site_target

a buffer overflow bug was introduced when the following code was
added:

  CORE_ADDR *saved = XOBNEWVAR (&objfile->objfile_obstack, CORE_ADDR,
                                addresses.size ());
  std::copy (addresses.begin (), addresses.end (), saved);

The definition of XOBNEWVAR is (from libiberty.h):

  #define XOBNEWVAR(O, T, S)	((T *) obstack_alloc ((O), (S)))

So 'saved' is going to point to addresses.size () bytes of memory,
however, the std::copy will write addresses.size () number of
CORE_ADDR sized entries to the address pointed to by 'saved', this is
going to result in memory corruption.

The mistake is that we should have used XOBNEWVEC, which allocates a
vector of entries, the definition of XOBNEWVEC is:

  #define XOBNEWVEC(O, T, N) \
    ((T *) obstack_alloc ((O), sizeof (T) * (N)))

Which means we will have set aside enough space to create a copy of
the contents of the addresses vector.

I'm not sure how to create a test for this problem, this issue cropped
up when debugging a particular i686 built binary, which just happened
to trigger a glibc assertion (likely due to random memory corruption),
debugging the same binary built for x86-64 appeared to work just fine.

Using valgrind on the failing GDB binary pointed straight to the cause
of the problem, and with this patch in place there are no longer
valgrind errors in this area.

If anyone has ideas for a test I'm happy to work on something.
---
 gdb/dwarf2/read.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index 98bedbc5d49..0f4d99109fb 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -10548,7 +10548,7 @@ read_call_site_scope (struct die_info *die, struct dwarf2_cu *cu)
 	  std::vector<unrelocated_addr> addresses;
 	  dwarf2_ranges_read_low_addrs (ranges_offset, target_cu,
 					target_die->tag, addresses);
-	  unrelocated_addr *saved = XOBNEWVAR (&objfile->objfile_obstack,
+	  unrelocated_addr *saved = XOBNEWVEC (&objfile->objfile_obstack,
 					       unrelocated_addr,
 					       addresses.size ());
 	  std::copy (addresses.begin (), addresses.end (), saved);

base-commit: d7680f13df105aa8fa0edbdf8efae31a5411f579
-- 
2.25.4


^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: [PATCH] gdb: fix buffer overflow in DWARF reader
  2023-09-14 15:48 [PATCH] gdb: fix buffer overflow in DWARF reader Andrew Burgess
@ 2023-09-14 16:03 ` Tom Tromey
  0 siblings, 0 replies; 2+ messages in thread
From: Tom Tromey @ 2023-09-14 16:03 UTC (permalink / raw)
  To: Andrew Burgess; +Cc: gdb-patches, Tom Tromey

>>>>> "Andrew" == Andrew Burgess <aburgess@redhat.com> writes:

Andrew> In this commit:
Andrew>   commit 48ac197b0c209ccf1f2de9704eb6cdf7c5c73a8e
Andrew>   Date:   Fri Nov 19 10:12:44 2021 -0700

Andrew>       Handle multiple addresses in call_site_target

Andrew> a buffer overflow bug was introduced when the following code was
Andrew> added:

Sorry about that.

Andrew> I'm not sure how to create a test for this problem, this issue cropped
Andrew> up when debugging a particular i686 built binary, which just happened
Andrew> to trigger a glibc assertion (likely due to random memory corruption),
Andrew> debugging the same binary built for x86-64 appeared to work just fine.

The best thing would be if we had valgrind annotations for obstack.
Then valgrind or perhaps ASAN would catch this kind of bug.

Approved-By: Tom Tromey <tom@tromey.com>

Tom

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2023-09-14 16:03 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-09-14 15:48 [PATCH] gdb: fix buffer overflow in DWARF reader Andrew Burgess
2023-09-14 16:03 ` Tom Tromey

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).