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: Michael Snyder <msnyder at redhat dot com>
- To: Andrew Cagney <ac131313 at cygnus dot com>
- Cc: Michael Snyder <msnyder at cygnus dot com>, gdb-patches at sources dot redhat dot com
- Date: Thu, 03 Jan 2002 11:22:35 -0800
- Subject: Re: [RFA] Crasher bug in infptrace.c
- Organization: Red Hat, Inc.
- References: <200112310020.fBV0Kr119534@reddwarf.cygnus.com> <3C33F983.4030605@cygnus.com>
Andrew Cagney wrote:
>
> > 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
OK, I'll resubmit the patch with a smaller limit, perhaps 4K or 8K.
> > 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 :-)
Will do.
> > + 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.
I don't think so -- this function gets called a lot. Heavy use of
xmalloc on small buffers might lead to fragmentation. Let's try the
idea of using alloca for small buffers and xmalloc for big ones.