From 9d78f827e0da9ab6fda2d6ef2d59cebb805b411f Mon Sep 17 00:00:00 2001 From: Tom Tromey Date: Thu, 9 Jun 2016 09:46:53 -0600 Subject: [PATCH] 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 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. --- gdb/ChangeLog | 8 ++++++++ gdb/mi/mi-main.c | 2 +- gdb/target.c | 15 +++++++++++---- 3 files changed, 20 insertions(+), 5 deletions(-) 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 + + 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 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; } -- 2.30.2