linux-kernel - Re: [PATCH v11 2/2] selftests: add tests for clone3() with *set

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Mon, 18 Nov 2019 08:42:31 +0100
From:   Christian Brauner <christian.brauner@...ntu.com>
To:     Andrei Vagin <avagin@...il.com>
Cc:     Adrian Reber <areber@...hat.com>,
        Eric Biederman <ebiederm@...ssion.com>,
        Pavel Emelyanov <ovzxemul@...il.com>,
        Jann Horn <jannh@...gle.com>, Oleg Nesterov <oleg@...hat.com>,
        Dmitry Safonov <0x7f454c46@...il.com>,
        Rasmus Villemoes <linux@...musvillemoes.dk>,
        linux-kernel@...r.kernel.org, Mike Rapoport <rppt@...ux.ibm.com>,
        Radostin Stoyanov <rstoyanov1@...il.com>
Subject: Re: [PATCH v11 2/2] selftests: add tests for clone3() with *set_tid

On Sun, Nov 17, 2019 at 05:46:42PM -0800, Andrei Vagin wrote:
> On Fri, Nov 15, 2019 at 01:36:21PM +0100, Adrian Reber wrote:
> > +	if (!WIFEXITED(status))
> > +		ksft_test_result_fail("Child error\n");
> > +
> > +	if (WEXITSTATUS(status))
> > +		/*
> > +		 * Update the number of total tests with the tests from the
> > +		 * child processes.
> > +		 */
> > +		ksft_cnt.ksft_pass = WEXITSTATUS(status);
> 
> I just found that accounting of failed test cases in this test doesn't
> work properly. And if one of the test cases fails in a child process,
> the whole test will have the pass status.
> 
> 
> For example, if we add a fake fail:
> 
> diff --git a/tools/testing/selftests/clone3/clone3_set_tid.c b/tools/testing/selftests/clone3/clone3_set_tid.c
> index 3480e1c46..82c99c42f 100644
> --- a/tools/testing/selftests/clone3/clone3_set_tid.c
> +++ b/tools/testing/selftests/clone3/clone3_set_tid.c
> @@ -301,7 +301,7 @@ int main(int argc, char *argv[])
>                  * namespaces. Again assuming this is running in the host's
>                  * PID namespace. Not yet nested.
>                  */
> -               test_clone3_set_tid(set_tid, 4, CLONE_NEWPID, -EINVAL, 0, 0);
> +               test_clone3_set_tid(set_tid, 4, CLONE_NEWPID, -EPERM, 0, 0);
>  
>                 /*
>                  * This should work and from the parent we should see
> 
> $ make run_tests 
> ....
> # ok 21 [21104] Result (0) matches expectation (0)
> # # unshare PID namespace
> # # [21104] Trying clone3() with CLONE_SET_TID to 21106 and 0x0
> # # Invalid argument - Failed to create new process
> # # [21104] clone3() with CLONE_SET_TID 21106 says :-22 - expected -22
> # ok 22 [21104] Result (-22) matches expectation (-22)
> # # [21104] Child is ready and waiting
> # ok 26 PIDs in all namespaces as expected (21106,42,1)
> # # Planned tests != run tests (27 != 26)
> # # Pass 26 Fail 0 Xfail 0 Xpass 0 Skip 0 Error 0
> ok 3 selftests: clone3: clone3_set_tid

Thanks for reporting this.
With your patch series you just sent this problem should be addressed.

Thanks!
Christian