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: sav-api~438
X-Git-Url: https://pintos-os.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=e2a7ada9e445ab2b1fd473345709c39157f30b9a;p=pspp

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 6f620a6a94..74e4c5692b 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 5faee1f1d6..2cd0bff591 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 be4ba3e21d..3aca77891e 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 09550e29ac..fb8082e921 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);
     }
 }