| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20221129111055.953833-2-peternewman@google.com>
Date: Tue, 29 Nov 2022 12:10:54 +0100
From: Peter Newman <peternewman@...gle.com>
To: reinette.chatre@...el.com, fenghua.yu@...el.com
Cc: bp@...en8.de, derkling@...gle.com, eranian@...gle.com,
hpa@...or.com, james.morse@....com, jannh@...gle.com,
kpsingh@...gle.com, linux-kernel@...r.kernel.org, mingo@...hat.com,
tglx@...utronix.de, x86@...nel.org,
Peter Newman <peternewman@...gle.com>
Subject: [PATCH v4 1/2] x86/resctrl: Update task closid/rmid with task_call_func()
When the user moves a running task to a new rdtgroup using the tasks
file interface, the resulting change in CLOSID/RMID must be immediately
propagated to the PQR_ASSOC MSR on the task's CPU.
It is possible for a task to wake up or migrate while it is being moved
to a new group. If __rdtgroup_move_task() fails to observe that a task
has begun running or misses that it migrated to a new CPU, the task will
continue to use the old CLOSID or RMID until it switches in again.
__rdtgroup_move_task() assumes that if the task migrates off of its CPU
before it can IPI the task, then the task has already observed the
updated CLOSID/RMID. Because this is done locklessly and an x86 CPU can
delay stores until after loads, the following incorrect scenarios are
possible:
1. __rdtgroup_move_task() stores the new closid and rmid in
the task structure after it loads task_curr() and task_cpu().
2. resctrl_sched_in() loads t->{closid,rmid} before the calling context
switch stores new task_curr() and task_cpu() values.
Use task_call_func() in __rdtgroup_move_task() to serialize updates to
the closid and rmid fields in the task_struct with context switch.
Signed-off-by: Peter Newman <peternewman@...gle.com>
Reviewed-by: James Morse <james.morse@....com>
---
arch/x86/kernel/cpu/resctrl/rdtgroup.c | 78 ++++++++++++++++----------
1 file changed, 47 insertions(+), 31 deletions(-)
diff --git a/arch/x86/kernel/cpu/resctrl/rdtgroup.c b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
index e5a48f05e787..59b7ffcd53bb 100644
--- a/arch/x86/kernel/cpu/resctrl/rdtgroup.c
+++ b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
@@ -528,6 +528,31 @@ static void rdtgroup_remove(struct rdtgroup *rdtgrp)
kfree(rdtgrp);
}
+static int update_locked_task_closid_rmid(struct task_struct *t, void *arg)
+{
+ struct rdtgroup *rdtgrp = arg;
+
+ /*
+ * Although task_call_func() serializes the writes below with the paired
+ * reads in resctrl_sched_in(), resctrl_sched_in() still needs
+ * READ_ONCE() due to rdt_move_group_tasks(), so use WRITE_ONCE() here
+ * to conform.
+ */
+ if (rdtgrp->type == RDTCTRL_GROUP) {
+ WRITE_ONCE(t->closid, rdtgrp->closid);
+ WRITE_ONCE(t->rmid, rdtgrp->mon.rmid);
+ } else if (rdtgrp->type == RDTMON_GROUP) {
+ WRITE_ONCE(t->rmid, rdtgrp->mon.rmid);
+ }
+
+ /*
+ * If the task is current on a CPU, the PQR_ASSOC MSR needs to be
+ * updated to make the resource group go into effect. If the task is not
+ * current, the MSR will be updated when the task is scheduled in.
+ */
+ return task_curr(t);
+}
+
static void _update_task_closid_rmid(void *task)
{
/*
@@ -538,10 +563,24 @@ static void _update_task_closid_rmid(void *task)
resctrl_sched_in();
}
-static void update_task_closid_rmid(struct task_struct *t)
+static void update_task_closid_rmid(struct task_struct *t,
+ struct rdtgroup *rdtgrp)
{
- if (IS_ENABLED(CONFIG_SMP) && task_curr(t))
- smp_call_function_single(task_cpu(t), _update_task_closid_rmid, t, 1);
+ /*
+ * Serialize the closid and rmid update with context switch. If
+ * task_call_func() indicates that the task was running during
+ * update_locked_task_closid_rmid(), then interrupt it.
+ */
+ if (task_call_func(t, update_locked_task_closid_rmid, rdtgrp) &&
+ IS_ENABLED(CONFIG_SMP))
+ /*
+ * If the task has migrated away from the CPU indicated by
+ * task_cpu() below, then it has already switched in on the
+ * new CPU using the updated closid and rmid and the call below
+ * is unnecessary, but harmless.
+ */
+ smp_call_function_single(task_cpu(t),
+ _update_task_closid_rmid, t, 1);
else
_update_task_closid_rmid(t);
}
@@ -557,39 +596,16 @@ static int __rdtgroup_move_task(struct task_struct *tsk,
return 0;
/*
- * Set the task's closid/rmid before the PQR_ASSOC MSR can be
- * updated by them.
- *
- * For ctrl_mon groups, move both closid and rmid.
* For monitor groups, can move the tasks only from
* their parent CTRL group.
*/
-
- if (rdtgrp->type == RDTCTRL_GROUP) {
- WRITE_ONCE(tsk->closid, rdtgrp->closid);
- WRITE_ONCE(tsk->rmid, rdtgrp->mon.rmid);
- } else if (rdtgrp->type == RDTMON_GROUP) {
- if (rdtgrp->mon.parent->closid == tsk->closid) {
- WRITE_ONCE(tsk->rmid, rdtgrp->mon.rmid);
- } else {
- rdt_last_cmd_puts("Can't move task to different control group\n");
- return -EINVAL;
- }
+ if (rdtgrp->type == RDTMON_GROUP &&
+ rdtgrp->mon.parent->closid != tsk->closid) {
+ rdt_last_cmd_puts("Can't move task to different control group\n");
+ return -EINVAL;
}
- /*
- * Ensure the task's closid and rmid are written before determining if
- * the task is current that will decide if it will be interrupted.
- */
- barrier();
-
- /*
- * By now, the task's closid and rmid are set. If the task is current
- * on a CPU, the PQR_ASSOC MSR needs to be updated to make the resource
- * group go into effect. If the task is not current, the MSR will be
- * updated when the task is scheduled in.
- */
- update_task_closid_rmid(tsk);
+ update_task_closid_rmid(tsk, rdtgrp);
return 0;
}
--
2.38.1.584.g0f3c55d4c2-goog
Powered by blists - more mailing lists