From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 24290 invoked by alias); 2 Dec 2003 16:07:08 -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 24282 invoked from network); 2 Dec 2003 16:07:06 -0000 Received: from unknown (HELO nevyn.them.org) (66.93.172.17) by sources.redhat.com with SMTP; 2 Dec 2003 16:07:06 -0000 Received: from drow by nevyn.them.org with local (Exim 4.24 #1 (Debian)) id 1ARD3I-0006Ab-Qs for ; Tue, 02 Dec 2003 11:07:04 -0500 Date: Tue, 02 Dec 2003 16:07:00 -0000 From: Daniel Jacobowitz To: gdb-patches@sources.redhat.com Subject: [RFA] Remove some sprintfs from vCont client support Message-ID: <20031202160704.GA22220@nevyn.them.org> Mail-Followup-To: gdb-patches@sources.redhat.com References: <20030929152831.GA23286@nevyn.them.org> <20030930211717.GB19869@nevyn.them.org> <3F8C917C.1080708@gnu.org> <20031016203156.GA24204@nevyn.them.org> <3F8F0B2B.9080506@redhat.com> <20031016221433.GA553@nevyn.them.org> <3F8F1B3B.7000904@redhat.com> <20031016225328.GA1542@nevyn.them.org> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20031016225328.GA1542@nevyn.them.org> User-Agent: Mutt/1.5.1i X-SW-Source: 2003-12/txt/msg00049.txt.bz2 On Thu, Oct 16, 2003 at 06:53:28PM -0400, Daniel Jacobowitz wrote: > On Thu, Oct 16, 2003 at 06:27:07PM -0400, Andrew Cagney wrote: > > >There are two probems: > > >> > > >>- the buffer can get very very large and that can blow the stack If that concerns you then I suggest the alloca's in putpkt_binary, which I found while fixing this. That's a whole lot more worrisome, since there's one as big as the packet plus one as big as the max packet length. > > >>- it isn't possible to audit this code (with out a deep understanding of > > >>that value) and hence demonstrate that the sprintf won't smash the > > >>stack/heap > > >> > > >>You'll need to also change the sprintf to snprintf (parameterized with > > >>remote_packet_size. > > > > > > > > >I don't see a point in doing that until someone expresses interest in > > >thread locking or some other feature which requires adding to the code. > > >The maximum length of any generated vcont packet is the length of: > > > vCont;C01:12341468;C02 > > >The minimum possible buffer size is about twenty times that. > > > > I wrote "it isn't possible to audit this code (with out a deep > > understanding of that [remote_packet_size] value)". The code should be > > locally robust. > > I wouldn't call the minimum size a deep understanding. It isn't > documented anywhere in the code but I think that it should be. > > But I'll fix it next week. OK, so it wasn't the next week. I took advantage of the new xstrprintf function to get something I'm happy with. How about you, Andrew - is this patch OK? -- Daniel Jacobowitz MontaVista Software Debian GNU/Linux Developer 2003-12-02 Daniel Jacobowitz * remote.c (remote_vcont_resume): Use xstrprintf instead of sprintf. Index: remote.c =================================================================== RCS file: /cvs/src/src/gdb/remote.c,v retrieving revision 1.122 diff -u -p -r1.122 remote.c --- remote.c 10 Nov 2003 21:20:44 -0000 1.122 +++ remote.c 2 Dec 2003 15:57:33 -0000 @@ -2578,7 +2578,7 @@ remote_vcont_resume (ptid_t ptid, int st { struct remote_state *rs = get_remote_state (); int pid = PIDGET (ptid); - char *buf = NULL; + char *buf = NULL, *outbuf; struct cleanup *old_cleanup; buf = xmalloc (rs->remote_packet_size); @@ -2603,40 +2603,45 @@ remote_vcont_resume (ptid_t ptid, int st don't have any PID numbers the inferior will understand. Make sure to only send forms that do not specify a PID. */ if (step && siggnal != TARGET_SIGNAL_0) - sprintf (buf, "vCont;S%02x", siggnal); + outbuf = xstrprintf ("vCont;S%02x", siggnal); else if (step) - sprintf (buf, "vCont;s"); + outbuf = xstrprintf ("vCont;s"); else if (siggnal != TARGET_SIGNAL_0) - sprintf (buf, "vCont;C%02x", siggnal); + outbuf = xstrprintf ("vCont;C%02x", siggnal); else - sprintf (buf, "vCont;c"); + outbuf = xstrprintf ("vCont;c"); } else if (pid == -1) { /* Resume all threads, with preference for INFERIOR_PTID. */ if (step && siggnal != TARGET_SIGNAL_0) - sprintf (buf, "vCont;S%02x:%x;c", siggnal, PIDGET (inferior_ptid)); + outbuf = xstrprintf ("vCont;S%02x:%x;c", siggnal, + PIDGET (inferior_ptid)); else if (step) - sprintf (buf, "vCont;s:%x;c", PIDGET (inferior_ptid)); + outbuf = xstrprintf ("vCont;s:%x;c", PIDGET (inferior_ptid)); else if (siggnal != TARGET_SIGNAL_0) - sprintf (buf, "vCont;C%02x:%x;c", siggnal, PIDGET (inferior_ptid)); + outbuf = xstrprintf ("vCont;C%02x:%x;c", siggnal, + PIDGET (inferior_ptid)); else - sprintf (buf, "vCont;c"); + outbuf = xstrprintf ("vCont;c"); } else { /* Scheduler locking; resume only PTID. */ if (step && siggnal != TARGET_SIGNAL_0) - sprintf (buf, "vCont;S%02x:%x", siggnal, pid); + outbuf = xstrprintf ("vCont;S%02x:%x", siggnal, pid); else if (step) - sprintf (buf, "vCont;s:%x", pid); + outbuf = xstrprintf ("vCont;s:%x", pid); else if (siggnal != TARGET_SIGNAL_0) - sprintf (buf, "vCont;C%02x:%x", siggnal, pid); + outbuf = xstrprintf ("vCont;C%02x:%x", siggnal, pid); else - sprintf (buf, "vCont;c:%x", pid); + outbuf = xstrprintf ("vCont;c:%x", pid); } - putpkt (buf); + gdb_assert (outbuf && strlen (outbuf) < rs->remote_packet_size); + make_cleanup (xfree, outbuf); + + putpkt (outbuf); do_cleanups (old_cleanup);