This is the mail archive of the
gdb-patches@sources.redhat.com
mailing list for the GDB project.
Re: [RFA] Revised patch: fix crasher bug in child_xfer_memory
- From: Michael Snyder <msnyder at redhat dot com>
- To: gdb-patches at sources dot redhat dot com
- Date: Mon, 07 Jan 2002 17:50:31 -0800
- Subject: Re: [RFA] Revised patch: fix crasher bug in child_xfer_memory
- Organization: Red Hat, Inc.
- References: <200201032032.g03KWSe05838@reddwarf.cygnus.com>
Michael Snyder wrote:
>
> I've revised the patch so that it will not use alloca for a buffer
> greater than 4K. I've also added a call to do_cleanups, so that if
> xmalloc is used, its memory will be freed as soon as possible.
> And I've added FIXME comments to the four other modules that use
> clones of this code (in case they also run into problems with
> large alloca buffers).
Committed.
>
> 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).
> * infttrace.c (child_xfer_memory): Add FIXME warning about use of
> alloca to allocate potentially large buffer.
> * rs6000-nat.c (child_xfer_memory): Ditto.
> * symm-nat.c (child_xfer_memory): Ditto.
> * x86-64-linux-nat.c (child_xfer_memory): Ditto.
>
> 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 -p -r1.19 infptrace.c
> *** infptrace.c 2001/11/19 23:59:55 1.19
> --- infptrace.c 2002/01/03 20:26:08
> *************** 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 0x1000
> + #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,530 ----
> /* 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;
> + struct cleanup *old_chain = NULL;
> +
> /* Allocate buffer of that many longwords. */
> ! if (len < GDB_MAX_ALLOCA)
> ! {
> ! buffer = (PTRACE_XFER_TYPE *) alloca (alloc);
> ! }
> ! else
> ! {
> ! buffer = (PTRACE_XFER_TYPE *) xmalloc (alloc);
> ! old_chain = make_cleanup (xfree, buffer);
> ! }
>
> if (write)
> {
> *************** child_xfer_memory (CORE_ADDR memaddr, ch
> *** 573,578 ****
> --- 589,596 ----
> len);
> }
>
> + if (old_chain != NULL)
> + do_cleanups (old_chain);
> return len;
> }
>
> Index: infttrace.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/infttrace.c,v
> retrieving revision 1.17
> diff -p -r1.17 infttrace.c
> *** infttrace.c 2001/12/19 19:16:50 1.17
> --- infttrace.c 2002/01/03 20:26:08
> *************** child_xfer_memory (CORE_ADDR memaddr, ch
> *** 4922,4929 ****
> = (((memaddr + len) - addr) + sizeof (TTRACE_XFER_TYPE) - 1)
> / sizeof (TTRACE_XFER_TYPE);
> /* Allocate buffer of that many longwords. */
> register TTRACE_XFER_TYPE *buffer
> ! = (TTRACE_XFER_TYPE *) alloca (count * sizeof (TTRACE_XFER_TYPE));
>
> if (write)
> {
> --- 4922,4932 ----
> = (((memaddr + len) - addr) + sizeof (TTRACE_XFER_TYPE) - 1)
> / sizeof (TTRACE_XFER_TYPE);
> /* Allocate buffer of that many longwords. */
> + /* FIXME (alloca): This code, cloned from infptrace.c, is unsafe
> + because it uses alloca to allocate a buffer of arbitrary size.
> + For very large xfers, this could crash GDB's stack. */
> register TTRACE_XFER_TYPE *buffer
> ! = (TTRACE_XFER_TYPE *) alloca (count * sizeof (TTRACE_XFER_TYPE));
>
> if (write)
> {
> Index: rs6000-nat.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/rs6000-nat.c,v
> retrieving revision 1.19
> diff -p -r1.19 rs6000-nat.c
> *** rs6000-nat.c 2001/12/20 23:29:24 1.19
> --- rs6000-nat.c 2002/01/03 20:26:08
> *************** child_xfer_memory (CORE_ADDR memaddr, ch
> *** 407,412 ****
> --- 407,415 ----
> / sizeof (int);
>
> /* Allocate word transfer buffer. */
> + /* FIXME (alloca): This code, cloned from infptrace.c, is unsafe
> + because it uses alloca to allocate a buffer of arbitrary size.
> + For very large xfers, this could crash GDB's stack. */
> int *buf = (int *) alloca (count * sizeof (int));
>
> int arch64 = ARCH64 ();
> Index: symm-nat.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/symm-nat.c,v
> retrieving revision 1.10
> diff -p -r1.10 symm-nat.c
> *** symm-nat.c 2001/07/26 02:23:57 1.10
> --- symm-nat.c 2002/01/03 20:26:08
> *************** child_xfer_memory (CORE_ADDR memaddr, ch
> *** 764,771 ****
> = (((memaddr + len) - addr) + sizeof (PTRACE_XFER_TYPE) - 1)
> / sizeof (PTRACE_XFER_TYPE);
> /* Allocate buffer of that many longwords. */
> register PTRACE_XFER_TYPE *buffer
> ! = (PTRACE_XFER_TYPE *) alloca (count * sizeof (PTRACE_XFER_TYPE));
>
> if (write)
> {
> --- 764,774 ----
> = (((memaddr + len) - addr) + sizeof (PTRACE_XFER_TYPE) - 1)
> / sizeof (PTRACE_XFER_TYPE);
> /* Allocate buffer of that many longwords. */
> + /* FIXME (alloca): This code, cloned from infptrace.c, is unsafe
> + because it uses alloca to allocate a buffer of arbitrary size.
> + For very large xfers, this could crash GDB's stack. */
> register PTRACE_XFER_TYPE *buffer
> ! = (PTRACE_XFER_TYPE *) alloca (count * sizeof (PTRACE_XFER_TYPE));
>
> if (write)
> {
> Index: x86-64-linux-nat.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/x86-64-linux-nat.c,v
> retrieving revision 1.1
> diff -p -r1.1 x86-64-linux-nat.c
> *** x86-64-linux-nat.c 2001/09/21 12:19:15 1.1
> --- x86-64-linux-nat.c 2002/01/03 20:26:08
> *************** child_xfer_memory (CORE_ADDR memaddr, ch
> *** 417,422 ****
> --- 416,424 ----
> = (((memaddr + len) - addr) + sizeof (PTRACE_XFER_TYPE) - 1)
> / sizeof (PTRACE_XFER_TYPE);
> /* Allocate buffer of that many longwords. */
> + /* FIXME (alloca): This code, cloned from infptrace.c, is unsafe
> + because it uses alloca to allocate a buffer of arbitrary size.
> + For very large xfers, this could crash GDB's stack. */
> register PTRACE_XFER_TYPE *buffer
> = (PTRACE_XFER_TYPE *) alloca (count * sizeof (PTRACE_XFER_TYPE));
>
> 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"
> +