From: Tom Tromey <tom@tromey.com>
Date: Thu, 9 Jun 2016 15:46:53 +0000 (-0600)
Subject: PR gdb/17210 - fix possible memory leak in read_memory_robust
X-Git-Url: https://git.libre-soc.org/?a=commitdiff_plain;h=9d78f827e0da9ab6fda2d6ef2d59cebb805b411f;p=binutils-gdb.git

PR gdb/17210 - fix possible memory leak in read_memory_robust

PR gdb/17210 concerns a possible memory leak in read_memory_robust.
The bug can happen because read_memory_robust allocates memory, does
not install any cleanups, and invokes QUIT.  Similarly, target_read
calls QUIT, so it too can potentially throw.

The fix is to install cleanups to guard the allocated memory.

Built and regtested on x86-64 Fedora 23.  I couldn't think of a way to
test this, so no new test; and of course this means it should have
more careful review.

2016-06-29  Tom Tromey  <tom@tromey.com>

	PR gdb/17210:
	* target.c (free_memory_read_result_vector): Take a pointer to the
	VEC as an argument.
	(read_memory_robust): Install a cleanup for "result".
	* mi/mi-main.c (mi_cmd_data_read_memory_bytes): Update.
---

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 97b609bf2f5..6b65cc486ff 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,11 @@
+2016-06-29  Tom Tromey  <tom@tromey.com>
+
+	PR gdb/17210:
+	* target.c (free_memory_read_result_vector): Take a pointer to the
+	VEC as an argument.
+	(read_memory_robust): Install a cleanup for "result".
+	* mi/mi-main.c (mi_cmd_data_read_memory_bytes): Update.
+
 2016-06-29  Manish Goregaokar  <manish@mozilla.com>
 
 gdb/ChangeLog:
diff --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c
index 188547bc52e..b1cbd8b0baa 100644
--- a/gdb/mi/mi-main.c
+++ b/gdb/mi/mi-main.c
@@ -1636,7 +1636,7 @@ mi_cmd_data_read_memory_bytes (char *command, char **argv, int argc)
 
   result = read_memory_robust (current_target.beneath, addr, length);
 
-  cleanups = make_cleanup (free_memory_read_result_vector, result);
+  cleanups = make_cleanup (free_memory_read_result_vector, &result);
 
   if (VEC_length (memory_read_result_s, result) == 0)
     error (_("Unable to read memory."));
diff --git a/gdb/target.c b/gdb/target.c
index 6f69ac37a1f..bb86adf9b70 100644
--- a/gdb/target.c
+++ b/gdb/target.c
@@ -1825,15 +1825,15 @@ read_whatever_is_readable (struct target_ops *ops,
 void
 free_memory_read_result_vector (void *x)
 {
-  VEC(memory_read_result_s) *v = (VEC(memory_read_result_s) *) x;
+  VEC(memory_read_result_s) **v = (VEC(memory_read_result_s) **) x;
   memory_read_result_s *current;
   int ix;
 
-  for (ix = 0; VEC_iterate (memory_read_result_s, v, ix, current); ++ix)
+  for (ix = 0; VEC_iterate (memory_read_result_s, *v, ix, current); ++ix)
     {
       xfree (current->data);
     }
-  VEC_free (memory_read_result_s, v);
+  VEC_free (memory_read_result_s, *v);
 }
 
 VEC(memory_read_result_s) *
@@ -1842,6 +1842,8 @@ read_memory_robust (struct target_ops *ops,
 {
   VEC(memory_read_result_s) *result = 0;
   int unit_size = gdbarch_addressable_memory_unit_size (target_gdbarch ());
+  struct cleanup *cleanup = make_cleanup (free_memory_read_result_vector,
+					  &result);
 
   LONGEST xfered_total = 0;
   while (xfered_total < len)
@@ -1868,6 +1870,7 @@ read_memory_robust (struct target_ops *ops,
 	{
 	  LONGEST to_read = min (len - xfered_total, region_len);
 	  gdb_byte *buffer = (gdb_byte *) xmalloc (to_read * unit_size);
+	  struct cleanup *inner_cleanup = make_cleanup (xfree, buffer);
 
 	  LONGEST xfered_partial =
 	      target_read (ops, TARGET_OBJECT_MEMORY, NULL,
@@ -1878,7 +1881,7 @@ read_memory_robust (struct target_ops *ops,
 	    {
 	      /* Got an error reading full chunk.  See if maybe we can read
 		 some subrange.  */
-	      xfree (buffer);
+	      do_cleanups (inner_cleanup);
 	      read_whatever_is_readable (ops, offset + xfered_total,
 					 offset + xfered_total + to_read,
 					 unit_size, &result);
@@ -1887,6 +1890,8 @@ read_memory_robust (struct target_ops *ops,
 	  else
 	    {
 	      struct memory_read_result r;
+
+	      discard_cleanups (inner_cleanup);
 	      r.data = buffer;
 	      r.begin = offset + xfered_total;
 	      r.end = r.begin + xfered_partial;
@@ -1896,6 +1901,8 @@ read_memory_robust (struct target_ops *ops,
 	  QUIT;
 	}
     }
+
+  discard_cleanups (cleanup);
   return result;
 }