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]

Re: [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.


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




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