Patchwork [PR,gdb/21870] aarch64: Leftover uncleared debug registers

login
register
mail settings
Submitter Weimin Pan
Date Oct. 7, 2017, 12:24 a.m.
Message ID <1507335892-115143-1-git-send-email-weimin.pan@oracle.com>
Download mbox | patch
Permalink /patch/23389/
State New
Headers show

Comments

Weimin Pan - Oct. 7, 2017, 12:24 a.m.
The root cause is that ptrace() does not validate either
    address or size when setting a hardware watchpoint/breakpoint.
    As a result, watchpoints were set at address 0, the initial value of
    aarch64_debug_reg_state in aarch64_process_info, when the
    PTRACE_SETREGSET request was first made in    
    aarch64_linux_set_debug_regs(), in preparation for resuming the thread.

    Other than changing the kernel ptrace() implementation, first attempt to
    fix this problem in gdb was to focus on aarch64_linux_new_thread().
    Instead of marking all hardware breakpoint/watchpoint register pairs for
    the new thread that have changed, tried to reflect the state by using
    either DR_MARK_ALL_CHANGED() if they have really been changed or
    DR_CLEAR_CHANGED() otherwise. But finding whether or not the registers
    have been changed by using parent's lwp_info or aarch64_process_info
    proved to be hard or incorrect, especially the latter which caused
    gdbserver to crash in the middle of the ptid_of_lwp() call.

    Another approach was then taken - add function initial_control_length()
    to validate the contents in the control registers, basing on the fact that
    the kernel only supports Byte Address Select (BAS) values of 0x1, 0x3, 0xf
    and 0xff, before calling ptrace() in aarch64_linux_set_debug_regs().

    Tested on aarch64-linux-gnu. No regressions.
---
 gdb/ChangeLog                    |  7 +++++++
 gdb/nat/aarch64-linux-hw-point.c | 18 +++++++++++++++++-
 2 files changed, 24 insertions(+), 1 deletion(-)

Patch

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index c4f55a8137..543e1a0487 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,10 @@ 
+2017-10-06  Weimin Pan  <weimin.pan@oracle.com>
+
+	PR gdb/21870
+	* nat/aarch64-linux-hw-point.c (aarch64_linux_set_debug_regs):
+	Call new function to validate the length in control registers.
+	(initial_control_length): New function.
+
 2017-09-15  Pedro Alves  <palves@redhat.com>
 
 	* compile/compile-c-types.c (convert_enum, convert_int)
diff --git a/gdb/nat/aarch64-linux-hw-point.c b/gdb/nat/aarch64-linux-hw-point.c
index 9800d9a59c..22c0a48c14 100644
--- a/gdb/nat/aarch64-linux-hw-point.c
+++ b/gdb/nat/aarch64-linux-hw-point.c
@@ -548,6 +548,22 @@  aarch64_handle_watchpoint (enum target_hw_bp_type type, CORE_ADDR addr,
 						state);
 }
 
+/* Validate the lengths of breakpoints/watchpoints, according to the
+   contents of these hardware debug control registers, and return
+   true if all these registers contain zero length.  */
+
+static bool
+initial_control_length (const unsigned int *ctrl, int count)
+{
+  for (int i = 0; i < count; i++)
+    {
+      if (DR_CONTROL_LENGTH (ctrl[i]))
+        return false;
+    }
+
+  return true;
+}
+
 /* Call ptrace to set the thread TID's hardware breakpoint/watchpoint
    registers with data from *STATE.  */
 
@@ -566,7 +582,7 @@  aarch64_linux_set_debug_regs (const struct aarch64_debug_reg_state *state,
   count = watchpoint ? aarch64_num_wp_regs : aarch64_num_bp_regs;
   addr = watchpoint ? state->dr_addr_wp : state->dr_addr_bp;
   ctrl = watchpoint ? state->dr_ctrl_wp : state->dr_ctrl_bp;
-  if (count == 0)
+  if (count == 0 || initial_control_length (ctrl, count))
     return;
   iov.iov_len = (offsetof (struct user_hwdebug_state, dbg_regs)
 		 + count * sizeof (regs.dbg_regs[0]));