[LTP] [PATCH v2] syscalls/prctl10: Add basic test for PR_SET/GET_TSC

[Richard Palethorpe]

Hello,

"xuyang2018.jy@fujitsu.com" <xuyang2018.jy@fujitsu.com> writes:

> Hi Richard
>
>
>> Hello,
>> 
>> Yang Xu <xuyang2018.jy@fujitsu.com> writes:
>> 
>>> Signed-off-by: Yang Xu <xuyang2018.jy@fujitsu.com>
>>> ---
>>>   include/lapi/prctl.h                       |   7 ++
>>>   runtest/syscalls                           |   1 +
>>>   testcases/kernel/syscalls/prctl/.gitignore |   1 +
>>>   testcases/kernel/syscalls/prctl/prctl10.c  | 108 +++++++++++++++++++++
>>>   4 files changed, 117 insertions(+)
>>>   create mode 100644 testcases/kernel/syscalls/prctl/prctl10.c
>>>
>>> diff --git a/include/lapi/prctl.h b/include/lapi/prctl.h
>>> index fa5922231..8d3ef5c32 100644
>>> --- a/include/lapi/prctl.h
>>> +++ b/include/lapi/prctl.h
>>> @@ -19,6 +19,13 @@
>>>   # define PR_SET_SECCOMP  22
>>>   #endif
>>>   
>>> +#ifndef PR_SET_TSC
>>> +# define PR_GET_TSC 25
>>> +# define PR_SET_TSC 26
>>> +# define PR_TSC_ENABLE  1
>>> +# define PR_TSC_SIGSEGV 2
>>> +#endif
>>> +
>>>   #ifndef PR_SET_TIMERSLACK
>>>   # define PR_SET_TIMERSLACK 29
>>>   # define PR_GET_TIMERSLACK 30
>>> diff --git a/runtest/syscalls b/runtest/syscalls
>>> index 61a7b7677..51de0a614 100644
>>> --- a/runtest/syscalls
>>> +++ b/runtest/syscalls
>>> @@ -1004,6 +1004,7 @@ prctl06 prctl06
>>>   prctl07 prctl07
>>>   prctl08 prctl08
>>>   prctl09 prctl09
>>> +prctl10 prctl10
>>>   
>>>   pread01 pread01
>>>   pread01_64 pread01_64
>>> diff --git a/testcases/kernel/syscalls/prctl/.gitignore b/testcases/kernel/syscalls/prctl/.gitignore
>>> index 0f2c9b194..50ee4bf60 100644
>>> --- a/testcases/kernel/syscalls/prctl/.gitignore
>>> +++ b/testcases/kernel/syscalls/prctl/.gitignore
>>> @@ -8,3 +8,4 @@
>>>   /prctl07
>>>   /prctl08
>>>   /prctl09
>>> +/prctl10
>>> diff --git a/testcases/kernel/syscalls/prctl/prctl10.c b/testcases/kernel/syscalls/prctl/prctl10.c
>>> new file mode 100644
>>> index 000000000..01307ecd7
>>> --- /dev/null
>>> +++ b/testcases/kernel/syscalls/prctl/prctl10.c
>>> @@ -0,0 +1,108 @@
>>> +// SPDX-License-Identifier: GPL-2.0-or-later
>>> +/*
>>> + * Copyright (c) 2022 FUJITSU LIMITED. All rights reserved.
>>> + * Author: Yang Xu <xuyang2018.jy@fujitsu.com>
>>> + */
>>> +
>>> +/*\
>>> + * [Description]
>>> + *
>>> + * Basic test to test behaviour of PR_GET_TSC and PR_SET_TSC.
>>> + *
>>> + * Set the state of the flag determining whether the timestamp counter can
>>> + * be read by the process.
>>> + *
>>> + * - Pass PR_TSC_ENABLE to arg2 to allow it to be read.
>>> + * - Pass PR_TSC_SIGSEGV to arg2 to generate a SIGSEGV when read.
>>> + */
>>> +
>>> +#include <sys/prctl.h>
>>> +#include <string.h>
>>> +#include <stdio.h>
>>> +#include <stdlib.h>
>>> +#include "tst_test.h"
>>> +#include "lapi/prctl.h"
>>> +
>>> +#define TCASE_ENTRY(tsc_read_stat) { .name = #tsc_read_stat, .read_stat = tsc_read_stat}
>>> +
>>> +static const char * const tsc_read_stat_names[] = {
>>> +	[0] = "[not set]",
>>> +	[PR_TSC_ENABLE] = "PR_TSC_ENABLE",
>>> +	[PR_TSC_SIGSEGV] = "PR_TSC_SIGSEGV",
>>> +};
>>> +
>>> +static struct tcase {
>>> +	char *name;
>>> +	int read_stat;
>>> +} tcases[] = {
>>> +	TCASE_ENTRY(PR_TSC_ENABLE),
>>> +	TCASE_ENTRY(PR_TSC_SIGSEGV)
>>> +};
>>> +
>>> +static uint64_t rdtsc(void)
>>> +{
>>> +	uint32_t lo, hi;
>>> +	/* We cannot use "=A", since this would use %rax on x86_64 */
>>> +	__asm__ __volatile__ ("rdtsc" : "=a" (lo), "=d" (hi));
>>> +	return (uint64_t)hi << 32 | lo;
>>> +}
>>> +
>>> +
>>> +static int expected_status(int status, int exp_status)
>>> +{
>>> +	if (!exp_status && WIFEXITED(status))
>>> +		return 0;
>>> +
>>> +	if (exp_status && WIFSIGNALED(status) && WTERMSIG(status) == exp_status)
>>> +		return 0;
>>> +
>>> +	return 1;
>>> +}
>>> +
>>> +static void verify_prctl(unsigned int n)
>>> +{
>>> +	struct tcase *tc = &tcases[n];
>>> +	unsigned long long time1, time2;
>>> +	int tsc_val = 0, pid, status;
>>> +
>>> +	pid = SAFE_FORK();
>>> +	if (!pid) {
>>> +		TST_EXP_PASS_SILENT(prctl(PR_SET_TSC, tc->read_stat));
>>> +		TST_EXP_PASS_SILENT(prctl(PR_GET_TSC, &tsc_val));
>>> +		if (tsc_val == tc->read_stat)
>>> +			tst_res(TPASS, "current state is %s(%d)",
>>> +					tc->name, tc->read_stat);
>>> +		else
>>> +			tst_res(TFAIL, "current state is %s(%d), expect %s(%d)",
>>> +					tsc_read_stat_names[tsc_val],
>>> +					tsc_val, tc->name, tc->read_stat);
>>> +
>>> +		time1 = rdtsc();
>>> +		time2 = rdtsc();
>>> +		if (time2 > time1)
>>> +			tst_res(TPASS, "rdtsc works correctly, %lld ->%lld",
>>> +				time1, time2);
>>> +		else
>>> +			tst_res(TFAIL, "rdtsc works incorrectly, %lld ->%lld",
>>> +				time1, time2);
>>> +		exit(0);
>>> +	}
>>> +	SAFE_WAITPID(pid, &status, 0);
>>> +
>>> +	if (expected_status(status, tc->read_stat == PR_TSC_SIGSEGV ? SIGSEGV : 0))
>>> +		tst_res(TFAIL, "Test %s failed", tc->name);
>>> +	else
>>> +		tst_res(TPASS, "Test %s succeeded", tc->name);
>>> +}
>>> +
>>> +static struct tst_test test = {
>>> +	.needs_root = 1,
>> 
>> Why did you add this?
>
> Oh, yes, I should drop this.
>
> So, should I send a v3 or you or other maintainers merge it by
> deleting it?

I'll do the fixup and merge, thanks!

>
> Best Regards
> Yang Xu
>> 
>> It doesn't require root, but it could be denied with seccomp or an
>> LSM. In some cases having root won't help (e.g. in a container). If
>> you want to handle scenarios like these, then it would be better to
>> check the return status of prctl.
>> 
>>> +	.forks_child = 1,
>>> +	.test = verify_prctl,
>>> +	.tcnt = ARRAY_SIZE(tcases),
>>> +	.supported_archs = (const char *const []) {
>>> +		"x86",
>>> +		"x86_64",
>>> +		NULL
>>> +	},
>>> +};
>> 
>> Otherwise LGTM.
>> 


-- 
Thank you,
Richard.