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

Shaojie Sun posted 1 patch 1 month, 2 weeks ago
tools/testing/selftests/cgroup/test_core.c | 11 ++---------
1 file changed, 2 insertions(+), 9 deletions(-)
[PATCH v2] selftests/cgroup: remove redundant chown in test_cgcore_lesser_ns_open
Posted by Shaojie Sun 1 month, 2 weeks ago
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
Posted by Tejun Heo 1 month, 2 weeks ago
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
Re: [PATCH v2] selftests/cgroup: remove redundant chown in test_cgcore_lesser_ns_open
Posted by Tao Cui 1 month, 2 weeks ago

在 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>