diff mbox series

[v2] Fix prctl02

Message ID 20200123151836.29484-1-mdoucha@suse.cz
State Accepted
Headers show
Series [v2] Fix prctl02 | expand

Commit Message

Martin Doucha Jan. 23, 2020, 3:18 p.m. UTC
The prctl() system call takes 5 integer arguments but only 3 of them were
passed in the test. This means that the system call read random garbage
from stack in place of the two missing arguments and failed even on some
perfectly valid combinations of arguments on some platforms.

- Change num_invalid to ULONG_MAX
- Fix arguments in test case 9, 13 and 14
- Fix test call of prctl() to have all 5 arguments

Signed-off-by: Martin Doucha <mdoucha@suse.cz>
CC: Yang Xu <xuyang2018.jy@cn.fujitsu.com>
---

Changes since v1:
- Change num_invalid to ULONG_MAX
- Return removed test cases and fix them instead

 testcases/kernel/syscalls/prctl/prctl02.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

Comments

Jan Stancek Jan. 23, 2020, 11:54 p.m. UTC | #1
----- Original Message -----
> The prctl() system call takes 5 integer arguments but only 3 of them were
> passed in the test. This means that the system call read random garbage
> from stack in place of the two missing arguments and failed even on some
> perfectly valid combinations of arguments on some platforms.
> 
> - Change num_invalid to ULONG_MAX
> - Fix arguments in test case 9, 13 and 14
> - Fix test call of prctl() to have all 5 arguments
> 
> Signed-off-by: Martin Doucha <mdoucha@suse.cz>
> CC: Yang Xu <xuyang2018.jy@cn.fujitsu.com>

Acked-by: Jan Stancek <jstancek@redhat.com>
xuyang Jan. 24, 2020, 11:10 a.m. UTC | #2
Hi
> The prctl() system call takes 5 integer arguments but only 3 of them were
> passed in the test. This means that the system call read random garbage
> from stack in place of the two missing arguments and failed even on some
> perfectly valid combinations of arguments on some platforms.
> 
> - Change num_invalid to ULONG_MAX
> - Fix arguments in test case 9, 13 and 14
> - Fix test call of prctl() to have all 5 arguments
looks prctl manpages and kernel code, you are right, Thanks for the fix!
Feel free to add,
Reviewed-by: xuyang_jy_0410@163.com
Tested-by: xuyang_jy_0410@163.com

Also, do we should use 5 arguments for other prctl test cases?

Best Regards
Yang Xu
> 
> Signed-off-by: Martin Doucha <mdoucha@suse.cz>
> CC: Yang Xu <xuyang2018.jy@cn.fujitsu.com>
> ---
>  
> Changes since v1:
> - Change num_invalid to ULONG_MAX
> - Return removed test cases and fix them instead
> 
>   testcases/kernel/syscalls/prctl/prctl02.c | 11 ++++++-----
>   1 file changed, 6 insertions(+), 5 deletions(-)
> 
> diff --git a/testcases/kernel/syscalls/prctl/prctl02.c b/testcases/kernel/syscalls/prctl/prctl02.c
> index 93f30b54a..ebc0e5060 100644
> --- a/testcases/kernel/syscalls/prctl/prctl02.c
> +++ b/testcases/kernel/syscalls/prctl/prctl02.c
> @@ -41,6 +41,7 @@
>   #include <unistd.h>
>   #include <stdlib.h>
>   #include <stddef.h>
> +#include <limits.h>
>   #include "config.h"
>   #include "lapi/prctl.h"
>   #include "lapi/seccomp.h"
> @@ -65,7 +66,7 @@ static unsigned long bad_addr;
>   static unsigned long num_0;
>   static unsigned long num_1 = 1;
>   static unsigned long num_2 = 2;
> -static unsigned long num_invalid = 999;
> +static unsigned long num_invalid = ULONG_MAX;
>   static int seccomp_nsup;
>   static int nonewprivs_nsup;
>   static int thpdisable_nsup;
> @@ -87,12 +88,12 @@ static struct tcase {
>   	{PR_SET_SECCOMP, &num_2, &strict_addr, EACCES, "PR_SET_SECCOMP"},
>   	{PR_SET_TIMING, &num_1, &num_0, EINVAL, "PR_SET_TIMING"},
>   	{PR_SET_NO_NEW_PRIVS, &num_0, &num_0, EINVAL, "PR_SET_NO_NEW_PRIVS"},
> -	{PR_SET_NO_NEW_PRIVS, &num_1, &num_0, EINVAL, "PR_SET_NO_NEW_PRIVS"},
> +	{PR_SET_NO_NEW_PRIVS, &num_1, &num_1, EINVAL, "PR_SET_NO_NEW_PRIVS"},
>   	{PR_GET_NO_NEW_PRIVS, &num_1, &num_0, EINVAL, "PR_GET_NO_NEW_PRIVS"},
>   	{PR_SET_THP_DISABLE, &num_0, &num_1, EINVAL, "PR_SET_THP_DISABLE"},
>   	{PR_GET_THP_DISABLE, &num_1, &num_1, EINVAL, "PR_GET_THP_DISABLE"},
> -	{PR_CAP_AMBIENT, &num_2, &num_1, EINVAL, "PR_CAP_AMBIENT"},
> -	{PR_GET_SPECULATION_CTRL, &num_1, &num_0, EINVAL, "PR_GET_SPECULATION_CTRL"},
> +	{PR_CAP_AMBIENT, &num_invalid, &num_0, EINVAL, "PR_CAP_AMBIENT"},
> +	{PR_GET_SPECULATION_CTRL, &num_0, &num_invalid, EINVAL, "PR_GET_SPECULATION_CTRL"},
>   	{PR_SET_SECUREBITS, &num_0, &num_0, EPERM, "PR_SET_SECUREBITS"},
>   	{PR_CAPBSET_DROP, &num_1, &num_0, EPERM, "PR_CAPBSET_DROP"},
>   };
> @@ -140,7 +141,7 @@ static void verify_prctl(unsigned int n)
>   	break;
>   	}
>   
> -	TEST(prctl(tc->option, *tc->arg2, *tc->arg3));
> +	TEST(prctl(tc->option, *tc->arg2, *tc->arg3, 0, 0));
>   	if (TST_RET == 0) {
>   		tst_res(TFAIL, "prctl() succeeded unexpectedly");
>   		return;
>
Cyril Hrubis Jan. 24, 2020, 12:48 p.m. UTC | #3
Hi!
> > The prctl() system call takes 5 integer arguments but only 3 of them were
> > passed in the test. This means that the system call read random garbage
> > from stack in place of the two missing arguments and failed even on some
> > perfectly valid combinations of arguments on some platforms.
> > 
> > - Change num_invalid to ULONG_MAX
> > - Fix arguments in test case 9, 13 and 14
> > - Fix test call of prctl() to have all 5 arguments
> looks prctl manpages and kernel code, you are right, Thanks for the fix!
> Feel free to add??
> Reviewed-by: xuyang_jy_0410@163.com
> Tested-by: xuyang_jy_0410@163.com
> 
> Also, do we should use 5 arguments for other prctl test cases?

It depends on the option argument, the PR_CAP_AMBIENT explicitely states
that arg4 and arg5 must be set to 0 as well as the
PR_GET_SPECULATION_CTRL.

Some of the options explicitely say that some arguments are ignored.

And some, including PR_SET_NO_NEW_PRIVS does not say anyting.
xuyang Jan. 24, 2020, 1:06 p.m. UTC | #4
Hi!
> Hi!
>>> The prctl() system call takes 5 integer arguments but only 3 of them were
>>> passed in the test. This means that the system call read random garbage
>>> from stack in place of the two missing arguments and failed even on some
>>> perfectly valid combinations of arguments on some platforms.
>>>
>>> - Change num_invalid to ULONG_MAX
>>> - Fix arguments in test case 9, 13 and 14
>>> - Fix test call of prctl() to have all 5 arguments
>> looks prctl manpages and kernel code, you are right, Thanks for the fix!
>> Feel free to add??
>> Reviewed-by: xuyang_jy_0410@163.com
>> Tested-by: xuyang_jy_0410@163.com
>>
>> Also, do we should use 5 arguments for other prctl test cases?
> 
> It depends on the option argument, the PR_CAP_AMBIENT explicitely states
> that arg4 and arg5 must be set to 0 as well as the
> PR_GET_SPECULATION_CTRL.
> 
> Some of the options explicitely say that some arguments are ignored.
> 
> And some, including PR_SET_NO_NEW_PRIVS does not say anyting.
Thanks for your answer. I see.
>
Cyril Hrubis Jan. 24, 2020, 2:14 p.m. UTC | #5
Hi!
Pushed, thanks.
diff mbox series

Patch

diff --git a/testcases/kernel/syscalls/prctl/prctl02.c b/testcases/kernel/syscalls/prctl/prctl02.c
index 93f30b54a..ebc0e5060 100644
--- a/testcases/kernel/syscalls/prctl/prctl02.c
+++ b/testcases/kernel/syscalls/prctl/prctl02.c
@@ -41,6 +41,7 @@ 
 #include <unistd.h>
 #include <stdlib.h>
 #include <stddef.h>
+#include <limits.h>
 #include "config.h"
 #include "lapi/prctl.h"
 #include "lapi/seccomp.h"
@@ -65,7 +66,7 @@  static unsigned long bad_addr;
 static unsigned long num_0;
 static unsigned long num_1 = 1;
 static unsigned long num_2 = 2;
-static unsigned long num_invalid = 999;
+static unsigned long num_invalid = ULONG_MAX;
 static int seccomp_nsup;
 static int nonewprivs_nsup;
 static int thpdisable_nsup;
@@ -87,12 +88,12 @@  static struct tcase {
 	{PR_SET_SECCOMP, &num_2, &strict_addr, EACCES, "PR_SET_SECCOMP"},
 	{PR_SET_TIMING, &num_1, &num_0, EINVAL, "PR_SET_TIMING"},
 	{PR_SET_NO_NEW_PRIVS, &num_0, &num_0, EINVAL, "PR_SET_NO_NEW_PRIVS"},
-	{PR_SET_NO_NEW_PRIVS, &num_1, &num_0, EINVAL, "PR_SET_NO_NEW_PRIVS"},
+	{PR_SET_NO_NEW_PRIVS, &num_1, &num_1, EINVAL, "PR_SET_NO_NEW_PRIVS"},
 	{PR_GET_NO_NEW_PRIVS, &num_1, &num_0, EINVAL, "PR_GET_NO_NEW_PRIVS"},
 	{PR_SET_THP_DISABLE, &num_0, &num_1, EINVAL, "PR_SET_THP_DISABLE"},
 	{PR_GET_THP_DISABLE, &num_1, &num_1, EINVAL, "PR_GET_THP_DISABLE"},
-	{PR_CAP_AMBIENT, &num_2, &num_1, EINVAL, "PR_CAP_AMBIENT"},
-	{PR_GET_SPECULATION_CTRL, &num_1, &num_0, EINVAL, "PR_GET_SPECULATION_CTRL"},
+	{PR_CAP_AMBIENT, &num_invalid, &num_0, EINVAL, "PR_CAP_AMBIENT"},
+	{PR_GET_SPECULATION_CTRL, &num_0, &num_invalid, EINVAL, "PR_GET_SPECULATION_CTRL"},
 	{PR_SET_SECUREBITS, &num_0, &num_0, EPERM, "PR_SET_SECUREBITS"},
 	{PR_CAPBSET_DROP, &num_1, &num_0, EPERM, "PR_CAPBSET_DROP"},
 };
@@ -140,7 +141,7 @@  static void verify_prctl(unsigned int n)
 	break;
 	}
 
-	TEST(prctl(tc->option, *tc->arg2, *tc->arg3));
+	TEST(prctl(tc->option, *tc->arg2, *tc->arg3, 0, 0));
 	if (TST_RET == 0) {
 		tst_res(TFAIL, "prctl() succeeded unexpectedly");
 		return;