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

[He Zhe]

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.

>
>>> 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?


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;
>>>    	}
>>>