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