[PATCH v3 0/2] cgroup/cpuset: Fix v1 task migration failure from empty cpuset

[PATCH v3 0/2] cgroup/cpuset: Fix v1 task migration failure from empty cpuset

[Waiman Long]

v3:
 - Drop patch 3
 - Further simplify patch 2 by dropping the flag and only check for
   v1 cpuset with no CPU to disable security check.

v2:
  - Add a new CS_TASKS_OUT flag to signal that task migration out of
    empty cpuset is allowed without setsched security check as suggested
    by Tejun.
  - Add 2 more patches with minor changes.

As it is found that the cpuset v1 task migration out of cpuset with no
CPU can be blocked by a strict security policy, we need to work around
that issue by treating it as an exceptional case that is allowed without
security check in cpuset_can_attach().

Waiman Long (2):
  cgroup/cpuset: Simplify setsched decision check in task iteration loop
    of cpuset_can_attach()
  cgroup/cpuset: Skip security check for hotplug induced v1 task
    migration

 kernel/cgroup/cpuset.c | 29 ++++++++++++++++++++---------
 1 file changed, 20 insertions(+), 9 deletions(-)

-- 
2.53.0

[PATCH v3 1/2] cgroup/cpuset: Simplify setsched decision check in task iteration loop of cpuset_can_attach()

[Waiman Long]

Centralize the check required to run security_task_setscheduler() in
the task iteration loop of cpuset_can_attach() outside of the loop as
it has no dependency on the characteristics of the tasks themselves.

There is no functional change.

Signed-off-by: Waiman Long <longman@redhat.com>
---
 kernel/cgroup/cpuset.c | 19 ++++++++++---------
 1 file changed, 10 insertions(+), 9 deletions(-)

diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
index d21868455341..58c5b7b72cca 100644
--- a/kernel/cgroup/cpuset.c
+++ b/kernel/cgroup/cpuset.c
@@ -2988,7 +2988,7 @@ static int cpuset_can_attach(struct cgroup_taskset *tset)
 	struct cgroup_subsys_state *css;
 	struct cpuset *cs, *oldcs;
 	struct task_struct *task;
-	bool cpus_updated, mems_updated;
+	bool setsched_check;
 	int ret;
 
 	/* used later by cpuset_attach() */
@@ -3003,20 +3003,21 @@ static int cpuset_can_attach(struct cgroup_taskset *tset)
 	if (ret)
 		goto out_unlock;
 
-	cpus_updated = !cpumask_equal(cs->effective_cpus, oldcs->effective_cpus);
-	mems_updated = !nodes_equal(cs->effective_mems, oldcs->effective_mems);
+	/*
+	 * Skip rights over task setsched check in v2 when nothing changes,
+	 * migration permission derives from hierarchy ownership in
+	 * cgroup_procs_write_permission()).
+	 */
+	setsched_check = !cpuset_v2() ||
+		!cpumask_equal(cs->effective_cpus, oldcs->effective_cpus) ||
+		!nodes_equal(cs->effective_mems, oldcs->effective_mems);
 
 	cgroup_taskset_for_each(task, css, tset) {
 		ret = task_can_attach(task);
 		if (ret)
 			goto out_unlock;
 
-		/*
-		 * Skip rights over task check in v2 when nothing changes,
-		 * migration permission derives from hierarchy ownership in
-		 * cgroup_procs_write_permission()).
-		 */
-		if (!cpuset_v2() || (cpus_updated || mems_updated)) {
+		if (setsched_check) {
 			ret = security_task_setscheduler(task);
 			if (ret)
 				goto out_unlock;
-- 
2.53.0

[PATCH v3 2/2] cgroup/cpuset: Skip security check for hotplug induced v1 task migration

[Waiman Long]

When a CPU hot removal causes a v1 cpuset to lose all its CPUs, the
cpuset hotplug handler will schedule a work function to migrate tasks
in that cpuset with no CPU to its ancestor to enable those tasks to
continue running.

If a strict security policy is in place, however, the task migration
may fail when security_task_setscheduler() call in cpuset_can_attach()
returns a -EACCESS error. That will mean that those tasks will have
no CPU to run on. The system administrators will have to explicitly
intervene to either add CPUs to that cpuset or move the tasks elsewhere
if they are aware of it.

This problem was found by a reported test failure in the LTP's
cpuset_hotplug_test.sh. Fix this problem by treating this special case as
an exception to skip the setsched security check in cpuset_can_attach()
when a v1 cpuset with tasks have no CPU left.

With that patch applied, the cpuset_hotplug_test.sh test can be run
successfully without failure.

Signed-off-by: Waiman Long <longman@redhat.com>
---
 kernel/cgroup/cpuset.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
index 58c5b7b72cca..1335e437098e 100644
--- a/kernel/cgroup/cpuset.c
+++ b/kernel/cgroup/cpuset.c
@@ -3012,6 +3012,16 @@ static int cpuset_can_attach(struct cgroup_taskset *tset)
 		!cpumask_equal(cs->effective_cpus, oldcs->effective_cpus) ||
 		!nodes_equal(cs->effective_mems, oldcs->effective_mems);
 
+	/*
+	 * A v1 cpuset with tasks will have no CPU left only when CPU hotplug
+	 * brings the last online CPU offline as users are not allowed to empty
+	 * cpuset.cpus when there are active tasks inside. When that happens,
+	 * we should allow tasks to migrate out without security check to make
+	 * sure they will be able to run after migration.
+	 */
+	if (!is_in_v2_mode() && cpumask_empty(oldcs->effective_cpus))
+		setsched_check = false;
+
 	cgroup_taskset_for_each(task, css, tset) {
 		ret = task_can_attach(task);
 		if (ret)
-- 
2.53.0

Re: [PATCH v3 0/2] cgroup/cpuset: Fix v1 task migration failure from empty cpuset

[Tejun Heo]

Hello,

> Waiman Long (2):
>   cgroup/cpuset: Simplify setsched decision check in task iteration loop
>     of cpuset_can_attach()
>   cgroup/cpuset: Skip security check for hotplug induced v1 task
>     migration

Applied 1-2 to cgroup/for-7.0-fixes with a s/EACCESS/EACCES/ typo fix
in the second patch's commit message.

Thanks.

--
tejun