diff mbox series

kselftest/runner: allow to properly deliver signals to tests

Message ID 20200410100259.GA457752@xps-13 (mailing list archive)
State Mainlined
Commit 651e0d881461ab2b1cd5cbec3a642d22fc8d6057
Headers show
Series kselftest/runner: allow to properly deliver signals to tests | expand

Commit Message

Andrea Righi April 10, 2020, 10:02 a.m. UTC
While running seccomp_bpf, kill_after_ptrace() gets stuck if we run it
via /usr/bin/timeout (that is the default), until the timeout expires.

This is because /usr/bin/timeout is preventing to properly deliver
signals to ptrace'd children (SIGSYS in this case).

This problem can be easily reproduced by running:

 $ sudo make TARGETS=seccomp kselftest
 ...

 # [ RUN      ] TRACE_syscall.skip_a#
 not ok 1 selftests: seccomp: seccomp_bpf # TIMEOUT

The test is hanging at this point until the timeout expires and then it
reports the timeout error.

Prevent this problem by passing --foreground to /usr/bin/timeout,
allowing to properly deliver signals to children processes.

Signed-off-by: Andrea Righi <andrea.righi@canonical.com>
---
 tools/testing/selftests/kselftest/runner.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Kees Cook April 10, 2020, 8:33 p.m. UTC | #1
On Fri, Apr 10, 2020 at 12:02:59PM +0200, Andrea Righi wrote:
> While running seccomp_bpf, kill_after_ptrace() gets stuck if we run it
> via /usr/bin/timeout (that is the default), until the timeout expires.
> 
> This is because /usr/bin/timeout is preventing to properly deliver
> signals to ptrace'd children (SIGSYS in this case).
> 
> This problem can be easily reproduced by running:
> 
>  $ sudo make TARGETS=seccomp kselftest
>  ...
> 
>  # [ RUN      ] TRACE_syscall.skip_a#
>  not ok 1 selftests: seccomp: seccomp_bpf # TIMEOUT
> 
> The test is hanging at this point until the timeout expires and then it
> reports the timeout error.
> 
> Prevent this problem by passing --foreground to /usr/bin/timeout,
> allowing to properly deliver signals to children processes.

Interesting! This seems a poor behavior on "timeout"'s part. Is it
setting a signal mask? Hmpf. Thanks for tracking this down!  This seems
like the best solution now, so:

Acked-by: Kees Cook <keescook@chromium.org>

-Kees

> 
> Signed-off-by: Andrea Righi <andrea.righi@canonical.com>
> ---
>  tools/testing/selftests/kselftest/runner.sh | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tools/testing/selftests/kselftest/runner.sh b/tools/testing/selftests/kselftest/runner.sh
> index e84d901f8567..676b3a8b114d 100644
> --- a/tools/testing/selftests/kselftest/runner.sh
> +++ b/tools/testing/selftests/kselftest/runner.sh
> @@ -33,7 +33,7 @@ tap_timeout()
>  {
>  	# Make sure tests will time out if utility is available.
>  	if [ -x /usr/bin/timeout ] ; then
> -		/usr/bin/timeout "$kselftest_timeout" "$1"
> +		/usr/bin/timeout --foreground "$kselftest_timeout" "$1"
>  	else
>  		"$1"
>  	fi
> -- 
> 2.25.1
>
diff mbox series

Patch

diff --git a/tools/testing/selftests/kselftest/runner.sh b/tools/testing/selftests/kselftest/runner.sh
index e84d901f8567..676b3a8b114d 100644
--- a/tools/testing/selftests/kselftest/runner.sh
+++ b/tools/testing/selftests/kselftest/runner.sh
@@ -33,7 +33,7 @@  tap_timeout()
 {
 	# Make sure tests will time out if utility is available.
 	if [ -x /usr/bin/timeout ] ; then
-		/usr/bin/timeout "$kselftest_timeout" "$1"
+		/usr/bin/timeout --foreground "$kselftest_timeout" "$1"
 	else
 		"$1"
 	fi