From: Ben Pfaff <blp@gnu.org>
Date: Mon, 18 Jan 2010 23:59:56 +0000 (-0800)
Subject: message: Make msg_dup() copy and msg_destroy() free the file name.
X-Git-Tag: fc11-i386-build78~4
X-Git-Url: https://pintos-os.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=e2a7ada9e445ab2b1fd473345709c39157f30b9a;p=pspp-builds.git

message: Make msg_dup() copy and msg_destroy() free the file name.

This fixes a PSPPIRE bug in its message handling (found by inspection).  It
makes copies of messages to use later.  The file names were not being
copied, so in the meantime it was posible that they would be freed,
yielding a use-after-free error.

Since doing this sensibly required changing the file_name member of
struct msg_locator from "const char *" to "char *", it also touches up
places where this caused new warnings.
---

diff --git a/src/language/data-io/data-reader.c b/src/language/data-io/data-reader.c
index 6f620a6a..74e4c569 100644
--- a/src/language/data-io/data-reader.c
+++ b/src/language/data-io/data-reader.c
@@ -34,6 +34,7 @@
 #include <language/lexer/lexer.h>
 #include <language/prompt.h>
 #include <libpspp/assertion.h>
+#include <libpspp/cast.h>
 #include <libpspp/integer-format.h>
 #include <libpspp/message.h>
 #include <libpspp/str.h>
@@ -140,7 +141,7 @@ dfm_open_reader (struct file_handle *fh, struct lexer *lexer)
   if (fh_get_referent (fh) != FH_REF_INLINE)
     {
       struct stat s;
-      r->where.file_name = fh_get_file_name (fh);
+      r->where.file_name = CONST_CAST (char *, fh_get_file_name (fh));
       r->where.line_number = 0;
       r->file = fn_open (fh_get_file_name (fh),
                          fh_get_mode (fh) == FH_MODE_TEXT ? "r" : "rb");
diff --git a/src/libpspp/message.c b/src/libpspp/message.c
index 5faee1f1..2cd0bff5 100644
--- a/src/libpspp/message.c
+++ b/src/libpspp/message.c
@@ -79,19 +79,27 @@ msg_done (void)
 struct msg *
 msg_dup(const struct msg *m)
 {
-  struct msg *new_msg = xmalloc (sizeof *m);
+  struct msg *new_msg;
 
-  *new_msg = *m;
-  new_msg->text = xstrdup(m->text);
+  new_msg = xmemdup (m, sizeof *m);
+  if (m->where.file_name != NULL)
+    new_msg->where.file_name = xstrdup (m->where.file_name);
+  new_msg->text = xstrdup (m->text);
 
   return new_msg;
 }
 
+/* Frees a message created by msg_dup().
+
+   (Messages not created by msg_dup(), as well as their where.file_name
+   members, are typically not dynamically allocated, so this function should
+   not be used to destroy them.) */
 void
-msg_destroy(struct msg *m)
+msg_destroy (struct msg *m)
 {
-  free(m->text);
-  free(m);
+  free (m->where.file_name);
+  free (m->text);
+  free (m);
 }
 
 /* Emits M as an error message.
diff --git a/src/libpspp/message.h b/src/libpspp/message.h
index be4ba3e2..3aca7789 100644
--- a/src/libpspp/message.h
+++ b/src/libpspp/message.h
@@ -69,8 +69,8 @@ msg_class_from_category_and_severity (enum msg_category category,
 /* A file location.  */
 struct msg_locator
   {
-    const char *file_name;		/* File name. */
-    int line_number;			/* Line number. */
+    char *file_name;		/* File name. */
+    int line_number;            /* Line number. */
   };
 
 /* A message. */
diff --git a/src/libpspp/msg-locator.c b/src/libpspp/msg-locator.c
index 09550e29..fb8082e9 100644
--- a/src/libpspp/msg-locator.c
+++ b/src/libpspp/msg-locator.c
@@ -17,8 +17,9 @@
 #include <config.h>
 #include <stdlib.h>
 #include "msg-locator.h"
-#include <libpspp/message.h>
 #include <libpspp/assertion.h>
+#include <libpspp/cast.h>
+#include <libpspp/message.h>
 #include "getl.h"
 
 #include "xalloc.h"
@@ -77,7 +78,7 @@ get_msg_location (const struct source_stream *ss, struct msg_locator *loc)
     }
   else
     {
-      loc->file_name = getl_source_name (ss);
+      loc->file_name = CONST_CAST (char *, getl_source_name (ss));
       loc->line_number = getl_source_location (ss);
     }
 }