Fix memory corruption in Guile command interface

Message ID 87k2ypp40d.fsf@igalia.com
State New, archived
Headers

Commit Message

Andy Wingo March 10, 2015, 9:59 a.m. UTC
  From 1edd8ea75766ab3c10cd0b4e0ce33a6c1274de21 Mon Sep 17 00:00:00 2001
From: Andy Wingo <wingo@igalia.com>
Date: Tue, 10 Mar 2015 10:56:54 +0100
Subject: [PATCH] Fix memory corruption in Guile command interface

Re-registering a command will delete previous commands of the same name,
running the destroyer for the command object.  The Guile destroyer
incorrectly tried to xfree the name and other strings, which is invalid
as they are on the GC heap.

gdb/ChangeLog:

	* guile/scm-cmd.c (cmdscm_destroyer): Don't xfree the name and
	other strings, as these are on the GC'd heap, and will be
	collected along with the smob.
---
 gdb/ChangeLog       | 6 ++++++
 gdb/guile/scm-cmd.c | 5 -----
 2 files changed, 6 insertions(+), 5 deletions(-)
  

Comments

Doug Evans March 10, 2015, 4:13 p.m. UTC | #1
On Tue, Mar 10, 2015 at 2:59 AM, Andy Wingo <wingo@igalia.com> wrote:
> From 1edd8ea75766ab3c10cd0b4e0ce33a6c1274de21 Mon Sep 17 00:00:00 2001
> From: Andy Wingo <wingo@igalia.com>
> Date: Tue, 10 Mar 2015 10:56:54 +0100
> Subject: [PATCH] Fix memory corruption in Guile command interface
>
> Re-registering a command will delete previous commands of the same name,
> running the destroyer for the command object.  The Guile destroyer
> incorrectly tried to xfree the name and other strings, which is invalid
> as they are on the GC heap.
>
> gdb/ChangeLog:
>
>         * guile/scm-cmd.c (cmdscm_destroyer): Don't xfree the name and
>         other strings, as these are on the GC'd heap, and will be
>         collected along with the smob.
> ---
>  gdb/ChangeLog       | 6 ++++++
>  gdb/guile/scm-cmd.c | 5 -----
>  2 files changed, 6 insertions(+), 5 deletions(-)
>
> diff --git a/gdb/ChangeLog b/gdb/ChangeLog
> index a0bfe3d..7c1bda6 100644
> --- a/gdb/ChangeLog
> +++ b/gdb/ChangeLog
> @@ -1,3 +1,9 @@
> +2015-03-10  Andy Wingo  <wingo@igalia.com>
> +
> +       * guile/scm-cmd.c (cmdscm_destroyer): Don't xfree the name and
> +       other strings, as these are on the GC'd heap, and will be
> +       collected along with the smob.
> +
>  2015-03-05  Andy Wingo  <wingo@igalia.com>
>
>         * guile/scm-symbol.c (gdbscm_lookup_symbol): Don't error if there
> diff --git a/gdb/guile/scm-cmd.c b/gdb/guile/scm-cmd.c
> index 7c6d010..0fa6cca 100644
> --- a/gdb/guile/scm-cmd.c
> +++ b/gdb/guile/scm-cmd.c
> @@ -286,11 +286,6 @@ cmdscm_destroyer (struct cmd_list_element *self, void *context)
>    command_smob *c_smob = (command_smob *) context;
>
>    cmdscm_release_command (c_smob);
> -
> -  /* We allocated the name, doc string, and perhaps the prefix name.  */
> -  xfree ((char *) self->name);
> -  xfree ((char *) self->doc);
> -  xfree ((char *) self->prefixname);
>  }
>
>  /* Called by gdb to invoke the command.  */

LGTM
  

Patch

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index a0bfe3d..7c1bda6 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,9 @@ 
+2015-03-10  Andy Wingo  <wingo@igalia.com>
+
+	* guile/scm-cmd.c (cmdscm_destroyer): Don't xfree the name and
+	other strings, as these are on the GC'd heap, and will be
+	collected along with the smob.
+
 2015-03-05  Andy Wingo  <wingo@igalia.com>
 
 	* guile/scm-symbol.c (gdbscm_lookup_symbol): Don't error if there
diff --git a/gdb/guile/scm-cmd.c b/gdb/guile/scm-cmd.c
index 7c6d010..0fa6cca 100644
--- a/gdb/guile/scm-cmd.c
+++ b/gdb/guile/scm-cmd.c
@@ -286,11 +286,6 @@  cmdscm_destroyer (struct cmd_list_element *self, void *context)
   command_smob *c_smob = (command_smob *) context;
 
   cmdscm_release_command (c_smob);
-
-  /* We allocated the name, doc string, and perhaps the prefix name.  */
-  xfree ((char *) self->name);
-  xfree ((char *) self->doc);
-  xfree ((char *) self->prefixname);
 }
 
 /* Called by gdb to invoke the command.  */