[RFA] remote.c: Avoid multiple serial_close calls on baud rate error

Kevin Buettner kevinb@redhat.com
Fri Dec 5 20:36:00 GMT 2003


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);
     }
 



More information about the Gdb-patches mailing list