[v2] selftests: prctl: Add new prctl test for PR_SET_NAME

  This patch will add the new test, which covers the prctl call
PR_SET_NAME command. The test tries to give a name using the PR_SET_NAME
call and then confirm it that it changed correctly by using  PR_GET_NAME.
It also tries to rename it with empty name.In the test PR_GET_NAME is
tested by passing null pointer to it and check its behaviour.

Signed-off-by: Osama Muhammad <osmtendev@gmail.com>

---
changes since v1:
	-Used TASK_COMM_LEN instead of using numerical value 16.
	 
---
 tools/testing/selftests/prctl/Makefile        |  2 +-
 .../selftests/prctl/set-process-name.c        | 62 +++++++++++++++++++
 2 files changed, 63 insertions(+), 1 deletion(-)
 create mode 100644 tools/testing/selftests/prctl/set-process-name.c
  

On 6/7/23 09:36, Osama Muhammad wrote:
> This patch will add the new test, which covers the prctl call
> PR_SET_NAME command. The test tries to give a name using the PR_SET_NAME
> call and then confirm it that it changed correctly by using  PR_GET_NAME.
> It also tries to rename it with empty name.In the test PR_GET_NAME is
> tested by passing null pointer to it and check its behaviour.
> 
> Signed-off-by: Osama Muhammad <osmtendev@gmail.com>
> 
> ---
> changes since v1:
> 	-Used TASK_COMM_LEN instead of using numerical value 16.

Please add linux/sched.h here as an include to pull this.
It is good to add an explicit include as opposed taking
a chance on it being included from another include.

thanks,
-- Shuah
  

Hi all,

I looked into it and tried to use TASK_COMM_LEN in the test. Even
though I included "linux/sched.h '', I was not able to compile the
test because it couldn't find it in the header file.
I dived deep into the issue and turns out header file mapped in
/usr/include/linux/sched.h is actually mapped to
/include/uapi/linux/sched.h[1] in linux source,
where TASK_COMM_LEN is not even defined. Instead TASK_COMM_LEN is
defined in /include/linux/sched.h which is not mapped to any header
files in
userspace(/(/usr/include/linux).
I also tried to find the TASK_COM_LEN in /usr/include/linux/ but I
couldn't find it. Following are the search results.
grep -rnw '/usr/include/linux/' -e 'TASK_COMM_LEN"
RESULTS OF COMMAND :- /usr/include/linux/taskstats.h:38:#define
TS_COMM_LEN 32 /* should be >= TASK_COMM_LEN
Based on this information, I have two questions:
1. Would this require a fix to move 'TASK_COMM_LEN' macro from
/include/linux/sched.h to UAPI headers /include/uapi/linux/sched.h.
2. Is there any other way to access TASK_COMM_LEN in the selftest that
I'm not aware of?

[1]:-https://elixir.bootlin.com/linux/v5.15.116/source/include/uapi/linux/sched.h

Thanks,
Osama

On Sat, 10 Jun 2023 at 02:43, Shuah Khan <skhan@linuxfoundation.org> wrote:
>
> On 6/7/23 09:36, Osama Muhammad wrote:
> > This patch will add the new test, which covers the prctl call
> > PR_SET_NAME command. The test tries to give a name using the PR_SET_NAME
> > call and then confirm it that it changed correctly by using  PR_GET_NAME.
> > It also tries to rename it with empty name.In the test PR_GET_NAME is
> > tested by passing null pointer to it and check its behaviour.
> >
> > Signed-off-by: Osama Muhammad <osmtendev@gmail.com>
> >
> > ---
> > changes since v1:
> >       -Used TASK_COMM_LEN instead of using numerical value 16.
>
> Please add linux/sched.h here as an include to pull this.
> It is good to add an explicit include as opposed taking
> a chance on it being included from another include.
>
> thanks,
> -- Shuah
  

On 6/10/23 07:01, Osama Muhammad wrote:
> Hi all,
> 
> I looked into it and tried to use TASK_COMM_LEN in the test. Even
> though I included "linux/sched.h '', I was not able to compile the
> test because it couldn't find it in the header file.
> I dived deep into the issue and turns out header file mapped in
> /usr/include/linux/sched.h is actually mapped to
> /include/uapi/linux/sched.h[1] in linux source,
> where TASK_COMM_LEN is not even defined. Instead TASK_COMM_LEN is
> defined in /include/linux/sched.h which is not mapped to any header
> files in
> userspace(/(/usr/include/linux).
> I also tried to find the TASK_COM_LEN in /usr/include/linux/ but I
> couldn't find it. Following are the search results.
> grep -rnw '/usr/include/linux/' -e 'TASK_COMM_LEN"
> RESULTS OF COMMAND :- /usr/include/linux/taskstats.h:38:#define
> TS_COMM_LEN 32 /* should be >= TASK_COMM_LEN
> Based on this information, I have two questions:
> 1. Would this require a fix to move 'TASK_COMM_LEN' macro from
> /include/linux/sched.h to UAPI headers /include/uapi/linux/sched.h.
> 2. Is there any other way to access TASK_COMM_LEN in the selftest that
> I'm not aware of?
> 
> [1]:-https://elixir.bootlin.com/linux/v5.15.116/source/include/uapi/linux/sched.h
> 

The best source is Linux mainline.

Take a look at test files that include linux/sched.h

arm64/abi/tpidr2.c is one of them.

Did you install headers before compiling the test?
  make headers_install

thanks,
-- Shuah
  

Hi,

Yes, I did install the latest kernel headers and TASK_COMM_LEN is not
accessible in userspace.

I looked into the test which uses TASK_COMM_LEN but the test defines
it in its own header file.

Example:  https://elixir.bootlin.com/linux/latest/source/tools/testing/selftests/bpf/progs/pyperf.h#L13

TASK_COMM_LEN is defined in include/linux/sched.h, but this header
file is not exposed to userspace.

TASK_COMM_LEN is not defined in /include/uapi/linux/sched.h which is
exposed to userspace kernel headers.
Please find the link to the header file exposed to user space :-
-https://elixir.bootlin.com/linux/v5.15.116/source/include/uapi/linux/sched.h

As for arm64/abi/tpidr2.c It includes linux/sched.h which will be
/include/uapi/linux/sched.h because the user space program is
including it.
So it also cannot use TASK_COMM_LEN directly.

Regards,
Osama

On Tue, 13 Jun 2023 at 02:56, Shuah Khan <skhan@linuxfoundation.org> wrote:
>
> On 6/10/23 07:01, Osama Muhammad wrote:
> > Hi all,
> >
> > I looked into it and tried to use TASK_COMM_LEN in the test. Even
> > though I included "linux/sched.h '', I was not able to compile the
> > test because it couldn't find it in the header file.
> > I dived deep into the issue and turns out header file mapped in
> > /usr/include/linux/sched.h is actually mapped to
> > /include/uapi/linux/sched.h[1] in linux source,
> > where TASK_COMM_LEN is not even defined. Instead TASK_COMM_LEN is
> > defined in /include/linux/sched.h which is not mapped to any header
> > files in
> > userspace(/(/usr/include/linux).
> > I also tried to find the TASK_COM_LEN in /usr/include/linux/ but I
> > couldn't find it. Following are the search results.
> > grep -rnw '/usr/include/linux/' -e 'TASK_COMM_LEN"
> > RESULTS OF COMMAND :- /usr/include/linux/taskstats.h:38:#define
> > TS_COMM_LEN 32 /* should be >= TASK_COMM_LEN
> > Based on this information, I have two questions:
> > 1. Would this require a fix to move 'TASK_COMM_LEN' macro from
> > /include/linux/sched.h to UAPI headers /include/uapi/linux/sched.h.
> > 2. Is there any other way to access TASK_COMM_LEN in the selftest that
> > I'm not aware of?
> >
> > [1]:-https://elixir.bootlin.com/linux/v5.15.116/source/include/uapi/linux/sched.h
> >
>
> The best source is Linux mainline.
>
> Take a look at test files that include linux/sched.h
>
> arm64/abi/tpidr2.c is one of them.
>
> Did you install headers before compiling the test?
>   make headers_install
>
> thanks,
> -- Shuah
>
>
  

Hi Shuah,

Any feedback on this patch?.

Thanks,
Osama


On Sat, 17 Jun 2023 at 18:01, Osama Muhammad <osmtendev@gmail.com> wrote:
>
> Hi,
>
> Yes, I did install the latest kernel headers and TASK_COMM_LEN is not
> accessible in userspace.
>
> I looked into the test which uses TASK_COMM_LEN but the test defines
> it in its own header file.
>
> Example:  https://elixir.bootlin.com/linux/latest/source/tools/testing/selftests/bpf/progs/pyperf.h#L13
>
> TASK_COMM_LEN is defined in include/linux/sched.h, but this header
> file is not exposed to userspace.
>
> TASK_COMM_LEN is not defined in /include/uapi/linux/sched.h which is
> exposed to userspace kernel headers.
> Please find the link to the header file exposed to user space :-
> -https://elixir.bootlin.com/linux/v5.15.116/source/include/uapi/linux/sched.h
>
> As for arm64/abi/tpidr2.c It includes linux/sched.h which will be
> /include/uapi/linux/sched.h because the user space program is
> including it.
> So it also cannot use TASK_COMM_LEN directly.
>
> Regards,
> Osama
>
> On Tue, 13 Jun 2023 at 02:56, Shuah Khan <skhan@linuxfoundation.org> wrote:
> >
> > On 6/10/23 07:01, Osama Muhammad wrote:
> > > Hi all,
> > >
> > > I looked into it and tried to use TASK_COMM_LEN in the test. Even
> > > though I included "linux/sched.h '', I was not able to compile the
> > > test because it couldn't find it in the header file.
> > > I dived deep into the issue and turns out header file mapped in
> > > /usr/include/linux/sched.h is actually mapped to
> > > /include/uapi/linux/sched.h[1] in linux source,
> > > where TASK_COMM_LEN is not even defined. Instead TASK_COMM_LEN is
> > > defined in /include/linux/sched.h which is not mapped to any header
> > > files in
> > > userspace(/(/usr/include/linux).
> > > I also tried to find the TASK_COM_LEN in /usr/include/linux/ but I
> > > couldn't find it. Following are the search results.
> > > grep -rnw '/usr/include/linux/' -e 'TASK_COMM_LEN"
> > > RESULTS OF COMMAND :- /usr/include/linux/taskstats.h:38:#define
> > > TS_COMM_LEN 32 /* should be >= TASK_COMM_LEN
> > > Based on this information, I have two questions:
> > > 1. Would this require a fix to move 'TASK_COMM_LEN' macro from
> > > /include/linux/sched.h to UAPI headers /include/uapi/linux/sched.h.
> > > 2. Is there any other way to access TASK_COMM_LEN in the selftest that
> > > I'm not aware of?
> > >
> > > [1]:-https://elixir.bootlin.com/linux/v5.15.116/source/include/uapi/linux/sched.h
> > >
> >
> > The best source is Linux mainline.
> >
> > Take a look at test files that include linux/sched.h
> >
> > arm64/abi/tpidr2.c is one of them.
> >
> > Did you install headers before compiling the test?
> >   make headers_install
> >
> > thanks,
> > -- Shuah
> >
> >
  

On 6/10/23 2:43 AM, Shuah Khan wrote:
> On 6/7/23 09:36, Osama Muhammad wrote:
>> This patch will add the new test, which covers the prctl call
>> PR_SET_NAME command. The test tries to give a name using the PR_SET_NAME
>> call and then confirm it that it changed correctly by using  PR_GET_NAME.
>> It also tries to rename it with empty name.In the test PR_GET_NAME is
>> tested by passing null pointer to it and check its behaviour.
>>
>> Signed-off-by: Osama Muhammad <osmtendev@gmail.com>
>>
>> ---
>> changes since v1:
>>     -Used TASK_COMM_LEN instead of using numerical value 16.
> 
> Please add linux/sched.h here as an include to pull this.
> It is good to add an explicit include as opposed taking
> a chance on it being included from another include.
TASK_COMM_LEN is defined in include/linux/sched.h. It is only to be used by
the kernel.

If we look at include/uapi/linux/sched.h, TASK_COMM_LEN isn't defined. So
this test looks good to me.

> 
> thanks,
> -- Shuah
  

Hi,

Thank you for the patch. This patch cleanly applies on next-20230627.

Unrelated to this patch:
I'm not sure if this patch was written against linux next. Always try to
send patches against latest next tag from following repo:

https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git

On 6/26/23 11:36 PM, Osama Muhammad wrote:
> Hi Shuah,
> 
> Any feedback on this patch?.
> 
> Thanks,
> Osama
> 
> 
> On Sat, 17 Jun 2023 at 18:01, Osama Muhammad <osmtendev@gmail.com> wrote:
>>
>> Hi,
>>
>> Yes, I did install the latest kernel headers and TASK_COMM_LEN is not
>> accessible in userspace.
>>
>> I looked into the test which uses TASK_COMM_LEN but the test defines
>> it in its own header file.
>>
>> Example:  https://elixir.bootlin.com/linux/latest/source/tools/testing/selftests/bpf/progs/pyperf.h#L13
>>
>> TASK_COMM_LEN is defined in include/linux/sched.h, but this header
>> file is not exposed to userspace.
>>
>> TASK_COMM_LEN is not defined in /include/uapi/linux/sched.h which is
>> exposed to userspace kernel headers.
>> Please find the link to the header file exposed to user space :-
>> -https://elixir.bootlin.com/linux/v5.15.116/source/include/uapi/linux/sched.h
>>
>> As for arm64/abi/tpidr2.c It includes linux/sched.h which will be
>> /include/uapi/linux/sched.h because the user space program is
>> including it.
>> So it also cannot use TASK_COMM_LEN directly.
>>
>> Regards,
>> Osama
>>
>> On Tue, 13 Jun 2023 at 02:56, Shuah Khan <skhan@linuxfoundation.org> wrote:
>>>
>>> On 6/10/23 07:01, Osama Muhammad wrote:
>>>> Hi all,
>>>>
>>>> I looked into it and tried to use TASK_COMM_LEN in the test. Even
>>>> though I included "linux/sched.h '', I was not able to compile the
>>>> test because it couldn't find it in the header file.
>>>> I dived deep into the issue and turns out header file mapped in
>>>> /usr/include/linux/sched.h is actually mapped to
>>>> /include/uapi/linux/sched.h[1] in linux source,
>>>> where TASK_COMM_LEN is not even defined. Instead TASK_COMM_LEN is
>>>> defined in /include/linux/sched.h which is not mapped to any header
>>>> files in
>>>> userspace(/(/usr/include/linux).
>>>> I also tried to find the TASK_COM_LEN in /usr/include/linux/ but I
>>>> couldn't find it. Following are the search results.
>>>> grep -rnw '/usr/include/linux/' -e 'TASK_COMM_LEN"
>>>> RESULTS OF COMMAND :- /usr/include/linux/taskstats.h:38:#define
>>>> TS_COMM_LEN 32 /* should be >= TASK_COMM_LEN
>>>> Based on this information, I have two questions:
>>>> 1. Would this require a fix to move 'TASK_COMM_LEN' macro from
>>>> /include/linux/sched.h to UAPI headers /include/uapi/linux/sched.h.
>>>> 2. Is there any other way to access TASK_COMM_LEN in the selftest that
>>>> I'm not aware of?
>>>>
>>>> [1]:-https://elixir.bootlin.com/linux/v5.15.116/source/include/uapi/linux/sched.h
>>>>
>>>
>>> The best source is Linux mainline.
>>>
>>> Take a look at test files that include linux/sched.h
>>>
>>> arm64/abi/tpidr2.c is one of them.
>>>
>>> Did you install headers before compiling the test?
>>>   make headers_install
>>>
>>> thanks,
>>> -- Shuah
>>>
>>>
  

Unrelated to this patch:
You can build the user header files by running `make headers` in a kernel
repository to process the kernel header files. They are generated in
<kernel_source>/include/uapi Then run `make -C
tools/testing/selftests/prctl` to build the test etc.

On 6/7/23 8:36 PM, Osama Muhammad wrote:
> This patch will add the new test, which covers the prctl call
> PR_SET_NAME command. The test tries to give a name using the PR_SET_NAME
> call and then confirm it that it changed correctly by using  PR_GET_NAME.
> It also tries to rename it with empty name.In the test PR_GET_NAME is
> tested by passing null pointer to it and check its behaviour.
> 
> Signed-off-by: Osama Muhammad <osmtendev@gmail.com>
Reviewed-by: Muhammad Usama Anjum <usama.anjum@collabora.com>
Tested-by: Muhammad Usama Anjum <usama.anjum@collabora.com>

> 
> ---
> changes since v1:
> 	-Used TASK_COMM_LEN instead of using numerical value 16.
> 	 
> ---
>  tools/testing/selftests/prctl/Makefile        |  2 +-
>  .../selftests/prctl/set-process-name.c        | 62 +++++++++++++++++++
>  2 files changed, 63 insertions(+), 1 deletion(-)
>  create mode 100644 tools/testing/selftests/prctl/set-process-name.c
> 
> diff --git a/tools/testing/selftests/prctl/Makefile b/tools/testing/selftests/prctl/Makefile
> index c058b81ee..cfc35d29f 100644
> --- a/tools/testing/selftests/prctl/Makefile
> +++ b/tools/testing/selftests/prctl/Makefile
> @@ -5,7 +5,7 @@ ARCH ?= $(shell echo $(uname_M) | sed -e s/i.86/x86/ -e s/x86_64/x86/)
>  
>  ifeq ($(ARCH),x86)
>  TEST_PROGS := disable-tsc-ctxt-sw-stress-test disable-tsc-on-off-stress-test \
> -		disable-tsc-test set-anon-vma-name-test
> +		disable-tsc-test set-anon-vma-name-test set-process-name
>  all: $(TEST_PROGS)
>  
>  include ../lib.mk
> diff --git a/tools/testing/selftests/prctl/set-process-name.c b/tools/testing/selftests/prctl/set-process-name.c
> new file mode 100644
> index 000000000..3bc5e0e09
> --- /dev/null
> +++ b/tools/testing/selftests/prctl/set-process-name.c
> @@ -0,0 +1,62 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * This test covers the PR_SET_NAME functionality of prctl calls
> + */
> +
> +#include <errno.h>
> +#include <sys/prctl.h>
> +#include <string.h>
> +
> +#include "../kselftest_harness.h"
> +
> +#define CHANGE_NAME "changename"
> +#define EMPTY_NAME ""
> +#define TASK_COMM_LEN 16
> +
> +int set_name(char *name)
> +{
> +	int res;
> +
> +	res = prctl(PR_SET_NAME, name, NULL, NULL, NULL);
> +
> +	if (res < 0)
> +		return -errno;
> +	return res;
> +}
> +
> +int check_is_name_correct(char *check_name)
> +{
> +	char name[TASK_COMM_LEN];
> +	int res;
> +
> +	res = prctl(PR_GET_NAME, name, NULL, NULL, NULL);
> +
> +	if (res < 0)
> +		return -errno;
> +
> +	return !strcmp(name, check_name);
> +}
> +
> +int check_null_pointer(char *check_name)
> +{
> +	char *name = NULL;
> +	int res;
> +
> +	res = prctl(PR_GET_NAME, name, NULL, NULL, NULL);
> +
> +	return res;
> +}
> +
> +TEST(rename_process) {
> +
> +	EXPECT_GE(set_name(CHANGE_NAME), 0);
> +	EXPECT_TRUE(check_is_name_correct(CHANGE_NAME));
> +
> +	EXPECT_GE(set_name(EMPTY_NAME), 0);
> +	EXPECT_TRUE(check_is_name_correct(EMPTY_NAME));
> +
> +	EXPECT_GE(set_name(CHANGE_NAME), 0);
> +	EXPECT_LT(check_null_pointer(CHANGE_NAME), 0);
> +}
> +
> +TEST_HARNESS_MAIN
  

On 6/26/23 12:36, Osama Muhammad wrote:
> Hi Shuah,
> 
> Any feedback on this patch?.
> 
> Thanks,
> Osama
> 
> 

Please don't top post when you are responding on kernel
mailing lists. It gets very difficult to follow the
comments in the email thread.

> On Sat, 17 Jun 2023 at 18:01, Osama Muhammad <osmtendev@gmail.com> wrote:
>>
>> Hi,
>>
>> Yes, I did install the latest kernel headers and TASK_COMM_LEN is not
>> accessible in userspace.
>>
>> I looked into the test which uses TASK_COMM_LEN but the test defines
>> it in its own header file.
>>
>> Example:  https://elixir.bootlin.com/linux/latest/source/tools/testing/selftests/bpf/progs/pyperf.h#L13

bfp test does things differently because its dependencies
on run-time environment.

>>
>> TASK_COMM_LEN is defined in include/linux/sched.h, but this header
>> file is not exposed to userspace.

Correct. you can include linux/sched.h like other tests do
Take a look at tools/testing/selftests/clone3

thanks,
-- Shuah
  

On 7/14/23 10:21, Shuah Khan wrote:
> On 6/26/23 12:36, Osama Muhammad wrote:
>> Hi Shuah,
>>
>> Any feedback on this patch?.
>>
>> Thanks,
>> Osama
>>
>>
> 
> Please don't top post when you are responding on kernel
> mailing lists. It gets very difficult to follow the
> comments in the email thread.
> 
>> On Sat, 17 Jun 2023 at 18:01, Osama Muhammad <osmtendev@gmail.com> wrote:
>>>
>>> Hi,
>>>
>>> Yes, I did install the latest kernel headers and TASK_COMM_LEN is not
>>> accessible in userspace.
>>>
>>> I looked into the test which uses TASK_COMM_LEN but the test defines
>>> it in its own header file.
>>>
>>> Example:  https://elixir.bootlin.com/linux/latest/source/tools/testing/selftests/bpf/progs/pyperf.h#L13
> 
> bfp test does things differently because its dependencies
> on run-time environment.
> 
>>>
>>> TASK_COMM_LEN is defined in include/linux/sched.h, but this header
>>> file is not exposed to userspace.
> 
> Correct. you can include linux/sched.h like other tests do
> Take a look at tools/testing/selftests/clone3
> 

You are right about TASK_COMM_LEN not visible to user-space.
This patch has been applied to linux-kselftest next for 6.6-rc1

thanks,
-- Shuah