From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 6879 invoked by alias); 3 Jan 2002 19:27:17 -0000 Mailing-List: contact gdb-patches-help@sources.redhat.com; run by ezmlm Precedence: bulk List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sources.redhat.com Received: (qmail 6836 invoked from network); 3 Jan 2002 19:27:14 -0000 Received: from unknown (HELO cygnus.com) (205.180.230.5) by sources.redhat.com with SMTP; 3 Jan 2002 19:27:14 -0000 Received: from redhat.com (reddwarf.cygnus.com [205.180.231.12]) by runyon.cygnus.com (8.8.7-cygnus/8.8.7) with ESMTP id LAA22463; Thu, 3 Jan 2002 11:27:12 -0800 (PST) Message-ID: <3C34AF7B.D6658DC0@redhat.com> Date: Thu, 03 Jan 2002 11:27:00 -0000 From: Michael Snyder Organization: Red Hat, Inc. X-Mailer: Mozilla 4.76 [en] (X11; U; Linux 2.4.2-2smp i686) X-Accept-Language: en MIME-Version: 1.0 To: Andrew Cagney CC: Michael Snyder , gdb-patches@sources.redhat.com Subject: Re: [RFA] Crasher bug in infptrace.c References: <200112310020.fBV0Kr119534@reddwarf.cygnus.com> <3C33F983.4030605@cygnus.com> Content-Type: text/plain; charset=us-ascii Content-Transfer-Encoding: 7bit X-SW-Source: 2002-01/txt/msg00011.txt.bz2 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.