Re: [LTP] [PATCH v2] Confirming EPERM is returned when CAP_SYS_ADMIN is removed from clone3. Signed-off-by: Stephen Bertram <sbertram@redhat.com>

[Cyril Hrubis <chrubis@suse.cz>]

Hi!
First of all the patch description shouldn't be on a single line since
the first line ends up in the email subject. There should be newlines
between the subject, patch description and the signed-off-by.

> ---
>  runtest/syscalls                            |  1 +
>  testcases/kernel/syscalls/clone3/.gitignore |  1 +
>  testcases/kernel/syscalls/clone3/clone304.c | 63 +++++++++++++++++++++
>  3 files changed, 65 insertions(+)
>  create mode 100644 testcases/kernel/syscalls/clone3/clone304.c
> 
> diff --git a/runtest/syscalls b/runtest/syscalls
> index 54d94c0ca..b2c4f338e 100644
> --- a/runtest/syscalls
> +++ b/runtest/syscalls
> @@ -128,6 +128,7 @@ clone10 clone10
>  clone301 clone301
>  clone302 clone302
>  clone303 clone303
> ++clone304 clone304
   ^
   This + shouldn't be there I suppose.

>  close01 close01
>  close02 close02
> diff --git a/testcases/kernel/syscalls/clone3/.gitignore b/testcases/kernel/syscalls/clone3/.gitignore
> index 10369954b..e9b5312f4 100644
> --- a/testcases/kernel/syscalls/clone3/.gitignore
> +++ b/testcases/kernel/syscalls/clone3/.gitignore
> @@ -1,3 +1,4 @@
>  clone301
>  clone302
>  clone303
> +clone304
> diff --git a/testcases/kernel/syscalls/clone3/clone304.c b/testcases/kernel/syscalls/clone3/clone304.c
> new file mode 100644
> index 000000000..8d0d85bd4
> --- /dev/null
> +++ b/testcases/kernel/syscalls/clone3/clone304.c
> @@ -0,0 +1,63 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Copyright (c) 2025 Stephen Bertram <sbertram@redhat.com>
> + */
> +
> +/*\
> + * This test verifies that clone3() fals with EPERM when CAP_SYS_ADMIN
> + * has been dropped and ``clone_args.set_tid_size`` is greater than zero.
> + */
> +
> +#define _GNU_SOURCE
> +#include "tst_test.h"
> +#include "lapi/sched.h"
> +
> +struct clone_args args = {0};

Ideally this should be allocated via the guarded buffers:

	struct clone_args *args;

	struct tst_test test = {
		...
		.bufs = (struct tst_buffers []) {
			{&args, size = sizeof(*args)},
			{}
		}
		...
	}

https://linux-test-project.readthedocs.io/en/latest/developers/api_c_tests.html#guarded-buffers

> +static struct tcase {
> +	uint64_t flags;
> +	char *sflags;
> +} tcases[] = {
> +	{CLONE_NEWPID, "CLONE_NEWPID"},
> +	{CLONE_NEWCGROUP, "CLONE_NEWCGROUP"},
> +	{CLONE_NEWIPC, "CLONE_NEWIPC"},
> +	{CLONE_NEWNET, "CLONE_NEWNET"},
> +	{CLONE_NEWNS, "CLONE_NEWNS"},
> +	{CLONE_NEWUTS, "CLONE_NEWUTS"},
> +};
> +
> +static void run(unsigned int n)
> +{
> +	struct tcase *tc = &tcases[n];
> +	pid_t tid_array[4] = {0, 0, 0, 0};
> +
> +	args.flags = tc->flags;
> +	args.set_tid = (uint64_t)(uintptr_t)tid_array;
> +
> +	TST_EXP_FAIL(clone3(&args, sizeof(args)), EPERM, "clone3(%s) should fail with EPERM",tc->sflags);
                                                                                             ^
											     missing space
> +}
> +
> +static void setup(void)
> +{
> +	clone3_supported_by_kernel();
> +
> +	args.pidfd = 0;
> +	args.child_tid = 0;
> +	args.parent_tid = 0;
> +	args.exit_signal = 0;
> +	args.stack = 0;
> +	args.stack_size = 0;
> +	args.tls = 0;

memset(args, 0, sizeof(*args) is probably safer.

> +	args.set_tid_size = 4;  // Greater than zero - requires CAP_SYS_ADMIN
> +}
> +
> +static struct tst_test test = {
> +	.tcnt = ARRAY_SIZE(tcases),
> +	.setup = setup,
> +	.test = run,
> +	.needs_root = 1,
> +	.caps = (struct tst_cap []) {
> +				TST_CAP(TST_CAP_DROP, CAP_SYS_ADMIN),
> +				{}
           ^
	   Just single tab here.
> +	},
> +};
> -- 
> 2.49.0
> 
> 
> -- 
> Mailing list info: https://lists.linux.it/listinfo/ltp

-- 
Cyril Hrubis
chrubis@suse.cz

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp