Re: [PATCH v2] selftests/cgroup: remove redundant chown in test_cgcore_lesser_ns_open

在 2026/8/14 18:00, Shaojie Sun 写道:
> test_cgcore_lesser_ns_open runs as root throughout and never changes its
> euid, so chowning the two cgroup.procs files to a non-root uid has no
> effect on the test.
> 
> The ENOENT the test expects comes from the cgroup namespace delegation
> check in cgroup_procs_write_permission(): the source and destination
> cgroups must both be descendants of the namespace root captured at open
> time.  That check does not depend on file ownership.  In addition, the
> permission check only examines the common ancestor's cgroup.procs file
> (the test root here), which the chown calls do not touch.
> 
> Remove the redundant chown calls and the now unused test_euid and
> cg_test_a_procs variables.
> 
> Signed-off-by: Shaojie Sun <[email protected]>
> ---
> Changes in v2:
> - Remove the now unused cg_test_a_procs variable, as suggested by
>   sashiko-bot.
> 
>  tools/testing/selftests/cgroup/test_core.c | 11 ++---------
>  1 file changed, 2 insertions(+), 9 deletions(-)
> 
> diff --git a/tools/testing/selftests/cgroup/test_core.c b/tools/testing/selftests/cgroup/test_core.c
> index 88ca832d4fc1..5501be9912c0 100644
> --- a/tools/testing/selftests/cgroup/test_core.c
> +++ b/tools/testing/selftests/cgroup/test_core.c
> @@ -795,10 +795,9 @@ static int lesser_ns_open_thread_fn(void *arg)
>  static int test_cgcore_lesser_ns_open(const char *root)
>  {
>  	static char stack[65536];
> -	const uid_t test_euid = 65534;	/* usually nobody, any !root is fine */
>  	int ret = KSFT_FAIL;
>  	char *cg_test_a = NULL, *cg_test_b = NULL;
> -	char *cg_test_a_procs = NULL, *cg_test_b_procs = NULL;
> +	char *cg_test_b_procs = NULL;
>  	int cg_test_b_procs_fd = -1;
>  	struct lesser_ns_open_thread_arg targ = { .fd = -1 };
>  	pid_t pid;
> @@ -813,10 +812,9 @@ static int test_cgcore_lesser_ns_open(const char *root)
>  	if (!cg_test_a || !cg_test_b)
>  		goto cleanup;
>  
> -	cg_test_a_procs = cg_name(cg_test_a, "cgroup.procs");
>  	cg_test_b_procs = cg_name(cg_test_b, "cgroup.procs");
>  
> -	if (!cg_test_a_procs || !cg_test_b_procs)
> +	if (!cg_test_b_procs)
>  		goto cleanup;
>  
>  	if (cg_create(cg_test_a) || cg_create(cg_test_b))
> @@ -825,10 +823,6 @@ static int test_cgcore_lesser_ns_open(const char *root)
>  	if (cg_enter_current(cg_test_b))
>  		goto cleanup;
>  
> -	if (chown(cg_test_a_procs, test_euid, -1) ||
> -	    chown(cg_test_b_procs, test_euid, -1))
> -		goto cleanup;
> -
>  	targ.path = cg_test_b_procs;
>  	pid = clone(lesser_ns_open_thread_fn, stack + sizeof(stack),
>  		    CLONE_NEWCGROUP | CLONE_FILES | CLONE_VM | SIGCHLD,
> @@ -863,7 +857,6 @@ static int test_cgcore_lesser_ns_open(const char *root)
>  	if (cg_test_a)
>  		cg_destroy(cg_test_a);
>  	free(cg_test_b_procs);
> -	free(cg_test_a_procs);
>  	free(cg_test_b);
>  	free(cg_test_a);
>  	return ret;
Reviewed-by: Tao Cui <[email protected]>