The Linux Kernel Mailing List
 help / color / mirror / Atom feed
* [PATCH] selftests: cgroup: simplify cpuset subtree permissions test
@ 2026-08-19 10:31 Shaojie Sun
  2026-08-19 14:34 ` Waiman Long
  2026-08-19 20:10 ` Tejun Heo
  0 siblings, 2 replies; 3+ messages in thread
From: Shaojie Sun @ 2026-08-19 10:31 UTC (permalink / raw)
  To: Waiman Long, Ridong Chen, Tejun Heo, Johannes Weiner,
	Michal Koutný, Shuah Khan
  Cc: cgroups, linux-kselftest, linux-kernel, Shaojie Sun

Simplify test_cpuset_perms_subtree by removing unnecessary chown of
cgroup.procs files. The test verifies implicit migration triggered by
enabling/disabling the cpuset controller via cgroup.subtree_control.

Implicit migration is handled internally by the kernel through
cgroup_update_dfl_csses() when subtree_control is modified, and does
not require cgroup.procs write permission. Only cgroup.subtree_control
permission is needed to trigger the controller state change.

Remove the allocation and cleanup of parent_procs and child_procs
variables that are no longer needed.

Signed-off-by: Shaojie Sun <sunshaojie@kylinos.cn>
---
 tools/testing/selftests/cgroup/test_cpuset.c | 16 +++-------------
 1 file changed, 3 insertions(+), 13 deletions(-)

diff --git a/tools/testing/selftests/cgroup/test_cpuset.c b/tools/testing/selftests/cgroup/test_cpuset.c
index c5cf8b56ceb8..b323addcc519 100644
--- a/tools/testing/selftests/cgroup/test_cpuset.c
+++ b/tools/testing/selftests/cgroup/test_cpuset.c
@@ -167,7 +167,7 @@ static int test_cpuset_perms_object_deny(const char *root)
 static int test_cpuset_perms_subtree(const char *root)
 {
 	char *parent = NULL, *child = NULL;
-	char *parent_procs = NULL, *parent_subctl = NULL, *child_procs = NULL;
+	char *parent_subctl = NULL;
 	const uid_t test_euid = TEST_UID;
 	int object_pid = 0;
 	int ret = KSFT_FAIL;
@@ -175,9 +175,6 @@ static int test_cpuset_perms_subtree(const char *root)
 	parent = cg_name(root, "cpuset_test_0");
 	if (!parent)
 		goto cleanup;
-	parent_procs = cg_name(parent, "cgroup.procs");
-	if (!parent_procs)
-		goto cleanup;
 	parent_subctl = cg_name(parent, "cgroup.subtree_control");
 	if (!parent_subctl)
 		goto cleanup;
@@ -187,16 +184,11 @@ static int test_cpuset_perms_subtree(const char *root)
 	child = cg_name(parent, "cpuset_test_1");
 	if (!child)
 		goto cleanup;
-	child_procs = cg_name(child, "cgroup.procs");
-	if (!child_procs)
-		goto cleanup;
 	if (cg_create(child))
 		goto cleanup;
 
-	/* Enable permissions as in a delegated subtree */
-	if (chown(parent_procs, test_euid, -1) ||
-	    chown(parent_subctl, test_euid, -1) ||
-	    chown(child_procs, test_euid, -1))
+	/* Grant minimal subtree_control permission to trigger implicit migration */
+	if (chown(parent_subctl, test_euid, -1))
 		goto cleanup;
 
 	/* Put a privileged child in the subtree and modify controller state
@@ -221,12 +213,10 @@ static int test_cpuset_perms_subtree(const char *root)
 	}
 
 	cg_destroy(child);
-	free(child_procs);
 	free(child);
 
 	cg_destroy(parent);
 	free(parent_subctl);
-	free(parent_procs);
 	free(parent);
 
 	return ret;
-- 
2.50.1


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] selftests: cgroup: simplify cpuset subtree permissions test
  2026-08-19 10:31 [PATCH] selftests: cgroup: simplify cpuset subtree permissions test Shaojie Sun
@ 2026-08-19 14:34 ` Waiman Long
  2026-08-19 20:10 ` Tejun Heo
  1 sibling, 0 replies; 3+ messages in thread
From: Waiman Long @ 2026-08-19 14:34 UTC (permalink / raw)
  To: Shaojie Sun, Ridong Chen, Tejun Heo, Johannes Weiner,
	Michal Koutný, Shuah Khan
  Cc: cgroups, linux-kselftest, linux-kernel

On 8/19/26 6:31 AM, Shaojie Sun wrote:
> Simplify test_cpuset_perms_subtree by removing unnecessary chown of
> cgroup.procs files. The test verifies implicit migration triggered by
> enabling/disabling the cpuset controller via cgroup.subtree_control.
>
> Implicit migration is handled internally by the kernel through
> cgroup_update_dfl_csses() when subtree_control is modified, and does
> not require cgroup.procs write permission. Only cgroup.subtree_control
> permission is needed to trigger the controller state change.
>
> Remove the allocation and cleanup of parent_procs and child_procs
> variables that are no longer needed.
>
> Signed-off-by: Shaojie Sun <sunshaojie@kylinos.cn>
> ---
>   tools/testing/selftests/cgroup/test_cpuset.c | 16 +++-------------
>   1 file changed, 3 insertions(+), 13 deletions(-)
>
> diff --git a/tools/testing/selftests/cgroup/test_cpuset.c b/tools/testing/selftests/cgroup/test_cpuset.c
> index c5cf8b56ceb8..b323addcc519 100644
> --- a/tools/testing/selftests/cgroup/test_cpuset.c
> +++ b/tools/testing/selftests/cgroup/test_cpuset.c
> @@ -167,7 +167,7 @@ static int test_cpuset_perms_object_deny(const char *root)
>   static int test_cpuset_perms_subtree(const char *root)
>   {
>   	char *parent = NULL, *child = NULL;
> -	char *parent_procs = NULL, *parent_subctl = NULL, *child_procs = NULL;
> +	char *parent_subctl = NULL;
>   	const uid_t test_euid = TEST_UID;
>   	int object_pid = 0;
>   	int ret = KSFT_FAIL;
> @@ -175,9 +175,6 @@ static int test_cpuset_perms_subtree(const char *root)
>   	parent = cg_name(root, "cpuset_test_0");
>   	if (!parent)
>   		goto cleanup;
> -	parent_procs = cg_name(parent, "cgroup.procs");
> -	if (!parent_procs)
> -		goto cleanup;
>   	parent_subctl = cg_name(parent, "cgroup.subtree_control");
>   	if (!parent_subctl)
>   		goto cleanup;
> @@ -187,16 +184,11 @@ static int test_cpuset_perms_subtree(const char *root)
>   	child = cg_name(parent, "cpuset_test_1");
>   	if (!child)
>   		goto cleanup;
> -	child_procs = cg_name(child, "cgroup.procs");
> -	if (!child_procs)
> -		goto cleanup;
>   	if (cg_create(child))
>   		goto cleanup;
>   
> -	/* Enable permissions as in a delegated subtree */
> -	if (chown(parent_procs, test_euid, -1) ||
> -	    chown(parent_subctl, test_euid, -1) ||
> -	    chown(child_procs, test_euid, -1))
> +	/* Grant minimal subtree_control permission to trigger implicit migration */
> +	if (chown(parent_subctl, test_euid, -1))
>   		goto cleanup;
>   
>   	/* Put a privileged child in the subtree and modify controller state
> @@ -221,12 +213,10 @@ static int test_cpuset_perms_subtree(const char *root)
>   	}
>   
>   	cg_destroy(child);
> -	free(child_procs);
>   	free(child);
>   
>   	cg_destroy(parent);
>   	free(parent_subctl);
> -	free(parent_procs);
>   	free(parent);
>   
>   	return ret;
Reviewed-by: Waiman Long <longman@redhat.com>


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] selftests: cgroup: simplify cpuset subtree permissions test
  2026-08-19 10:31 [PATCH] selftests: cgroup: simplify cpuset subtree permissions test Shaojie Sun
  2026-08-19 14:34 ` Waiman Long
@ 2026-08-19 20:10 ` Tejun Heo
  1 sibling, 0 replies; 3+ messages in thread
From: Tejun Heo @ 2026-08-19 20:10 UTC (permalink / raw)
  To: Shaojie Sun
  Cc: Waiman Long, Ridong Chen, Johannes Weiner, Michal Koutny,
	Shuah Khan, cgroups, linux-kselftest, linux-kernel

On Wed, Aug 19, 2026 at 06:31:52PM +0800, Shaojie Sun wrote:
> Simplify test_cpuset_perms_subtree by removing unnecessary chown of
> cgroup.procs files. The test verifies implicit migration triggered by
> enabling/disabling the cpuset controller via cgroup.subtree_control.
>
> Implicit migration is handled internally by the kernel through
> cgroup_update_dfl_csses() when subtree_control is modified, and does
> not require cgroup.procs write permission. Only cgroup.subtree_control
> permission is needed to trigger the controller state change.

That's true but making the migration work isn't why the chowns are there.
cd3c6f682df4 ("selftests: cgroup: Add cpuset migrations testcase") added the
test to verify that migration permissions follow delegation boundaries, with
the setup emulating a delegated subtree, and delegating a subtree includes
granting write access to the "cgroup.procs" files (see the Delegation
section of Documentation/admin-guide/cgroup-v2.rst).

With the chowns removed, the test instead asserts that write access to
"cgroup.subtree_control" alone lets an unprivileged user trigger implicit
migration of a privileged task. That matches the current behavior but isn't
a documented property, and if implicit migration ever becomes stricter about
delegation, the modified test would fail without anything regressing. I'd
rather keep the setup matching the documented delegation model.

Thanks.

-- 
tejun

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-08-19 20:10 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-19 10:31 [PATCH] selftests: cgroup: simplify cpuset subtree permissions test Shaojie Sun
2026-08-19 14:34 ` Waiman Long
2026-08-19 20:10 ` Tejun Heo

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox