| Message ID | 20240807113601.3882356-1-maxj.fnst@fujitsu.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v4] getcpu: Add testcase for EFAULT | expand |
Hi Ma, > Add a testcase with the arguments point to an invalid address. Generally LGTM, I have few comments, but I'll fix them before merge. > Reviewed-by: Andrea Cervesato <andrea.cervesato@suse.de> > Reviewed-by: Petr Vorel <pvorel@suse.cz> > Reviewed-by: Cyril Hrubis <chrubis@suse.cz> > Signed-off-by: Ma Xinjian <maxj.fnst@fujitsu.com> @Andrea, Cyril, feel free to have final look during today (otherwise I'll merge later today). @Ma The test has changed 3 times quite significantly. IMHO it's better not to add Reviewed-by: unless you change just either simple thing or did changes you were explicitly asked to do. Better is to --cc all people who did the review when generating patches with 'git format-patch'. (It's good to add Reviewed-by: if your patchset has more commits and some of them were previously reviewed and they are unchanged in the following version.) > --- Also, not needed, but it helps, if you wrote some changes to the previous version after these '---' (it will not be part of the git commit message when other developers apply your patch with 'git am'. > runtest/syscalls | 1 + > testcases/kernel/syscalls/getcpu/getcpu02.c | 71 +++++++++++++++++++++ > 2 files changed, 72 insertions(+) > create mode 100644 testcases/kernel/syscalls/getcpu/getcpu02.c > diff --git a/runtest/syscalls b/runtest/syscalls > index b8728c1c5..1537b5022 100644 > --- a/runtest/syscalls > +++ b/runtest/syscalls > @@ -448,6 +448,7 @@ futimesat01 futimesat01 > getcontext01 getcontext01 > getcpu01 getcpu01 > +getcpu02 getcpu02 You miss adding "/getcpu02" into testcases/kernel/syscalls/getcpu/.gitignore. I'll do that before merge. > getcwd01 getcwd01 > getcwd02 getcwd02 > diff --git a/testcases/kernel/syscalls/getcpu/getcpu02.c b/testcases/kernel/syscalls/getcpu/getcpu02.c > new file mode 100644 > index 000000000..859cb0d3e > --- /dev/null > +++ b/testcases/kernel/syscalls/getcpu/getcpu02.c > @@ -0,0 +1,71 @@ > +// SPDX-License-Identifier: GPL-2.0-or-later > +/* > + * Copyright (c) 2024 FUJITSU LIMITED. All Rights Reserved. > + * Copyright (c) Linux Test Project, 2024 > + * Author: Ma Xinjian <maxj.fnst@fujitsu.com> > + * > + */ > + > +/*\ > + * [Description] > + * > + * Verify that getcpu(2) fails with EFAULT: > + * > + * 1) cpu_id points outside the calling process's address space. > + * 2) node_id points outside the calling process's address space. > + */ > + > +#define _GNU_SOURCE > + > +#include "tst_test.h" > +#include "lapi/sched.h" > + > +static unsigned int cpu_id, node_id; > + > +static struct tcase { > + unsigned int *cpu_id; > + unsigned int *node_id; > +} tcases[] = { > + {NULL, &node_id}, > + {&cpu_id, NULL} I meant to add char *desc here instead of tst_res(TINFO later in check_getcpu() > +}; > + > +static void check_getcpu(unsigned int n) > +{ > + struct tcase *tc = &tcases[n]; > + int status; > + pid_t pid; > + > + if (n == 0) { It might be better to check this as: if (!tc->cpu_id) (set cpu_id only if not set. > + tst_res(TINFO, "Make cpu_id point outside the calling process's address space."); > + tc->cpu_id = tst_get_bad_addr(NULL); > + } else if (n == 1) { and here either if (!tc->node_id) or simple else. > + tst_res(TINFO, "Make node_id point outside the calling process's address space."); > + tc->node_id = tst_get_bad_addr(NULL); Also, we usually try to set values which does not change in the setup function (because one can run test more times, e.g. 1000x with :/getcpu02 -i1000. Thus things which don't have to be repeated go to the setup function. But when I tried that, even without setup function, using this pointer always causes SIGSEGV (test passes). And I use direct static values it does *not* cause SIGSEGV (test fails): static unsigned int cpu_id, node_id; static unsigned int *p_cpu_id = &cpu_id, *p_node_id = &node_id; static struct tcase { unsigned int **cpu_id; unsigned int **node_id; char *desc; } tcases[] = { {NULL, NULL, "none"}, {NULL, &p_node_id, "cpu_id"}, {&p_cpu_id, NULL, "node_id"}, }; static void check_getcpu(unsigned int n) { struct tcase *tc = &tcases[n]; int status; pid_t pid; tst_res(TINFO, "Test %s outside process's address space", tc->desc); pid = SAFE_FORK(); if (!pid) { TST_EXP_FAIL(getcpu(p_cpu_id, p_node_id), EFAULT); // this would always pass TST_EXP_FAIL(getcpu(*tc->cpu_id, *tc->node_id), EFAULT); // this always fail, even for none exit(0); } I guess I miss something obvious. Therefore I suggest to merge (or see the diff below). Kind regards, Petr static struct tcase { unsigned int *cpu_id; unsigned int *node_id; char *desc; } tcases[] = { {NULL, &node_id, "cpu_id"}, {&cpu_id, NULL, "node_id"}, }; static void check_getcpu(unsigned int n) { struct tcase *tc = &tcases[n]; int status; pid_t pid; tst_res(TINFO, "Test %s outside process's address space", tc->desc); if (!tc->cpu_id) tc->cpu_id = tst_get_bad_addr(NULL); if (!tc->node_id) tc->node_id = tst_get_bad_addr(NULL); pid = SAFE_FORK(); if (!pid) { TST_EXP_FAIL(getcpu(tc->cpu_id, tc->node_id), EFAULT); exit(0); } SAFE_WAITPID(pid, &status, 0); if (WIFSIGNALED(status) && WTERMSIG(status) == SIGSEGV) { tst_res(TPASS, "getcpu() caused SIGSEGV"); return; } if (WIFEXITED(status) && WEXITSTATUS(status) == 0) return; tst_res(TFAIL, "child %s", tst_strstatus(status)); } diff --git testcases/kernel/syscalls/getcpu/.gitignore testcases/kernel/syscalls/getcpu/.gitignore index 31fec5d35e..cd3022bbb1 100644 --- testcases/kernel/syscalls/getcpu/.gitignore +++ testcases/kernel/syscalls/getcpu/.gitignore @@ -1 +1,2 @@ /getcpu01 +/getcpu02 diff --git testcases/kernel/syscalls/getcpu/getcpu02.c testcases/kernel/syscalls/getcpu/getcpu02.c index 859cb0d3eb..83d236a78c 100644 --- testcases/kernel/syscalls/getcpu/getcpu02.c +++ testcases/kernel/syscalls/getcpu/getcpu02.c @@ -9,7 +9,7 @@ /*\ * [Description] * - * Verify that getcpu(2) fails with EFAULT: + * Verify that getcpu(2) fails with EFAULT if: * * 1) cpu_id points outside the calling process's address space. * 2) node_id points outside the calling process's address space. @@ -25,9 +25,10 @@ static unsigned int cpu_id, node_id; static struct tcase { unsigned int *cpu_id; unsigned int *node_id; + char *desc; } tcases[] = { - {NULL, &node_id}, - {&cpu_id, NULL} + {NULL, &node_id, "cpu_id"}, + {&cpu_id, NULL, "node_id"}, }; static void check_getcpu(unsigned int n) @@ -36,13 +37,13 @@ static void check_getcpu(unsigned int n) int status; pid_t pid; - if (n == 0) { - tst_res(TINFO, "Make cpu_id point outside the calling process's address space."); + tst_res(TINFO, "Test %s outside process's address space", tc->desc); + + if (!tc->cpu_id) tc->cpu_id = tst_get_bad_addr(NULL); - } else if (n == 1) { - tst_res(TINFO, "Make node_id point outside the calling process's address space."); + + if (!tc->node_id) tc->node_id = tst_get_bad_addr(NULL); - } pid = SAFE_FORK(); if (!pid) {
Hi Ma, On 8/8/24 11:16, Petr Vorel wrote: > Hi Ma, > >> Add a testcase with the arguments point to an invalid address. > Generally LGTM, I have few comments, but I'll fix them before merge. > >> Reviewed-by: Andrea Cervesato <andrea.cervesato@suse.de> >> Reviewed-by: Petr Vorel <pvorel@suse.cz> >> Reviewed-by: Cyril Hrubis <chrubis@suse.cz> >> Signed-off-by: Ma Xinjian <maxj.fnst@fujitsu.com> > @Andrea, Cyril, feel free to have final look during today (otherwise I'll merge > later today). > > @Ma The test has changed 3 times quite significantly. IMHO it's better not to > add Reviewed-by: unless you change just either simple thing or did changes you > were explicitly asked to do. Better is to --cc all people who did the review > when generating patches with 'git format-patch'. Yes, please add Reviewed-by tag only when the reviewer replied with the tag. That actually means "patch is ok for me, so it can be submitted" instead of "i took a look at the patch and it needs to be modified". > > (It's good to add Reviewed-by: if your patchset has more commits and some of > them were previously reviewed and they are unchanged in the following version.) > >> --- > Also, not needed, but it helps, if you wrote some changes to the previous > version after these '---' (it will not be part of the git commit message when > other developers apply your patch with 'git am'. > >> runtest/syscalls | 1 + >> testcases/kernel/syscalls/getcpu/getcpu02.c | 71 +++++++++++++++++++++ >> 2 files changed, 72 insertions(+) >> create mode 100644 testcases/kernel/syscalls/getcpu/getcpu02.c >> diff --git a/runtest/syscalls b/runtest/syscalls >> index b8728c1c5..1537b5022 100644 >> --- a/runtest/syscalls >> +++ b/runtest/syscalls >> @@ -448,6 +448,7 @@ futimesat01 futimesat01 >> getcontext01 getcontext01 >> getcpu01 getcpu01 >> +getcpu02 getcpu02 > You miss adding "/getcpu02" into testcases/kernel/syscalls/getcpu/.gitignore. > I'll do that before merge. > >> getcwd01 getcwd01 >> getcwd02 getcwd02 > >> diff --git a/testcases/kernel/syscalls/getcpu/getcpu02.c b/testcases/kernel/syscalls/getcpu/getcpu02.c >> new file mode 100644 >> index 000000000..859cb0d3e >> --- /dev/null >> +++ b/testcases/kernel/syscalls/getcpu/getcpu02.c >> @@ -0,0 +1,71 @@ >> +// SPDX-License-Identifier: GPL-2.0-or-later >> +/* >> + * Copyright (c) 2024 FUJITSU LIMITED. All Rights Reserved. >> + * Copyright (c) Linux Test Project, 2024 >> + * Author: Ma Xinjian <maxj.fnst@fujitsu.com> >> + * >> + */ >> + >> +/*\ >> + * [Description] >> + * >> + * Verify that getcpu(2) fails with EFAULT: >> + * >> + * 1) cpu_id points outside the calling process's address space. >> + * 2) node_id points outside the calling process's address space. >> + */ >> + >> +#define _GNU_SOURCE >> + >> +#include "tst_test.h" >> +#include "lapi/sched.h" >> + >> +static unsigned int cpu_id, node_id; >> + >> +static struct tcase { >> + unsigned int *cpu_id; >> + unsigned int *node_id; >> +} tcases[] = { >> + {NULL, &node_id}, >> + {&cpu_id, NULL} > I meant to add char *desc here instead of tst_res(TINFO later in check_getcpu() >> +}; >> + >> +static void check_getcpu(unsigned int n) >> +{ >> + struct tcase *tc = &tcases[n]; >> + int status; >> + pid_t pid; >> + >> + if (n == 0) { > It might be better to check this as: > if (!tc->cpu_id) > (set cpu_id only if not set. > >> + tst_res(TINFO, "Make cpu_id point outside the calling process's address space."); >> + tc->cpu_id = tst_get_bad_addr(NULL); >> + } else if (n == 1) { > and here either if (!tc->node_id) or simple else. >> + tst_res(TINFO, "Make node_id point outside the calling process's address space."); >> + tc->node_id = tst_get_bad_addr(NULL); > Also, we usually try to set values which does not change in the setup function > (because one can run test more times, e.g. 1000x with :/getcpu02 -i1000. Thus > things which don't have to be repeated go to the setup function. > > But when I tried that, even without setup function, using this pointer always > causes SIGSEGV (test passes). And I use direct static values it does *not* cause > SIGSEGV (test fails): > > static unsigned int cpu_id, node_id; > static unsigned int *p_cpu_id = &cpu_id, *p_node_id = &node_id; > > static struct tcase { > unsigned int **cpu_id; > unsigned int **node_id; > char *desc; > } tcases[] = { > {NULL, NULL, "none"}, > {NULL, &p_node_id, "cpu_id"}, > {&p_cpu_id, NULL, "node_id"}, > }; > > static void check_getcpu(unsigned int n) > { > struct tcase *tc = &tcases[n]; > int status; > pid_t pid; > > tst_res(TINFO, "Test %s outside process's address space", tc->desc); > > pid = SAFE_FORK(); > if (!pid) { > TST_EXP_FAIL(getcpu(p_cpu_id, p_node_id), EFAULT); // this would always pass > TST_EXP_FAIL(getcpu(*tc->cpu_id, *tc->node_id), EFAULT); // this always fail, even for none > > exit(0); > } > > I guess I miss something obvious. Therefore I suggest to merge (or see the diff > below). > > Kind regards, > Petr > > static struct tcase { > unsigned int *cpu_id; > unsigned int *node_id; > char *desc; > } tcases[] = { > {NULL, &node_id, "cpu_id"}, > {&cpu_id, NULL, "node_id"}, > }; > > static void check_getcpu(unsigned int n) > { > struct tcase *tc = &tcases[n]; > int status; > pid_t pid; > > tst_res(TINFO, "Test %s outside process's address space", tc->desc); > > if (!tc->cpu_id) > tc->cpu_id = tst_get_bad_addr(NULL); > > if (!tc->node_id) > tc->node_id = tst_get_bad_addr(NULL); > > pid = SAFE_FORK(); > if (!pid) { > TST_EXP_FAIL(getcpu(tc->cpu_id, tc->node_id), EFAULT); > > exit(0); > } > > SAFE_WAITPID(pid, &status, 0); > > if (WIFSIGNALED(status) && WTERMSIG(status) == SIGSEGV) { > tst_res(TPASS, "getcpu() caused SIGSEGV"); > return; > } > > if (WIFEXITED(status) && WEXITSTATUS(status) == 0) > return; > > tst_res(TFAIL, "child %s", tst_strstatus(status)); > } > > diff --git testcases/kernel/syscalls/getcpu/.gitignore testcases/kernel/syscalls/getcpu/.gitignore > index 31fec5d35e..cd3022bbb1 100644 > --- testcases/kernel/syscalls/getcpu/.gitignore > +++ testcases/kernel/syscalls/getcpu/.gitignore > @@ -1 +1,2 @@ > /getcpu01 > +/getcpu02 > diff --git testcases/kernel/syscalls/getcpu/getcpu02.c testcases/kernel/syscalls/getcpu/getcpu02.c > index 859cb0d3eb..83d236a78c 100644 > --- testcases/kernel/syscalls/getcpu/getcpu02.c > +++ testcases/kernel/syscalls/getcpu/getcpu02.c > @@ -9,7 +9,7 @@ > /*\ > * [Description] > * > - * Verify that getcpu(2) fails with EFAULT: > + * Verify that getcpu(2) fails with EFAULT if: > * > * 1) cpu_id points outside the calling process's address space. > * 2) node_id points outside the calling process's address space. > @@ -25,9 +25,10 @@ static unsigned int cpu_id, node_id; > static struct tcase { > unsigned int *cpu_id; > unsigned int *node_id; > + char *desc; > } tcases[] = { > - {NULL, &node_id}, > - {&cpu_id, NULL} > + {NULL, &node_id, "cpu_id"}, > + {&cpu_id, NULL, "node_id"}, > }; > > static void check_getcpu(unsigned int n) > @@ -36,13 +37,13 @@ static void check_getcpu(unsigned int n) > int status; > pid_t pid; > > - if (n == 0) { > - tst_res(TINFO, "Make cpu_id point outside the calling process's address space."); > + tst_res(TINFO, "Test %s outside process's address space", tc->desc); > + > + if (!tc->cpu_id) > tc->cpu_id = tst_get_bad_addr(NULL); > - } else if (n == 1) { > - tst_res(TINFO, "Make node_id point outside the calling process's address space."); > + > + if (!tc->node_id) > tc->node_id = tst_get_bad_addr(NULL); > - } > > pid = SAFE_FORK(); > if (!pid) { Regards, Andrea Cervesato
> Hi Ma, > > > On 8/8/24 11:16, Petr Vorel wrote: > > Hi Ma, > > > >> Add a testcase with the arguments point to an invalid address. > > Generally LGTM, I have few comments, but I'll fix them before merge. > > > >> Reviewed-by: Andrea Cervesato <andrea.cervesato@suse.de> > >> Reviewed-by: Petr Vorel <pvorel@suse.cz> > >> Reviewed-by: Cyril Hrubis <chrubis@suse.cz> > >> Signed-off-by: Ma Xinjian <maxj.fnst@fujitsu.com> > > @Andrea, Cyril, feel free to have final look during today (otherwise > > I'll merge later today). > > > > @Ma The test has changed 3 times quite significantly. IMHO it's better > > not to add Reviewed-by: unless you change just either simple thing or > > did changes you were explicitly asked to do. Better is to --cc all > > people who did the review when generating patches with 'git format-patch'. > Yes, please add Reviewed-by tag only when the reviewer replied with the tag. > That actually means "patch is ok for me, so it can be submitted" > instead of "i took a look at the patch and it needs to be modified". > > > > (It's good to add Reviewed-by: if your patchset has more commits and > > some of them were previously reviewed and they are unchanged in the > > following version.) > > > >> --- > > Also, not needed, but it helps, if you wrote some changes to the > > previous version after these '---' (it will not be part of the git > > commit message when other developers apply your patch with 'git am'. > > > >> runtest/syscalls | 1 + > >> testcases/kernel/syscalls/getcpu/getcpu02.c | 71 > +++++++++++++++++++++ > >> 2 files changed, 72 insertions(+) > >> create mode 100644 testcases/kernel/syscalls/getcpu/getcpu02.c > >> diff --git a/runtest/syscalls b/runtest/syscalls index > >> b8728c1c5..1537b5022 100644 > >> --- a/runtest/syscalls > >> +++ b/runtest/syscalls > >> @@ -448,6 +448,7 @@ futimesat01 futimesat01 > >> getcontext01 getcontext01 > >> getcpu01 getcpu01 > >> +getcpu02 getcpu02 > > You miss adding "/getcpu02" into testcases/kernel/syscalls/getcpu/.gitignore. > > I'll do that before merge. > > > >> getcwd01 getcwd01 > >> getcwd02 getcwd02 > > > >> diff --git a/testcases/kernel/syscalls/getcpu/getcpu02.c > >> b/testcases/kernel/syscalls/getcpu/getcpu02.c > >> new file mode 100644 > >> index 000000000..859cb0d3e > >> --- /dev/null > >> +++ b/testcases/kernel/syscalls/getcpu/getcpu02.c > >> @@ -0,0 +1,71 @@ > >> +// SPDX-License-Identifier: GPL-2.0-or-later > >> +/* > >> + * Copyright (c) 2024 FUJITSU LIMITED. All Rights Reserved. > >> + * Copyright (c) Linux Test Project, 2024 > >> + * Author: Ma Xinjian <maxj.fnst@fujitsu.com> > >> + * > >> + */ > >> + > >> +/*\ > >> + * [Description] > >> + * > >> + * Verify that getcpu(2) fails with EFAULT: > >> + * > >> + * 1) cpu_id points outside the calling process's address space. > >> + * 2) node_id points outside the calling process's address space. > >> + */ > >> + > >> +#define _GNU_SOURCE > >> + > >> +#include "tst_test.h" > >> +#include "lapi/sched.h" > >> + > >> +static unsigned int cpu_id, node_id; > >> + > >> +static struct tcase { > >> + unsigned int *cpu_id; > >> + unsigned int *node_id; > >> +} tcases[] = { > >> + {NULL, &node_id}, > >> + {&cpu_id, NULL} > > I meant to add char *desc here instead of tst_res(TINFO later in > > check_getcpu() > >> +}; > >> + > >> +static void check_getcpu(unsigned int n) { > >> + struct tcase *tc = &tcases[n]; > >> + int status; > >> + pid_t pid; > >> + > >> + if (n == 0) { > > It might be better to check this as: > > if (!tc->cpu_id) > > (set cpu_id only if not set. > > > >> + tst_res(TINFO, "Make cpu_id point outside the calling process's > address space."); > >> + tc->cpu_id = tst_get_bad_addr(NULL); > >> + } else if (n == 1) { > > and here either if (!tc->node_id) or simple else. > >> + tst_res(TINFO, "Make node_id point outside the calling process's > address space."); > >> + tc->node_id = tst_get_bad_addr(NULL); > > Also, we usually try to set values which does not change in the setup > > function (because one can run test more times, e.g. 1000x with > > :/getcpu02 -i1000. Thus things which don't have to be repeated go to the > setup function. > > > > But when I tried that, even without setup function, using this pointer > > always causes SIGSEGV (test passes). And I use direct static values it > > does *not* cause SIGSEGV (test fails): > > > > static unsigned int cpu_id, node_id; > > static unsigned int *p_cpu_id = &cpu_id, *p_node_id = &node_id; > > > > static struct tcase { > > unsigned int **cpu_id; > > unsigned int **node_id; > > char *desc; > > } tcases[] = { > > {NULL, NULL, "none"}, > > {NULL, &p_node_id, "cpu_id"}, > > {&p_cpu_id, NULL, "node_id"}, > > }; > > > > static void check_getcpu(unsigned int n) { > > struct tcase *tc = &tcases[n]; > > int status; > > pid_t pid; > > > > tst_res(TINFO, "Test %s outside process's address space", tc->desc); > > > > pid = SAFE_FORK(); > > if (!pid) { > > TST_EXP_FAIL(getcpu(p_cpu_id, p_node_id), EFAULT); // this would > always pass > > TST_EXP_FAIL(getcpu(*tc->cpu_id, *tc->node_id), EFAULT); // this > > always fail, even for none > > > > exit(0); > > } > > > > I guess I miss something obvious. Therefore I suggest to merge (or see > > the diff below). > > > > Kind regards, > > Petr > > > > static struct tcase { > > unsigned int *cpu_id; > > unsigned int *node_id; > > char *desc; > > } tcases[] = { > > {NULL, &node_id, "cpu_id"}, > > {&cpu_id, NULL, "node_id"}, > > }; > > > > static void check_getcpu(unsigned int n) { > > struct tcase *tc = &tcases[n]; > > int status; > > pid_t pid; > > > > tst_res(TINFO, "Test %s outside process's address space", tc->desc); > > > > if (!tc->cpu_id) > > tc->cpu_id = tst_get_bad_addr(NULL); > > > > if (!tc->node_id) > > tc->node_id = tst_get_bad_addr(NULL); > > > > pid = SAFE_FORK(); > > if (!pid) { > > TST_EXP_FAIL(getcpu(tc->cpu_id, tc->node_id), EFAULT); > > > > exit(0); > > } > > > > SAFE_WAITPID(pid, &status, 0); > > > > if (WIFSIGNALED(status) && WTERMSIG(status) == SIGSEGV) { > > tst_res(TPASS, "getcpu() caused SIGSEGV"); > > return; > > } > > > > if (WIFEXITED(status) && WEXITSTATUS(status) == 0) > > return; > > > > tst_res(TFAIL, "child %s", tst_strstatus(status)); } > > > > diff --git testcases/kernel/syscalls/getcpu/.gitignore > > testcases/kernel/syscalls/getcpu/.gitignore > > index 31fec5d35e..cd3022bbb1 100644 > > --- testcases/kernel/syscalls/getcpu/.gitignore > > +++ testcases/kernel/syscalls/getcpu/.gitignore > > @@ -1 +1,2 @@ > > /getcpu01 > > +/getcpu02 > > diff --git testcases/kernel/syscalls/getcpu/getcpu02.c > > testcases/kernel/syscalls/getcpu/getcpu02.c > > index 859cb0d3eb..83d236a78c 100644 > > --- testcases/kernel/syscalls/getcpu/getcpu02.c > > +++ testcases/kernel/syscalls/getcpu/getcpu02.c > > @@ -9,7 +9,7 @@ > > /*\ > > * [Description] > > * > > - * Verify that getcpu(2) fails with EFAULT: > > + * Verify that getcpu(2) fails with EFAULT if: > > * > > * 1) cpu_id points outside the calling process's address space. > > * 2) node_id points outside the calling process's address space. > > @@ -25,9 +25,10 @@ static unsigned int cpu_id, node_id; > > static struct tcase { > > unsigned int *cpu_id; > > unsigned int *node_id; > > + char *desc; > > } tcases[] = { > > - {NULL, &node_id}, > > - {&cpu_id, NULL} > > + {NULL, &node_id, "cpu_id"}, > > + {&cpu_id, NULL, "node_id"}, > > }; > > > > static void check_getcpu(unsigned int n) @@ -36,13 +37,13 @@ static > > void check_getcpu(unsigned int n) > > int status; > > pid_t pid; > > > > - if (n == 0) { > > - tst_res(TINFO, "Make cpu_id point outside the calling process's > address space."); > > + tst_res(TINFO, "Test %s outside process's address space", tc->desc); > > + > > + if (!tc->cpu_id) > > tc->cpu_id = tst_get_bad_addr(NULL); > > - } else if (n == 1) { > > - tst_res(TINFO, "Make node_id point outside the calling process's > address space."); > > + > > + if (!tc->node_id) > > tc->node_id = tst_get_bad_addr(NULL); > > - } > > > > pid = SAFE_FORK(); > > if (!pid) { > Regards, > Andrea Cervesato Hi Petr, Andrea Thank you for your patient review, I have submitted the [PATCH v5], PTAL. Best regards Ma
diff --git a/runtest/syscalls b/runtest/syscalls index b8728c1c5..1537b5022 100644 --- a/runtest/syscalls +++ b/runtest/syscalls @@ -448,6 +448,7 @@ futimesat01 futimesat01 getcontext01 getcontext01 getcpu01 getcpu01 +getcpu02 getcpu02 getcwd01 getcwd01 getcwd02 getcwd02 diff --git a/testcases/kernel/syscalls/getcpu/getcpu02.c b/testcases/kernel/syscalls/getcpu/getcpu02.c new file mode 100644 index 000000000..859cb0d3e --- /dev/null +++ b/testcases/kernel/syscalls/getcpu/getcpu02.c @@ -0,0 +1,71 @@ +// SPDX-License-Identifier: GPL-2.0-or-later +/* + * Copyright (c) 2024 FUJITSU LIMITED. All Rights Reserved. + * Copyright (c) Linux Test Project, 2024 + * Author: Ma Xinjian <maxj.fnst@fujitsu.com> + * + */ + +/*\ + * [Description] + * + * Verify that getcpu(2) fails with EFAULT: + * + * 1) cpu_id points outside the calling process's address space. + * 2) node_id points outside the calling process's address space. + */ + +#define _GNU_SOURCE + +#include "tst_test.h" +#include "lapi/sched.h" + +static unsigned int cpu_id, node_id; + +static struct tcase { + unsigned int *cpu_id; + unsigned int *node_id; +} tcases[] = { + {NULL, &node_id}, + {&cpu_id, NULL} +}; + +static void check_getcpu(unsigned int n) +{ + struct tcase *tc = &tcases[n]; + int status; + pid_t pid; + + if (n == 0) { + tst_res(TINFO, "Make cpu_id point outside the calling process's address space."); + tc->cpu_id = tst_get_bad_addr(NULL); + } else if (n == 1) { + tst_res(TINFO, "Make node_id point outside the calling process's address space."); + tc->node_id = tst_get_bad_addr(NULL); + } + + pid = SAFE_FORK(); + if (!pid) { + TST_EXP_FAIL(getcpu(tc->cpu_id, tc->node_id), EFAULT); + + exit(0); + } + + SAFE_WAITPID(pid, &status, 0); + + if (WIFSIGNALED(status) && WTERMSIG(status) == SIGSEGV) { + tst_res(TPASS, "getcpu() caused SIGSEGV"); + return; + } + + if (WIFEXITED(status) && WEXITSTATUS(status) == 0) + return; + + tst_res(TFAIL, "child %s", tst_strstatus(status)); +} + +static struct tst_test test = { + .test = check_getcpu, + .tcnt = ARRAY_SIZE(tcases), + .forks_child = 1, +};