Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Date:   Thu, 9 Apr 2020 08:28:10 +0200
From:   Andrea Righi <andrea.righi@canonical.com>
To:     Kees Cook <keescook@chromium.org>
Cc:     Shuah Khan <shuah@kernel.org>,
        Thadeu Lima de Souza Cascardo <cascardo@canonical.com>,
        linux-kselftest@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] kselftest/runner: avoid using timeout when timeout is
 disabled
Message-ID: <20200409062810.GB49260@xps-13>
References: <20200327093620.GB1223497@xps-13>
 <202003271208.0D9A3A48CC@keescook>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <202003271208.0D9A3A48CC@keescook>
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Fri, Mar 27, 2020 at 12:28:05PM -0700, Kees Cook wrote:
> On Fri, Mar 27, 2020 at 10:36:20AM +0100, Andrea Righi wrote:
> > Avoid using /usr/bin/timeout unnecessarily if timeout is set to 0
> > (disabled) in the "settings" file for a specific test.
> 
> That seems to be a reasonable optimization, sure.
> 
> > NOTE: without this change (and adding timeout=0 in the corresponding
> > settings file - tools/testing/selftests/seccomp/settings) the
> > seccomp_bpf selftest is always failing with a timeout event during the
> > syscall_restart step.
> 
> This, however, is worrisome. I think there is something else wrong here.
> I will investigate why the output of seccomp_bpf is weird when running
> under the runner scripts. Hmmm. The output looks corrupted...
> 
> -Kees

Hi Kees,

a quick update on this.

After further investigation Cascardo (added in cc) found that the
culprit of this issue was the usage of nanosleep() vs clock_nanosleep()
in glibc. He already sent a fix for this:

https://lkml.org/lkml/2020/4/8/968

Without this we are getting the following error:

 seccomp_bpf.c:2839:global.syscall_restart:Expected true (1) == WIFSTOPPED(status) (0)
 # global.syscall_restart: Test terminated by assertion

I still think my timeout optimization patch can be useful, but for this
particular problem we should definitely apply Cascardo's fix.

Thanks,
-Andrea

> 
> > 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..2cd3c8def0f6 100644
> > --- a/tools/testing/selftests/kselftest/runner.sh
> > +++ b/tools/testing/selftests/kselftest/runner.sh
> > @@ -32,7 +32,7 @@ tap_prefix()
> >  tap_timeout()
> >  {
> >  	# Make sure tests will time out if utility is available.
> > -	if [ -x /usr/bin/timeout ] ; then
> > +	if [ -x /usr/bin/timeout ] && [ $kselftest_timeout -gt 0 ] ; then
> >  		/usr/bin/timeout "$kselftest_timeout" "$1"
> >  	else
> >  		"$1"
> > -- 
> > 2.25.1
> > 
> 
> -- 
> Kees Cook