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

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

[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 variable.

Signed-off-by: Shaojie Sun <sunshaojie@kylinos.cn>
---
 tools/testing/selftests/cgroup/test_core.c | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/tools/testing/selftests/cgroup/test_core.c b/tools/testing/selftests/cgroup/test_core.c
index 88ca832d4fc1..483e8b063f12 100644
--- a/tools/testing/selftests/cgroup/test_core.c
+++ b/tools/testing/selftests/cgroup/test_core.c
@@ -795,7 +795,6 @@ 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;
@@ -825,10 +824,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,
-- 
2.25.1

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

[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 <sunshaojie@kylinos.cn>
---
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;
-- 
2.25.1

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

[Tao Cui]

在 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 <sunshaojie@kylinos.cn>
> ---
> 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 <cuitao@kylinos.cn>

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

[Tejun Heo]

On Fri, Aug 14, 2026 at 06:00:02PM +0800, Shaojie Sun wrote:
> 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.

Applied to cgroup/for-7.3 with the subject capitalized.

Thanks.

--
tejun