[rfc] GDB memory corruption in multi-forks test case

["Ulrich Weigand" <uweigand at de dot ibm dot com>]

Hello,

I was seeing random crashes on ARM when running the multi-forks test case,
due to apparent memory corruption.  Investigation with valgrind showed that
the root cause was double freeing of "ttystate" data structures:

The inflow.c inferior data contains a member ttystate, which is of type
serial_ttystate, a void pointer to a data structure allocated by the
current serial back-end (via serial_ops).

When the inferior forks (in "set detach off" mode), the inflow.c inferior
data is copied in copy_terminal_info.  This simply copies the ttystate
pointer over to the new inferior.  Note that there is no reference counting
whatsoever on this structure.  Subsequently, both inferiors will free
the ttystate pointer, leading to a double free bug.

The following patch fixes this by actually copying the ttystate.  This
requires a new routine serial_copy_tty_state and a corresponding serial_ops
callback (as was already suggested by a comment in inflow.c ...).

Tested on armv7l-linux-gnueabi and i386-linux with no regressions.

Does this look reasonable?

Bye,
Ulrich


ChangeLog:

	* inflow.c (terminal_init_inferior_with_pgrp): Copy ttystate.
	(terminal_save_ours): Remove misleading comment.
	(inflow_inferior_data_cleanup): Free ttystate.
	(inflow_inferior_exit): Likewise.
	(copy_terminal_info): Copy ttystate.

	* serial.c (serial_copy_tty_state): New function.
	* serial.h (serial_copy_tty_state): Add prototype.
	(struct serial_ops): Add copy_tty_state callback.
	* ser-base.c (ser_base_copy_tty_state): New function.
	* ser-base.h (ser_base_copy_tty_state): Add prototype.
	* ser-go32.c (dos_copy_tty_state): New function.
	(dos_ops): Install copy_tty_state callback.
	* ser-mingw.c (_initialize_ser_windows): Likewise.
	* ser-pipe.c (_initialize_ser_pipe): Likewise.
	* ser-unix.c (hardwire_copy_tty_state): New function.
	(_initialize_ser_hardwire): Install it.


Index: gdb/inflow.c
===================================================================
RCS file: /cvs/src/src/gdb/inflow.c,v
retrieving revision 1.65
diff -u -p -r1.65 inflow.c
--- gdb/inflow.c	31 Jan 2011 03:11:39 -0000	1.65
+++ gdb/inflow.c	4 Mar 2011 15:58:35 -0000
@@ -224,10 +224,9 @@ terminal_init_inferior_with_pgrp (int pg
       struct inferior *inf = current_inferior ();
       struct terminal_info *tinfo = get_inflow_inferior_data (inf);
 
-      /* We could just as well copy our_ttystate (if we felt like
-         adding a new function serial_copy_tty_state()).  */
       xfree (tinfo->ttystate);
-      tinfo->ttystate = serial_get_tty_state (stdin_serial);
+      tinfo->ttystate = serial_copy_tty_state (stdin_serial,
+					       our_terminal_info.ttystate);
 
 #ifdef PROCESS_GROUP_TYPE
       tinfo->process_group = pgrp;
@@ -249,8 +248,6 @@ terminal_save_ours (void)
 {
   if (gdb_has_a_terminal ())
     {
-      /* We could just as well copy our_ttystate (if we felt like adding
-         a new function serial_copy_tty_state).  */
       xfree (our_terminal_info.ttystate);
       our_terminal_info.ttystate = serial_get_tty_state (stdin_serial);
     }
@@ -495,6 +492,7 @@ inflow_inferior_data_cleanup (struct inf
   if (info != NULL)
     {
       xfree (info->run_terminal);
+      xfree (info->ttystate);
       xfree (info);
     }
 }
@@ -532,6 +530,7 @@ inflow_inferior_exit (struct inferior *i
   if (info != NULL)
     {
       xfree (info->run_terminal);
+      xfree (info->ttystate);
       xfree (info);
       set_inferior_data (inf, inflow_inferior_data, NULL);
     }
@@ -544,10 +543,19 @@ copy_terminal_info (struct inferior *to,
 
   tinfo_to = get_inflow_inferior_data (to);
   tinfo_from = get_inflow_inferior_data (from);
+
+  xfree (tinfo_to->run_terminal);
+  xfree (tinfo_to->ttystate);
+
   *tinfo_to = *tinfo_from;
+
   if (tinfo_from->run_terminal)
     tinfo_to->run_terminal
       = xstrdup (tinfo_from->run_terminal);
+
+  if (tinfo_from->ttystate)
+    tinfo_to->ttystate
+      = serial_copy_tty_state (stdin_serial, tinfo_from->ttystate);
 }
 
 void
Index: gdb/ser-base.c
===================================================================
RCS file: /cvs/src/src/gdb/ser-base.c,v
retrieving revision 1.21
diff -u -p -r1.21 ser-base.c
--- gdb/ser-base.c	11 Jan 2011 21:53:23 -0000	1.21
+++ gdb/ser-base.c	4 Mar 2011 15:58:35 -0000
@@ -463,6 +463,13 @@ ser_base_get_tty_state (struct serial *s
   return (serial_ttystate) XMALLOC (int);
 }
 
+serial_ttystate
+ser_base_copy_tty_state (struct serial *scb, serial_ttystate ttystate)
+{
+  /* Allocate another dummy.  */
+  return (serial_ttystate) XMALLOC (int);
+}
+
 int
 ser_base_set_tty_state (struct serial *scb, serial_ttystate ttystate)
 {
Index: gdb/ser-base.h
===================================================================
RCS file: /cvs/src/src/gdb/ser-base.h,v
retrieving revision 1.11
diff -u -p -r1.11 ser-base.h
--- gdb/ser-base.h	1 Jan 2011 15:33:14 -0000	1.11
+++ gdb/ser-base.h	4 Mar 2011 15:58:35 -0000
@@ -32,6 +32,8 @@ extern int ser_base_flush_input (struct 
 extern int ser_base_send_break (struct serial *scb);
 extern void ser_base_raw (struct serial *scb);
 extern serial_ttystate ser_base_get_tty_state (struct serial *scb);
+extern serial_ttystate ser_base_copy_tty_state (struct serial *scb,
+						serial_ttystate ttystate);
 extern int ser_base_set_tty_state (struct serial *scb,
 				   serial_ttystate ttystate);
 extern void ser_base_print_tty_state (struct serial *scb,
Index: gdb/ser-go32.c
===================================================================
RCS file: /cvs/src/src/gdb/ser-go32.c,v
retrieving revision 1.27
diff -u -p -r1.27 ser-go32.c
--- gdb/ser-go32.c	11 Jan 2011 21:53:23 -0000	1.27
+++ gdb/ser-go32.c	4 Mar 2011 15:58:35 -0000
@@ -652,6 +652,17 @@ dos_get_tty_state (struct serial *scb)
   return (serial_ttystate) state;
 }
 
+static serial_ttystate
+dos_copy_tty_state (struct serial *scb, serial_ttystate ttystate)
+{
+  struct dos_ttystate *state;
+
+  state = (struct dos_ttystate *) xmalloc (sizeof *state);
+  *state = *(struct dos_ttystate *) ttystate;
+
+  return (serial_ttystate) state;
+}
+
 static int
 dos_set_tty_state (struct serial *scb, serial_ttystate ttystate)
 {
@@ -851,6 +862,7 @@ static struct serial_ops dos_ops =
   dos_sendbreak,
   dos_raw,
   dos_get_tty_state,
+  dos_copy_tty_state,
   dos_set_tty_state,
   dos_print_tty_state,
   dos_noflush_set_tty_state,
Index: gdb/ser-mingw.c
===================================================================
RCS file: /cvs/src/src/gdb/ser-mingw.c,v
retrieving revision 1.28
diff -u -p -r1.28 ser-mingw.c
--- gdb/ser-mingw.c	21 Feb 2011 13:40:31 -0000	1.28
+++ gdb/ser-mingw.c	4 Mar 2011 15:58:35 -0000
@@ -1244,6 +1244,7 @@ _initialize_ser_windows (void)
   /* These are only used for stdin; we do not need them for serial
      ports, so supply the standard dummies.  */
   ops->get_tty_state = ser_base_get_tty_state;
+  ops->copy_tty_state = ser_base_copy_tty_state;
   ops->set_tty_state = ser_base_set_tty_state;
   ops->print_tty_state = ser_base_print_tty_state;
   ops->noflush_set_tty_state = ser_base_noflush_set_tty_state;
@@ -1272,6 +1273,7 @@ _initialize_ser_windows (void)
 
   ops->close = ser_console_close;
   ops->get_tty_state = ser_console_get_tty_state;
+  ops->copy_tty_state = ser_base_copy_tty_state;
   ops->set_tty_state = ser_base_set_tty_state;
   ops->print_tty_state = ser_base_print_tty_state;
   ops->noflush_set_tty_state = ser_base_noflush_set_tty_state;
@@ -1297,6 +1299,7 @@ _initialize_ser_windows (void)
   ops->send_break = ser_base_send_break;
   ops->go_raw = ser_base_raw;
   ops->get_tty_state = ser_base_get_tty_state;
+  ops->copy_tty_state = ser_base_copy_tty_state;
   ops->set_tty_state = ser_base_set_tty_state;
   ops->print_tty_state = ser_base_print_tty_state;
   ops->noflush_set_tty_state = ser_base_noflush_set_tty_state;
@@ -1331,6 +1334,7 @@ _initialize_ser_windows (void)
   ops->send_break = ser_tcp_send_break;
   ops->go_raw = ser_base_raw;
   ops->get_tty_state = ser_base_get_tty_state;
+  ops->copy_tty_state = ser_base_copy_tty_state;
   ops->set_tty_state = ser_base_set_tty_state;
   ops->print_tty_state = ser_base_print_tty_state;
   ops->noflush_set_tty_state = ser_base_noflush_set_tty_state;
Index: gdb/ser-pipe.c
===================================================================
RCS file: /cvs/src/src/gdb/ser-pipe.c,v
retrieving revision 1.31
diff -u -p -r1.31 ser-pipe.c
--- gdb/ser-pipe.c	11 Jan 2011 21:53:23 -0000	1.31
+++ gdb/ser-pipe.c	4 Mar 2011 15:58:35 -0000
@@ -214,6 +214,7 @@ _initialize_ser_pipe (void)
   ops->send_break = ser_base_send_break;
   ops->go_raw = ser_base_raw;
   ops->get_tty_state = ser_base_get_tty_state;
+  ops->copy_tty_state = ser_base_copy_tty_state;
   ops->set_tty_state = ser_base_set_tty_state;
   ops->print_tty_state = ser_base_print_tty_state;
   ops->noflush_set_tty_state = ser_base_noflush_set_tty_state;
Index: gdb/ser-tcp.c
===================================================================
RCS file: /cvs/src/src/gdb/ser-tcp.c,v
retrieving revision 1.38
diff -u -p -r1.38 ser-tcp.c
--- gdb/ser-tcp.c	11 Jan 2011 21:53:23 -0000	1.38
+++ gdb/ser-tcp.c	4 Mar 2011 15:58:35 -0000
@@ -390,6 +390,7 @@ _initialize_ser_tcp (void)
   ops->send_break = ser_tcp_send_break;
   ops->go_raw = ser_base_raw;
   ops->get_tty_state = ser_base_get_tty_state;
+  ops->copy_tty_state = ser_base_copy_tty_state;
   ops->set_tty_state = ser_base_set_tty_state;
   ops->print_tty_state = ser_base_print_tty_state;
   ops->noflush_set_tty_state = ser_base_noflush_set_tty_state;
Index: gdb/ser-unix.c
===================================================================
RCS file: /cvs/src/src/gdb/ser-unix.c,v
retrieving revision 1.38
diff -u -p -r1.38 ser-unix.c
--- gdb/ser-unix.c	11 Jan 2011 21:53:23 -0000	1.38
+++ gdb/ser-unix.c	4 Mar 2011 15:58:36 -0000
@@ -188,6 +188,17 @@ hardwire_get_tty_state (struct serial *s
   return (serial_ttystate) state;
 }
 
+static serial_ttystate
+hardwire_copy_tty_state (struct serial *scb, serial_ttystate ttystate)
+{
+  struct hardwire_ttystate *state;
+
+  state = (struct hardwire_ttystate *) xmalloc (sizeof *state);
+  *state = *(struct hardwire_ttystate *) ttystate;
+
+  return (serial_ttystate) state;
+}
+
 static int
 hardwire_set_tty_state (struct serial *scb, serial_ttystate ttystate)
 {
@@ -911,6 +922,7 @@ _initialize_ser_hardwire (void)
   ops->send_break = hardwire_send_break;
   ops->go_raw = hardwire_raw;
   ops->get_tty_state = hardwire_get_tty_state;
+  ops->copy_tty_state = hardwire_copy_tty_state;
   ops->set_tty_state = hardwire_set_tty_state;
   ops->print_tty_state = hardwire_print_tty_state;
   ops->noflush_set_tty_state = hardwire_noflush_set_tty_state;
Index: gdb/serial.c
===================================================================
RCS file: /cvs/src/src/gdb/serial.c,v
retrieving revision 1.43
diff -u -p -r1.43 serial.c
--- gdb/serial.c	27 Feb 2011 16:25:37 -0000	1.43
+++ gdb/serial.c	4 Mar 2011 15:58:36 -0000
@@ -493,6 +493,12 @@ serial_get_tty_state (struct serial *scb
   return scb->ops->get_tty_state (scb);
 }
 
+serial_ttystate
+serial_copy_tty_state (struct serial *scb, serial_ttystate ttystate)
+{
+  return scb->ops->copy_tty_state (scb, ttystate);
+}
+
 int
 serial_set_tty_state (struct serial *scb, serial_ttystate ttystate)
 {
Index: gdb/serial.h
===================================================================
RCS file: /cvs/src/src/gdb/serial.h,v
retrieving revision 1.27
diff -u -p -r1.27 serial.h
--- gdb/serial.h	11 Jan 2011 21:53:23 -0000	1.27
+++ gdb/serial.h	4 Mar 2011 15:58:36 -0000
@@ -129,6 +129,12 @@ extern void serial_raw (struct serial *s
 
 extern serial_ttystate serial_get_tty_state (struct serial *scb);
 
+/* Return a pointer to a newly malloc'd ttystate containing a copy
+   of the state in TTYSTATE.  */
+
+extern serial_ttystate serial_copy_tty_state (struct serial *scb,
+					      serial_ttystate ttystate);
+
 /* Set the state of the tty to TTYSTATE.  The change is immediate.
    When changing to or from raw mode, input might be discarded.
    Returns 0 for success, negative value for error (in which case
@@ -250,6 +256,7 @@ struct serial_ops
     int (*send_break) (struct serial *);
     void (*go_raw) (struct serial *);
     serial_ttystate (*get_tty_state) (struct serial *);
+    serial_ttystate (*copy_tty_state) (struct serial *, serial_ttystate);
     int (*set_tty_state) (struct serial *, serial_ttystate);
     void (*print_tty_state) (struct serial *, serial_ttystate,
 			     struct ui_file *);
-- 
  Dr. Ulrich Weigand
  GNU Toolchain for Linux on System z and Cell BE
  Ulrich.Weigand@de.ibm.com