From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 3236 invoked by alias); 5 Dec 2003 20:36:39 -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 3229 invoked from network); 5 Dec 2003 20:36:38 -0000 Received: from unknown (HELO mx1.redhat.com) (66.187.233.31) by sources.redhat.com with SMTP; 5 Dec 2003 20:36:38 -0000 Received: from int-mx1.corp.redhat.com (int-mx1.corp.redhat.com [172.16.52.254]) by mx1.redhat.com (8.11.6/8.11.6) with ESMTP id hB5Kac227878 for ; Fri, 5 Dec 2003 15:36:38 -0500 Received: from pobox.corp.redhat.com (pobox.corp.redhat.com [172.16.52.156]) by int-mx1.corp.redhat.com (8.11.6/8.11.6) with ESMTP id hB5Kab230097 for ; Fri, 5 Dec 2003 15:36:37 -0500 Received: from localhost.localdomain (vpn50-70.rdu.redhat.com [172.16.50.70]) by pobox.corp.redhat.com (8.12.8/8.12.8) with ESMTP id hB5Kabm5026163 for ; Fri, 5 Dec 2003 15:36:37 -0500 Received: (from kev@localhost) by localhost.localdomain (8.11.6/8.11.6) id hB5KaWr15514 for gdb-patches@sources.redhat.com; Fri, 5 Dec 2003 13:36:32 -0700 Date: Fri, 05 Dec 2003 20:36:00 -0000 From: Kevin Buettner Message-Id: <1031205203631.ZM15513@localhost.localdomain> To: gdb-patches@sources.redhat.com Subject: [RFA] remote.c: Avoid multiple serial_close calls on baud rate error MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii X-SW-Source: 2003-12/txt/msg00224.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. Okay? * remote.c (remote_open_1, remote_cisco_open): Avoid closing remote_desc more than once. 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 5 Dec 2003 19:58:17 -0000 @@ -2299,7 +2299,12 @@ remote_open_1 (char *name, int from_tty, { if (serial_setbaudrate (remote_desc, baud_rate)) { + /* The requested speed could not be set. Error out to + top level after closing remote_desc. Take care to + set remote_desc to NULL to avoid closing remote_desc + more than once. */ serial_close (remote_desc); + remote_desc = NULL; perror_with_name (name); } } @@ -5552,7 +5557,12 @@ remote_cisco_open (char *name, int from_ baud_rate = (baud_rate > 0) ? baud_rate : 9600; if (serial_setbaudrate (remote_desc, baud_rate)) { + /* The requested speed could not be set. Error out to + top level after closing remote_desc. Take care to + set remote_desc to NULL to avoid closing remote_desc + more than once. */ serial_close (remote_desc); + remote_desc = NULL; perror_with_name (name); }