This is the mail archive of the gdb-patches@sources.redhat.com mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[RFA] Crasher bug in infptrace.c


Here's one for the books...

Child_xfer_memory (one of the oldest functions in gdb)  uses alloca 
to allocate a buffer that can be arbitrarily large (as large as the
size of a memory read/write).  Alloca is known to be unsafe for large
enough chunks of memory, because it puts them on the stack -- and 
sure enough, it turns out that you can crash GDB by reading a large
enough data object from target memory.  For Linux, "large enough"
appears to be about 8 megabytes.  But this code has been as it is
for over ten years, and I've never heard of a problem with it before.

Test case attached (although because it causes GDB to core dump, 
it results in an ERROR instead of a FAIL...)

I don't believe this buffer is actually needed at all, but I've
gone with the minimum change instead of rewriting the function
so that it doesn't use a local buffer.

By the way, this code has been cloned in rs6000-nat.c, symm-nat.c, 
infttrace.c, and x86-64-linux-nat.c, so they probably have the
same bug.  I haven't touched them because I can't easily test them.


2001-12-30  Michael Snyder  <msnyder@redhat.com>

	* infptrace.c (GDB_MAX_ALLOCA): New define.
	(child_xfer_memory): Use xmalloc/xfree instead of alloca if the
	size of the buffer exceeds GDB_MAX_ALLOCA (default 1 megabyte, 
	can be overridden with whatever value is appropriate to the host).

2001-12-30  Michael Snyder  <msnyder@redhat.com>

	* gdb.base/huge.exp: New test.  Print a very large target data object.
	(skip_huge_test): New test variable.  Define if you want to skip this
	test.  The test reads an 8 megabyte data object from the target, so it
	might be very time consuming on remote targets with a slow connection.
	* gdb.base/huge.c: New file.  Test case for above.

Index: infptrace.c
===================================================================
RCS file: /cvs/src/src/gdb/infptrace.c,v
retrieving revision 1.19
diff -c -3 -p -r1.19 infptrace.c
*** infptrace.c	2001/11/19 23:59:55	1.19
--- infptrace.c	2001/12/31 00:10:49
*************** store_inferior_registers (int regno)
*** 480,485 ****
--- 480,490 ----
  #endif /* !defined (FETCH_INFERIOR_REGISTERS).  */
  
  
+ /* Set an upper limit on alloca.  */
+ #ifndef GDB_MAX_ALLOCA
+ #define GDB_MAX_ALLOCA 0x100000
+ #endif
+ 
  #if !defined (CHILD_XFER_MEMORY)
  /* NOTE! I tried using PTRACE_READDATA, etc., to read and write memory
     in the NEW_SUN_PTRACE case.  It ought to be straightforward.  But
*************** child_xfer_memory (CORE_ADDR memaddr, ch
*** 506,514 ****
    /* Round ending address up; get number of longwords that makes.  */
    int count = ((((memaddr + len) - addr) + sizeof (PTRACE_XFER_TYPE) - 1)
  	       / sizeof (PTRACE_XFER_TYPE));
    /* Allocate buffer of that many longwords.  */
!   PTRACE_XFER_TYPE *buffer =
!     (PTRACE_XFER_TYPE *) alloca (count * sizeof (PTRACE_XFER_TYPE));
  
    if (write)
      {
--- 511,529 ----
    /* Round ending address up; get number of longwords that makes.  */
    int count = ((((memaddr + len) - addr) + sizeof (PTRACE_XFER_TYPE) - 1)
  	       / sizeof (PTRACE_XFER_TYPE));
+   int alloc = count * sizeof (PTRACE_XFER_TYPE);
+   PTRACE_XFER_TYPE *buffer;
+ 
    /* Allocate buffer of that many longwords.  */
!   if (len < GDB_MAX_ALLOCA)
!     {
!       buffer = (PTRACE_XFER_TYPE *) alloca (alloc);
!     }
!   else
!     {
!       buffer = (PTRACE_XFER_TYPE *) xmalloc (alloc);
!       make_cleanup (xfree, buffer);
!     }
  
    if (write)
      {
Index: testsuite/gdb.base/huge.c
===================================================================
RCS file: huge.c
diff -N huge.c
*** /dev/null	Tue May  5 13:32:27 1998
--- huge.c	Sun Dec 30 16:10:49 2001
***************
*** 0 ****
--- 1,19 ----
+ /*
+  * Test GDB's ability to read a very large data object from target memory.
+  */
+ 
+ /* 
+  * A value that will produce a target data object 
+  * large enough to crash GDB.  0x200000 is big enough
+  * on Linux, other systems may need a larger number.
+  */
+ 
+ #define CRASH_GDB 0x200000
+ 
+ static int a[CRASH_GDB], b[CRASH_GDB];
+ 
+ main()
+ {
+   memcpy (a, b, sizeof (a));
+   return 0;
+ }
Index: testsuite/gdb.base/huge.exp
===================================================================
RCS file: huge.exp
diff -N huge.exp
*** /dev/null	Tue May  5 13:32:27 1998
--- huge.exp	Sun Dec 30 16:10:49 2001
***************
*** 0 ****
--- 1,57 ----
+ # Copyright 2001 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 2 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, write to the Free Software
+ # Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA.  
+ 
+ # Please email any bugs, comments, and/or additions to this file to:
+ # bug-gdb@prep.ai.mit.edu
+ 
+ # This file was written by Michael Snyder (msnyder@redhat.com)
+ 
+ if $tracelevel then {
+ 	strace $tracelevel
+ }
+ 
+ set prms_id 0
+ set bug_id 0
+ 
+ # Define if you want to skip this test
+ # (could be very time-consuming on remote targets with slow connection).
+ #
+ if [target_info exists gdb,skip_huge_test] {
+     return;
+ }
+ 
+ set testfile "huge"
+ set srcfile ${testfile}.c
+ set binfile ${objdir}/${subdir}/${testfile}
+ if  { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable {debug}] != "" } {
+      gdb_suppress_entire_file "Testcase compile failed, so all tests in this file will automatically fail."
+ }
+ 
+ # Start with a fresh gdb.
+ 
+ gdb_exit
+ gdb_start
+ gdb_reinitialize_dir $srcdir/$subdir
+ gdb_load ${binfile}
+ 
+ set timeout 30
+ 
+ if { ! [ runto main ] } then {
+     gdb_suppress_entire_file "Run to main failed, so all tests in this file will automatically fail."
+ }
+ 
+ gdb_test "print a" ".1 = .0 .repeats \[0123456789\]+ times.." "print a very large data object"
+ 


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]