This is the mail archive of the
gdb-patches@sources.redhat.com
mailing list for the GDB project.
Re: [RFA] Crasher bug in infptrace.c
- From: Andrew Cagney <ac131313 at cygnus dot com>
- To: Michael Snyder <msnyder at cygnus dot com>
- Cc: gdb-patches at sources dot redhat dot com
- Date: Thu, 03 Jan 2002 15:56:11 +0930
- Subject: Re: [RFA] Crasher bug in infptrace.c
- References: <200112310020.fBV0Kr119534@reddwarf.cygnus.com>
> 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.
BTW, the gdbint.texinfo document suggests that anything more than a few
k is dangerous.
http://sources.redhat.com/gdb/onlinedocs/gdbint_13.html#SEC103
> 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.
Probably a good move, perhaps add a FIXME comment to them so that the
person that does encounter the bug knows they are not seeing things :-)
> + 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);
> ! }
I think it would be better to just abandon the alloca() case and just
use xmalloc(). That way the same code path (xmalloc()) is always used
and mysterious / obscure bugs that end up being attributed to
len?=GDB_MAX_ALLOCA can be avoided.
Andrew