From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 17717 invoked by alias); 7 Dec 2003 01:57:28 -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 17543 invoked from network); 7 Dec 2003 01:57:26 -0000 Received: from unknown (HELO localhost.redhat.com) (65.49.0.121) by sources.redhat.com with SMTP; 7 Dec 2003 01:57:26 -0000 Received: from gnu.org (localhost [127.0.0.1]) by localhost.redhat.com (Postfix) with ESMTP id 221502B8F; Sat, 6 Dec 2003 20:57:20 -0500 (EST) Message-ID: <3FD288FF.70009@gnu.org> Date: Sun, 07 Dec 2003 01:57:00 -0000 From: Andrew Cagney User-Agent: Mozilla/5.0 (X11; U; NetBSD macppc; en-US; rv:1.0.2) Gecko/20030820 X-Accept-Language: en-us, en MIME-Version: 1.0 To: Kevin Buettner Cc: gdb-patches@sources.redhat.com Subject: Re: [RFA] remote.c: Avoid multiple serial_close calls on baud rate error References: <1031205203631.ZM15513@localhost.localdomain> Content-Type: text/plain; charset=us-ascii; format=flowed Content-Transfer-Encoding: 7bit X-SW-Source: 2003-12/txt/msg00242.txt.bz2 > One of my colleagues recently noticed the following: > > (gdb) set remotebaud 0x100000 > (gdb) target remote /dev/ttyS0 > warning: Invalid baud rate 1048576. Maximum value is 460800. > /dev/ttyS0: Invalid argument. > (gdb) set remotebaud 230400 > (gdb) target remote /dev/ttyS0 > Segmentation fault > > The reason for this SEGV is that remote.c was closing ``remote_desc'' > twice. On the second attempted close, it was accessing some data > structures through some already freed (and probably even reallocated) > memory. > > The comment that I've added explains how the double close is avoided. > > FWIW, I considered calling remote_close(), but decided against it > since remote_desc can not be passed explicitly to this function. > Also, if the implementation of remote_close() were to change in some > way, it may end up doing more (or less) than what's desired for > handling the baud rate error. Conversely, a hypothetical change in > remote_close() may require that the error handling code be changed in > a similar fashion, so the preferred path to fixing this problem isn't > quite so clear cut. Therefore, I'm willing to revise this patch to > call remote_close() instead if that's deemed preferable. > > With regard to the testcase above, it'd be nice if this could be added > to the testsuite, but I can't think of a portable way of doing so. Calling target_close() here wouldn't be right. The target isn't yet open, the push call only occures further down. This begs the question: why was open called twice? I suspect unpush_target should only call target_close on open/pushed targets. Anyway, this change is fine. It makes the relevant code more robust. Andrew