From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 15135 invoked by alias); 4 Mar 2011 17:52:23 -0000 Received: (qmail 14642 invoked by uid 22791); 4 Mar 2011 17:52:01 -0000 X-SWARE-Spam-Status: No, hits=-1.3 required=5.0 tests=AWL,BAYES_00,MSGID_FROM_MTA_HEADER,SPF_SOFTFAIL,T_RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from mtagate7.uk.ibm.com (HELO mtagate7.uk.ibm.com) (194.196.100.167) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Fri, 04 Mar 2011 17:51:51 +0000 Received: from d06nrmr1707.portsmouth.uk.ibm.com (d06nrmr1707.portsmouth.uk.ibm.com [9.149.39.225]) by mtagate7.uk.ibm.com (8.13.1/8.13.1) with ESMTP id p24Hpmsu013253 for ; Fri, 4 Mar 2011 17:51:48 GMT Received: from d06av02.portsmouth.uk.ibm.com (d06av02.portsmouth.uk.ibm.com [9.149.37.228]) by d06nrmr1707.portsmouth.uk.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id p24Hq0oK2007148 for ; Fri, 4 Mar 2011 17:52:00 GMT Received: from d06av02.portsmouth.uk.ibm.com (loopback [127.0.0.1]) by d06av02.portsmouth.uk.ibm.com (8.14.4/8.13.1/NCO v10.0 AVout) with ESMTP id p24HpmYY031042 for ; Fri, 4 Mar 2011 10:51:48 -0700 Received: from tuxmaker.boeblingen.de.ibm.com (tuxmaker.boeblingen.de.ibm.com [9.152.85.9]) by d06av02.portsmouth.uk.ibm.com (8.14.4/8.13.1/NCO v10.0 AVin) with SMTP id p24HpkaJ031028 for ; Fri, 4 Mar 2011 10:51:46 -0700 Message-Id: <201103041751.p24HpkaJ031028@d06av02.portsmouth.uk.ibm.com> Received: by tuxmaker.boeblingen.de.ibm.com (sSMTP sendmail emulation); Fri, 04 Mar 2011 18:51:46 +0100 Subject: [rfc] GDB memory corruption in multi-forks test case To: gdb-patches@sourceware.org Date: Fri, 04 Mar 2011 17:52:00 -0000 From: "Ulrich Weigand" MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Transfer-Encoding: 7bit Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2011-03/txt/msg00298.txt.bz2 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