Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756385AbbLDVnF (ORCPT ); Fri, 4 Dec 2015 16:43:05 -0500 Received: from flmx07.ccur.com ([12.220.59.12]:3756 "EHLO flmx07.ccur.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755010AbbLDVnD (ORCPT ); Fri, 4 Dec 2015 16:43:03 -0500 From: John Blackwood Subject: Re: [PATCH] ARM64: Clear out any singlestep state on a ptrace detach operation To: Will Deacon CC: "linux-kernel@vger.kernel.org" , "oleg@redhat.com" , "fweisbec@gmail.com" Message-ID: <566208E1.4050703@ccur.com> Date: Fri, 4 Dec 2015 15:42:57 -0600 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.4.0 MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8"; format=flowed Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 5366 Lines: 156 > Subject: Re: [PATCH] ARM64: Clear out any singlestep state on a ptrace detach operation > From: Will Deacon > Date: 12/04/2015 04:03 AM > To: "Blackwood, John" > CC: "linux-kernel@vger.kernel.org" , "oleg@redhat.com" , "fweisbec@gmail.com" > > On Thu, Dec 03, 2015 at 02:05:31PM -0600, John Blackwood wrote: > > Hello Will, > > Hi John, > > Thanks for the patch. > > > I have a patch for a ptrace(2) issue that we encountered on arm64 kernels. > > If a debugger singlesteps a ptraced task, and then does a ptrace(2) > > PTRACE_DETACH command, the task will not resume successfully. It seems > > that clearing out the singlestep state, as something like a ptrace(2) > > PTRACE_CONT does, gets this working. > > > > Thank you for your time and considerations. > > I think you're correct that we should be doing this, but actually, why > isn't this done by the core code? It looks to me like this should be > part of ptrace_detach, just like it is part of ptrace_resume when a > PTRACE_CONT request is handled. That would also be a step towards making > ptrace_disable optional (since x86 and powerpc are doing stuff that could > be done in core code too). > > I hacked up a quick patch below (not even compile-tested), but I'm not > sure what to do about hardware {break,watch}points. Some architectures > explicitly clear those on detach, whereas others appear to leave them > alone. Thoughts? Hi Will, Thanks for looking into this issue. I can see your point about possibly having the common code handle the singlestep cleanup on detach. However, I'm a newbie to arm64, and also not knowledgeable at all in any of the other architectures, so I wouldn't be able to comment on whether you patch is a good fit for all other architectures. I took a look at the hardware breakpoints on x86, and indeed, it is up to the debugger to clear out any current hardware breakpoints (and software breakpoints for that matter) before doing a PTRACE_DETACH. Otherwise, the previously ptraced task(s) may hit leftover breakpoints and SIGTRAP and die. I wrote an x86 test that sets a hw breakpoint and then detaches and the ptraced task will/can hit the breakpoint and die with a SIGTRAP. The only clearing of hw breakpoints is when a task execs or when a task forks and the new task will not inherit the task->ptrace_bps[] values. The kernel's perf event support is used to context switch in and out a task's hw breakpoint state and unregistering this perf event is only done in flush_thread() during execs. However, unlike the situation with detaching after an arm64 singlestep operation occurred, the debugger does have the opportunity to remove the hw/sw breakpoints before detaching if it wants the task to be able to continue on successfully. Just in case you are interested, I have below a simple test that shows the arm64 singlestep/detach sequence issue. When the singlestep state is not cleaned up, then the 'PASSED' echo will not show up. Thanks again for your time and considerations. ----- /* * Singlestep detach test * If sucessful, 'PASSED' should be seen when executing this test. * Checks for a kernel bug in arm64 where detaching after a singlestep * would cause the previously ptraced task to send itself a SIGTRAP * signal and die. */ #include #include #include #include #include #include #include static int inferior_wait (int inferior) { int wait_status; int status = waitpid(inferior, &wait_status, 0); if (status == -1) { perror("waitpid"); return 1; } printf("pid: %d ", status); if (WIFEXITED(wait_status)) { printf("exited with status %d", WEXITSTATUS(wait_status)); } else if (WIFSIGNALED(wait_status)) { printf("terminated with signal %d %s", WTERMSIG(wait_status), strsignal(WTERMSIG(wait_status))); } else if (WIFSTOPPED(wait_status)) { printf("stopped with signal %d %s", WSTOPSIG(wait_status), strsignal(WSTOPSIG(wait_status))); } else { printf("don't understand wait status"); } printf("\n"); return 0; } #define unused __attribute__((unused)) int main(int unused argc, unused char **argv) { int status, inferior; inferior = fork(); if (inferior == -1) { perror("fork"); return 1; } if (inferior == 0) { status = ptrace(PTRACE_TRACEME, 0, 0, 0); if (status == -1) { perror("inferior ptrace(PTRACE_TRACEME,...)"); return 1; } execl("/bin/echo", "/bin/echo", "PASSED", NULL); perror("inferior execl"); return 1; } /* Parent (debugger) from here on */ status = inferior_wait(inferior); if (status != 0) return status; status = ptrace(PTRACE_SINGLESTEP, inferior, 0, 0); if (status == -1) { perror("ptrace(PTRACE_SINGLE_STEP,...)"); return 1; } status = inferior_wait(inferior); if (status != 0) return status; status = ptrace(PTRACE_DETACH, inferior, 0, 0); if (status == -1) { perror("ptrace(PTRACE_DETACH,...)"); return 1; } return 0; } -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/