<div dir="ltr"> <div class="gmail_extra"> <div class="gmail_quote">On 10 January 2015 at 01:05, Bryce Harrington <<a href="mailto:bryce@osg.samsung.com" target="_blank">bryce@osg.samsung.com</a>> wrote: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">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." This prctl call has no effect if Yama LSM is not loaded. 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. This patch can be tested by running sanity-test with /proc/sys/kernel/yama/ptrace_scope set to 0 or 1; the test must pass for either value. 1: <a href="http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=2d514487faf188938a4ee4fb3464eeecfbdcf8eb" target="_blank">http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=2d514487faf188938a4ee4fb3464eeecfbdcf8eb</a> 2: <a href="https://wiki.ubuntu.com/SecurityTeam/Roadmap/KernelHardening#ptrace_Protection" target="_blank">https://wiki.ubuntu.com/SecurityTeam/Roadmap/KernelHardening#ptrace_Protection</a> Signed-off-by: Bryce Harrington <<a href="mailto:bryce@osg.samsung.com">bryce@osg.samsung.com</a>> Reviewed-by: Marek Chalupa <<a href="mailto:mchqwerty@gmail.com">mchqwerty@gmail.com</a>> --- tests/test-runner.c | 41 +++++++++++++++++++++++++++++++++++++++-- 1 file changed, 39 insertions(+), 2 deletions(-) diff --git a/tests/test-runner.c b/tests/test-runner.c index 48e9a22..4750038 100644 --- a/tests/test-runner.c +++ b/tests/test-runner.c @@ -34,6 +34,10 @@ #include <errno.h> #include <limits.h> #include <sys/ptrace.h> +#include <sys/prctl.h> +#ifndef PR_SET_PTRACER +# define PR_SET_PTRACER 0x59616d61 +#endif #include "test-runner.h" @@ -249,16 +253,30 @@ stderr_reset_color(void) /* this function is taken from libinput/test/litest.c * (rev 028513a0a723e97941c39) + * + * Returns: 1 if a debugger is confirmed present; 0 if no debugger is + * present or if it can't be determined. */ static int is_debugger_attached(void) { int status; int rc; - pid_t pid = fork(); + pid_t pid; + int pipefd[2]; + + if (pipe(pipefd) == -1) { + perror("pipe"); + return 0; + } - if (pid == -1) + pid = fork(); + if (pid == -1) { + perror("fork"); + close(pipefd[0]); + close(pipefd[1]); return 0; + } if (pid == 0) { pid_t ppid = getppid(); @@ -272,6 +290,25 @@ is_debugger_attached(void) } _exit(rc); } else { + close(pipefd[0]); + + /* Enable child to ptrace the parent process */ + rc = prctl(PR_SET_PTRACER, pid); + if (rc != 0 && rc != EINVAL) { + /* An error prevents us from telling if a debugger is attached. + * Instead of propagating the error, assume no debugger present. + * But note the error to the log as a clue for troubleshooting. + */ + perror("prctl"); + close(pipefd[1]); + return 0; </blockquote><div> </div><div>Here we're creating a zombie, aren't we? </div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> + } + + /* Signal to client that parent is ready. No need to write + * anything to the child; the closure of the pipe will be + * sufficient to signal we're ready. */ + close(pipefd[1]); + waitpid(pid, &status, 0); rc = WEXITSTATUS(status); } -- 1.9.1 _______________________________________________ wayland-devel mailing list <a href="mailto:wayland-devel@lists.freedesktop.org">wayland-devel@lists.freedesktop.org</a> <a href="http://lists.freedesktop.org/mailman/listinfo/wayland-devel" target="_blank">http://lists.freedesktop.org/mailman/listinfo/wayland-devel</a> </blockquote><div> </div><div>Thanks for the added comments :) </div><div>Marek </div></div> </div></div>