Subject: Re: [PATCH] ARM64: Clear out any singlestep state on a ptrace detach
operation
From: Will Deacon <will.dea...@arm.com>
Date: 12/04/2015 04:03 AM
To: "Blackwood, John" <john.blackw...@ccur.com>
CC: "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>, "o...@redhat.com"
<o...@redhat.com>, "fweis...@gmail.com" <fweis...@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 <unistd.h>
#include <stdio.h>
#include <errno.h>
#include <sys/ptrace.h>
#include <sys/types.h>
#include <sys/wait.h>
#include <string.h>
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 majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
