[v3] getcpu: Add testcase for EFAULT

Add a testcase with the arguments point to an invalid address.

Reviewed-by: Petr Vorel <pvorel@suse.cz>
Reviewed-by: Cyril Hrubis <chrubis@suse.cz>
Signed-off-by: Ma Xinjian <maxj.fnst@fujitsu.com>
---
 runtest/syscalls                            |  1 +
 testcases/kernel/syscalls/getcpu/getcpu02.c | 97 +++++++++++++++++++++
 2 files changed, 98 insertions(+)
 create mode 100644 testcases/kernel/syscalls/getcpu/getcpu02.c

Hi!

On 8/5/24 11:22, Ma Xinjian via ltp wrote:
> Add a testcase with the arguments point to an invalid address.
>
> Reviewed-by: Petr Vorel <pvorel@suse.cz>
> Reviewed-by: Cyril Hrubis <chrubis@suse.cz>
> Signed-off-by: Ma Xinjian <maxj.fnst@fujitsu.com>
> ---
>   runtest/syscalls                            |  1 +
>   testcases/kernel/syscalls/getcpu/getcpu02.c | 97 +++++++++++++++++++++
>   2 files changed, 98 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
>   
>   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..f32660ef9
> --- /dev/null
> +++ b/testcases/kernel/syscalls/getcpu/getcpu02.c
> @@ -0,0 +1,97 @@
> +// 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 arguments point outside the calling process's address
> + *          space.
We can squeeze the description in one single line since EFAULT is the 
only one we are gonna test.
> + */
> +
> +#define _GNU_SOURCE
> +#include <errno.h>
> +#include <sys/resource.h>
> +#include <sys/time.h>
> +#include <sys/wait.h>
> +#include <stdlib.h>
tst_test.h is importing these already.
> +
> +#include "tst_test.h"
> +#include "lapi/sched.h"
> +
> +static void *bad_addr;
> +
> +static void setup(void)
> +{
> +	bad_addr = tst_get_bad_addr(NULL);
> +}
> +
> +static void check_bad_cpu_id(void *bad_addr, unsigned int *node_id)
> +{
> +	int status;
> +	pid_t pid;
> +
> +	pid = SAFE_FORK();
> +	if (!pid) {
> +		TST_EXP_FAIL(getcpu(bad_addr, 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 void check_bad_node_id(unsigned int *cpu_id, void *bad_addr)
> +{
> +	int status;
> +	pid_t pid;
> +
> +	pid = SAFE_FORK();
> +	if (!pid) {
> +		TST_EXP_FAIL(getcpu(cpu_id, bad_addr), 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 void run_test(void)
> +{
> +	unsigned int cpu_id, node_id = 0;
> +
> +	check_bad_cpu_id(bad_addr, &node_id);
> +	check_bad_node_id(&cpu_id, bad_addr);
Here we can use .test/.tcnt , defining 2 testcases with different 
getcpu() input values.
> +}
> +
> +static struct tst_test test = {
> +	.setup = setup,
> +	.test_all = run_test,
> +	.forks_child = 1,
> +};

Best regards,
Andrea Cervesato

> Hi!

> On 8/5/24 11:22, Ma Xinjian via ltp wrote:
> > Add a testcase with the arguments point to an invalid address.

> > Reviewed-by: Petr Vorel <pvorel@suse.cz>
> > Reviewed-by: Cyril Hrubis <chrubis@suse.cz>
> > Signed-off-by: Ma Xinjian <maxj.fnst@fujitsu.com>
> > ---
> >   runtest/syscalls                            |  1 +
> >   testcases/kernel/syscalls/getcpu/getcpu02.c | 97 +++++++++++++++++++++
> >   2 files changed, 98 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
> >   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..f32660ef9
> > --- /dev/null
> > +++ b/testcases/kernel/syscalls/getcpu/getcpu02.c
> > @@ -0,0 +1,97 @@
> > +// 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 arguments point outside the calling process's address
> > + *          space.
> We can squeeze the description in one single line since EFAULT is the only
> one we are gonna test.

Good point. As Andrea suggested to use .tcnt, defining 2 testcases with
different getcpu() input values, these two would deserve to be described.

Also it'd be nice to add some tst_res(TINFO, ...) description in the testing
function (similar to one above in /*\.

Kind regards,
Petr

> > + */
> > +
> > +#define _GNU_SOURCE
> > +#include <errno.h>
> > +#include <sys/resource.h>
> > +#include <sys/time.h>
> > +#include <sys/wait.h>
> > +#include <stdlib.h>
> tst_test.h is importing these already.
> > +
> > +#include "tst_test.h"
> > +#include "lapi/sched.h"
> > +
> > +static void *bad_addr;
> > +
> > +static void setup(void)
> > +{
> > +	bad_addr = tst_get_bad_addr(NULL);
> > +}
> > +
> > +static void check_bad_cpu_id(void *bad_addr, unsigned int *node_id)
> > +{
> > +	int status;
> > +	pid_t pid;
> > +
> > +	pid = SAFE_FORK();
> > +	if (!pid) {
> > +		TST_EXP_FAIL(getcpu(bad_addr, 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 void check_bad_node_id(unsigned int *cpu_id, void *bad_addr)
> > +{
> > +	int status;
> > +	pid_t pid;
> > +
> > +	pid = SAFE_FORK();
> > +	if (!pid) {
> > +		TST_EXP_FAIL(getcpu(cpu_id, bad_addr), 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));
> > +}

These two testing functions are nearly the same, only parameters are different.
How about creating a single function and just pass a different params?
Also, only bad_addr and one of the functions holding "0" as id is needed.
> > +
> > +static void run_test(void)
> > +{
> > +	unsigned int cpu_id, node_id = 0;
> > +
> > +	check_bad_cpu_id(bad_addr, &node_id);
> > +	check_bad_node_id(&cpu_id, bad_addr);

> Here we can use .test/.tcnt , defining 2 testcases with different getcpu()
> input values.
+1

NOTE: sometimes we create struct tcase, but it'd be overkill for these 2
functions.

Kind regards,
Petr

> > +}
> > +
> > +static struct tst_test test = {
> > +	.setup = setup,
> > +	.test_all = run_test,
> > +	.forks_child = 1,
> > +};

> Best regards,
> Andrea Cervesato