Just resending to wayland-devel... On 9 January 2015 at 11:26, Marek Chalupa <[email protected]> wrote:
> > > On 7 January 2015 at 21:34, Bryce Harrington <[email protected]> > wrote: > >> This fixes a regression in the testsuite since c3653f7f, where four of >> the timeout tests fail with "Timeouts suppressed" messages. >> >> The timeouts are being suppressed because the testsuite is erroneously >> detecting that a debugger is attached. This detection mechanism >> (adopted from libinput) uses ptrace to test if there is a debugger >> parent process that can be attached. Unfortunately, this is an >> unreliable test: Kernel security policies exist to restrict the scope of >> ptrace to prevent processes from snooping on one another.[1] This >> security policy is set as the default on Ubuntu, and potentially other >> Linux distributions.[2] >> >> The Yama documentation suggests, "For software that has defined >> application-specific relationships between a debugging process and its >> inferior (crash handlers, etc), prctl(PR_SET_PTRACER, pid, ...) can be >> used. An inferior can declare which other process (and its descendents) >> are allowed to call PTRACE_ATTACH against it." >> >> The child needs to be synchronized to the client to prevent a race >> condition where the child might try to operate before the parent has >> finished its prctl call. This synchronization is done via pipes. >> >> 1: >> http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=2d514487faf188938a4ee4fb3464eeecfbdcf8eb >> 2: >> https://wiki.ubuntu.com/SecurityTeam/Roadmap/KernelHardening#ptrace_Protection >> >> Signed-off-by: Bryce Harrington <[email protected]> >> --- >> tests/test-runner.c | 35 ++++++++++++++++++++++++++++++++--- >> 1 file changed, 32 insertions(+), 3 deletions(-) >> >> diff --git a/tests/test-runner.c b/tests/test-runner.c >> index b813b69..11d064e 100644 >> --- a/tests/test-runner.c >> +++ b/tests/test-runner.c >> @@ -34,6 +34,7 @@ >> #include <errno.h> >> #include <limits.h> >> #include <sys/ptrace.h> >> +#include <linux/prctl.h> >> > > Better use <sys/prctl.h>. prctl() is declared in sys/prctl.h. > linux/prctl.h only defines the values for arguments (and is included by > sys/prctl.h) > > >> >> #include "test-runner.h" >> >> @@ -255,13 +256,33 @@ is_debugger_attached(void) >> { >> int status; >> int rc; >> - pid_t pid = fork(); >> + pid_t pid; >> + int pipefd[2]; >> >> - if (pid == -1) >> + if (pipe(pipefd) == -1) { >> + perror("pipe"); >> return 0; >> + } >> + >> + pid = fork(); >> + if (pid == -1) { >> + perror("fork"); >> + close(pipefd[0]); >> + close(pipefd[1]); >> + return 0; >> + } >> >> if (pid == 0) { >> - pid_t ppid = getppid(); >> + char buf; >> + pid_t ppid; >> + >> + /* Wait until parent is ready */ >> + close(pipefd[1]); /* Close unused write end */ >> + while (read(pipefd[0], &buf, 1) > 0); >> > + close(pipefd[0]); >> + >> + /* Attempt to ptrace the parent */ >> + ppid = getppid(); >> rc = ptrace(PTRACE_ATTACH, ppid, NULL, NULL); >> if (rc == 0) { >> waitpid(ppid, NULL, 0); >> @@ -273,6 +294,14 @@ is_debugger_attached(void) >> } >> _exit(rc); >> } else { >> + close(pipefd[0]); >> + >> + /* Enable child to ptrace the parent process */ >> + prctl(PR_SET_PTRACER, pid); >> + >> + /* Signal to client that parent is ready */ >> + close(pipefd[1]); >> > > Maybe this would deserve better description, because signaling something > by closing the pipe is not clear IMO. > > > >> waitpid(pid, &status, 0); >> rc = WEXITSTATUS(status); >> } >> -- >> 1.9.1 >> >> _______________________________________________ >> wayland-devel mailing list >> [email protected] >> http://lists.freedesktop.org/mailman/listinfo/wayland-devel >> > > > For both patches: > > Reviewed-by: Marek Chalupa <[email protected]> > >
_______________________________________________ wayland-devel mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/wayland-devel
