[LTP] [PATCH v3] syscalls/prctl04: Fix false positive report when SECCOMP_MODE_FILTER is not supported

[xuyang2018.jy@fujitsu.com]

Hi He
> 
> 
> On 11/23/22 14:17, xuyang2018.jy@fujitsu.com wrote:
>> Hi he
>>> Hi He
>>>
>>>> The child process really should not receive the expected siganl, SIGSYS, when
>>>> kernel doesn't support SECCOMP_MODE_FILTER.
>>> I still feel confused, so which subtestcase has problem since we have do
>>> check whether support SECCOMP_MODE_FILTER in check_filter_mode.
>>
>> It seems kernel without CONFIG_SECCOMP doesn't report errror when set
>> filter, so the previous check doesn't work.
> 
> kernel does report EINVAL as we can see 4 lines of "doesn't support",
> corresponding to 4 filter cases, in the commit log. But later verify_prctl
> doesn't realize it's not supported and gives a FAIL for not receiving the
> related signal. So we add mode_filter_not_supported to inform it.

yes.
> 
>>
>>>> This patch tests if SECCOMP_MODE_FILTER is supported in setup and adds a
>>>> variable to record it.
>>>>
>>>> Before this patch:
>>>> root@xilinx-zynq:~# /opt/ltp/testcases/bin/prctl04
>>>> tst_test.c:1431: TINFO: Timeout per run is 0h 05m 00s
>>>> ---- snip ----
>>>> prctl04.c:154: TCONF: kernel doesn't support SECCOMP_MODE_FILTER
>>>> prctl04.c:154: TCONF: kernel doesn't support SECCOMP_MODE_FILTER
>>>> prctl04.c:154: TCONF: kernel doesn't support SECCOMP_MODE_FILTER
>>>> prctl04.c:204: TFAIL: SECCOMP_MODE_FILTER permits exit() unexpectedly
>>>> prctl04.c:154: TCONF: kernel doesn't support SECCOMP_MODE_FILTER
>>>>
>>>> After this patch:
>>>> root@xilinx-zynq:~# /opt/ltp/testcases/bin/prctl04
>>>> tst_test.c:1431: TINFO: Timeout per run is 0h 05m 00s
>>>> ---- snip ----
>>>> prctl04.c:154: TCONF: kernel doesn't support SECCOMP_MODE_FILTER
>>>> prctl04.c:154: TCONF: kernel doesn't support SECCOMP_MODE_FILTER
>>>> prctl04.c:154: TCONF: kernel doesn't support SECCOMP_MODE_FILTER
>>>> prctl04.c:154: TCONF: kernel doesn't support SECCOMP_MODE_FILTER
>>>
>>> The line 154 and 204 is refer to origin case[1], so do you use the
>>> lastest ltp?
>>>
>>> [1]
>>> https://github.com/linux-test-project/ltp/commit/3ddc217d7b466f16782c257e29e18b251969edec#diff-6ae2f0e9ae152457424103cc20b7885e242f33f58b2e543b7941671f418d9485R154
>>>
>>> Best Regards
>>> Yang Xu
>>>> Signed-off-by: He Zhe <zhe.he@windriver.com>
>>>> ---
>>>> v2: Add a variable to record the support status instead of exit(1)
>>>> v3: Move mode_filter_not_supported check a bit upper to save a prctl call
>>>>
>>>>     testcases/kernel/syscalls/prctl/prctl04.c | 30 +++++++++++++++++------
>>>>     1 file changed, 22 insertions(+), 8 deletions(-)
>>>>
>>>> diff --git a/testcases/kernel/syscalls/prctl/prctl04.c b/testcases/kernel/syscalls/prctl/prctl04.c
>>>> index b9f4c2a10..d3de4b0d6 100644
>>>> --- a/testcases/kernel/syscalls/prctl/prctl04.c
>>>> +++ b/testcases/kernel/syscalls/prctl/prctl04.c
>>>> @@ -93,6 +93,9 @@ static struct tcase {
>>>>     	"SECCOMP_MODE_FILTER doesn't permit exit()"}
>>>>     };
>>>>     
>>>> +
>>>> +static int mode_filter_not_supported;
>>>> +
>>>>     static void check_filter_mode_inherit(void)
>>>>     {
>>>>     	int childpid;
>>>> @@ -154,16 +157,17 @@ static void check_filter_mode(int val)
>>>>     {
>>>>     	int fd;
>>>>     
>>>> +	if (mode_filter_not_supported == 1) {
>>>> +		tst_res(TCONF, "kernel doesn't support SECCOMP_MODE_FILTER");
>>>> +		return;
>>>> +	}
>>>> +
>>>>     	fd = SAFE_OPEN(FNAME, O_RDWR | O_CREAT, 0666);
>>>>     
>>>>     	TEST(prctl(PR_SET_SECCOMP, SECCOMP_MODE_FILTER, &strict));
>>>>     	if (TST_RET == -1) {
>>>> -		if (TST_ERR == EINVAL)
>>>> -			tst_res(TCONF,
>>>> -				"kernel doesn't support SECCOMP_MODE_FILTER");
>>>> -		else
>>>> -			tst_res(TFAIL | TERRNO,
>>>> -				"prctl(PR_SET_SECCOMP) sets SECCOMP_MODE_FILTER failed");
>>>> +		tst_res(TFAIL | TERRNO,
>>>> +			"prctl(PR_SET_SECCOMP) sets SECCOMP_MODE_FILTER failed");
>>>>     		return;
>>>>     	}
>>>>     
>>>> @@ -208,7 +212,7 @@ static void verify_prctl(unsigned int n)
>>>>     			return;
>>>>     		}
>>>>     
>>>> -		if (tc->pass_flag == 2)
>>>> +		if (mode_filter_not_supported == 0 && tc->pass_flag == 2)
>> I prefer to use "tc->pass_flag == 2 && mode_filter_not_supported ==
>> 0"because only one case's pass_flag value is 2, so we don't need to run
>> the latter to many times when kernel without CONFIG_SECCOMP_FILTER.
> 
> I'm OK with this.
> 
>>
>>
>> with commit message fix and this fix,
> 
> What does "commit message fix" mean please?

https://github.com/linux-test-project/ltp/blob/master/testcases/kernel/syscalls/prctl/prctl04.c#L154

Your commit message log use wrong line number,  prctl04.c line154 
doesn't print not supported info. Please use lastest ltp code to run.

Best Regards
Yang Xu
> 
> 
> Regards,
> Zhe
> 
>>
>> Reviewed-by: Yang Xu <xuyang2018.jy@fujitsu.com>
>>
>>
>> ps:BTW, I think split this case into two cases by checking strict mode
>> and filter_mode is more clear ie prctl04_1.c prctl04_2.c, so we can add
>> these kernel checks by using tst_test struct's need_kconfig member.
>>
>> Best Regards
>> Yang Xu
>>>>     			tst_res(TFAIL,
>>>>     				"SECCOMP_MODE_FILTER permits exit() unexpectedly");
>>>>     	}
>>>> @@ -218,7 +222,17 @@ static void setup(void)
>>>>     {
>>>>     	TEST(prctl(PR_GET_SECCOMP));
>>>>     	if (TST_RET == 0) {
>>>> -		tst_res(TINFO, "kernel support PR_GET/SET_SECCOMP");
>>>> +		tst_res(TINFO, "kernel supports PR_GET/SET_SECCOMP");
>>>> +
>>>> +		TEST(prctl(PR_SET_SECCOMP, SECCOMP_MODE_FILTER, NULL));
>>>> +		if (TST_RET == -1)
>>>> +			if (TST_ERR == EINVAL) {
>>>> +				mode_filter_not_supported = 1;
>>>> +				return;
>>>> +			}
>>>> +
>>>> +		tst_res(TINFO, "kernel supports SECCOMP_MODE_FILTER");
>>>> +
>>>>     		return;
>>>>     	}
>>>>     
>