From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 110071 invoked by alias); 10 Nov 2017 01:07:00 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 110060 invoked by uid 89); 10 Nov 2017 01:06:59 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-26.9 required=5.0 tests=BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,RP_MATCHES_RCVD,SPF_PASS autolearn=ham version=3.3.2 spammy=proved, H*u:6.1, H*UA:6.1, HContent-Transfer-Encoding:8bit X-HELO: aserp1040.oracle.com Received: from aserp1040.oracle.com (HELO aserp1040.oracle.com) (141.146.126.69) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Fri, 10 Nov 2017 01:06:57 +0000 Received: from userv0021.oracle.com (userv0021.oracle.com [156.151.31.71]) by aserp1040.oracle.com (Sentrion-MTA-4.3.2/Sentrion-MTA-4.3.2) with ESMTP id vAA16teD018875 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK) for ; Fri, 10 Nov 2017 01:06:56 GMT Received: from aserv0122.oracle.com (aserv0122.oracle.com [141.146.126.236]) by userv0021.oracle.com (8.14.4/8.14.4) with ESMTP id vAA16stl011916 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK) for ; Fri, 10 Nov 2017 01:06:55 GMT Received: from abhmp0003.oracle.com (abhmp0003.oracle.com [141.146.116.9]) by aserv0122.oracle.com (8.14.4/8.14.4) with ESMTP id vAA16s3t027186 for ; Fri, 10 Nov 2017 01:06:54 GMT Received: from [10.159.138.71] (/10.159.138.71) by default (Oracle Beehive Gateway v4.0) with ESMTP ; Thu, 09 Nov 2017 17:06:54 -0800 Subject: Re: [PING 2][PATCH PR gdb/21870] aarch64: Leftover uncleared debug registers From: Wei-min Pan To: "gdb-patches@sourceware.org" References: <1507328314-114545-1-git-send-email-weimin.pan@oracle.com> <300a7ff5-11c4-ef89-907a-c82c08973f29@oracle.com> Message-ID: <7c25007d-00a1-0456-6f51-0e9a39427785@oracle.com> Date: Fri, 10 Nov 2017 01:07:00 -0000 User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:52.0) Gecko/20100101 Thunderbird/52.4.0 MIME-Version: 1.0 In-Reply-To: <300a7ff5-11c4-ef89-907a-c82c08973f29@oracle.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit X-SW-Source: 2017-11/txt/msg00220.txt.bz2 On 10/20/2017 5:58 PM, Wei-min Pan wrote: > On 10/6/2017 3:18 PM, Weimin Pan wrote: >>      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(-) >> >> 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  >> + >> +    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  >>         * 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])); >