[LTP] [PATCH v4] syscalls/prctl02: add more error tests

[Yang Xu]

on 2019/11/12 0:31, Cyril Hrubis wrote:

> Hi!
>> +static const struct sock_fprog strict = {
>> +	.len = (unsigned short)ARRAY_SIZE(strict_filter),
>> +	.filter = (struct sock_filter *)strict_filter
>> +};
>> +
>> +static const struct sock_fprog *strict_addr = &strict;
> This should be:
>
> static unsigned long strict_addr = (unsigned long)&strict;

OK.

>
>> +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 struct tcase {
>>   	int option;
>> -	unsigned long arg2;
>> +	unsigned long *arg2;
>> +	unsigned long *arg3;
>>   	int exp_errno;
>>   } tcases[] = {
>> -	{OPTION_INVALID, 0, EINVAL},
>> -	{PR_SET_PDEATHSIG, INVALID_ARG, EINVAL},
>> +	{OPTION_INVALID, &num_1, &num_0, EINVAL},
>> +	{PR_SET_PDEATHSIG, &num_invalid, &num_0, EINVAL},
>> +	{PR_SET_DUMPABLE, &num_2, &num_0, EINVAL},
>> +	{PR_SET_NAME, &bad_addr, &num_0, EFAULT},
>> +	{PR_SET_SECCOMP, &num_2, &bad_addr, EFAULT},
>> +	{PR_SET_SECCOMP, &num_2, &strict_addr, EACCES},
>> +	{PR_SET_TIMING, &num_1, &num_0, EINVAL},
>> +#ifdef HAVE_DECL_PR_SET_NO_NEW_PRIVS
>> +	{PR_SET_NO_NEW_PRIVS, &num_0, &num_0, EINVAL},
>> +	{PR_SET_NO_NEW_PRIVS, &num_1, &num_0, EINVAL},
>> +	{PR_GET_NO_NEW_PRIVS, &num_1, &num_0, EINVAL},
>> +#endif
>> +#ifdef HAVE_DECL_PR_SET_THP_DISABLE
>> +	{PR_SET_THP_DISABLE, &num_0, &num_1, EINVAL},
>> +	{PR_GET_THP_DISABLE, &num_1, &num_1, EINVAL},
>> +#endif
>> +#ifdef HAVE_DECL_PR_CAP_AMBIENT
>> +	{PR_CAP_AMBIENT, &num_2, &num_1, EINVAL},
>> +#endif
>> +#ifdef HAVE_DECL_PR_GET_SPECULATION_CTRL
>> +	{PR_GET_SPECULATION_CTRL, &num_1, &num_0, EINVAL},
>> +#endif
> Why the ifdefs, you have even added a fallback definitions into the lapi
> header?
>
> The usuall way how to deal with these is to:
>
> 1) Add fallback definitions to lapi
> 2) Ensure these tests does not fail on older kernels
>
>     We do expect EINVAL in these cases anyways, which is what we would
>     get if the prctl() option is unknown to the kernel anyways, so here
>     we can just get rid of these ifdefs and things should work fine.

For me, a fallback definitions into the lapi header is only for fixing undefined error on old kernel.

IMO, we only test options that kernel supports.
If we test an unsupported option, our case reports EINVAL that will give user a false impression(kernel
supports it, but argument or environment is bad). I think we should check they whether supported before run
(ifdef is a way).

ps: If we test EPERM error(cap is not in PI or PP) of PR_CAP_AMBIENT on old kernel,  they will report EINVAL.
So, I think ifdef is needed.

>
>> +	{PR_SET_SECUREBITS, &num_0, &num_0, EPERM},
>> +	{PR_CAPBSET_DROP, &num_1, &num_0, EPERM},
>>   };
>>   
>>   static void verify_prctl(unsigned int n)
>>   {
>>   	struct tcase *tc = &tcases[n];
>>   
>> -	TEST(prctl(tc->option, tc->arg2));
>> +	TEST(prctl(tc->option, *tc->arg2, *tc->arg3));
>>   	if (TST_RET == 0) {
>>   		tst_res(TFAIL, "prctl() succeeded unexpectedly");
>>   		return;
>>   	}
>>   
>>   	if (tc->exp_errno == TST_ERR) {
>> -		tst_res(TPASS | TTERRNO, "prctl() failed as expected");
>> +		tst_res(TPASS | TTERRNO, "prctl() %d failed as expected", tc->option);
>>   	} else {
>> -		tst_res(TFAIL | TTERRNO, "prctl() failed unexpectedly, expected %s",
>> +		if (tc->option == PR_SET_SECCOMP && TST_ERR == EINVAL)
>> +			tst_res(TCONF, "current system was not built with CONFIG_SECCOMP.");
>> +		else
>> +			tst_res(TFAIL | TTERRNO, "prctl() failed unexpectedly, expected %s",
>>   				tst_strerrno(tc->exp_errno));
>>   	}
>>   }
>>   
>> +static void setup(void)
>> +{
>> +	bad_addr = (unsigned long)tst_get_bad_addr(NULL);
>> +}
>> +
>>   static struct tst_test test = {
>> +	.setup = setup,
>>   	.tcnt = ARRAY_SIZE(tcases),
>>   	.test = verify_prctl,
>> +	.caps = (struct tst_cap []) {
>> +		TST_CAP(TST_CAP_DROP, CAP_SYS_ADMIN),
>> +		TST_CAP(TST_CAP_DROP, CAP_SETPCAP),
>> +		{}
>> +	},
>>   };
>> -- 
>> 2.18.0
>>
>>
>>


-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.linux.it/pipermail/ltp/attachments/20191112/7d08e222/attachment.htm>