* [PATCH v2] sched/cache: Remove the old cache group footprint on exec
@ 2026-10-10 11:16 Jemmy Wong
2026-10-10 15:04 ` Chen Yu
0 siblings, 1 reply; 2+ messages in thread
From: Jemmy Wong @ 2026-10-10 11:16 UTC (permalink / raw)
To: Ingo Molnar, Peter Zijlstra, Tim Chen
Cc: Juri Lelli, Vincent Guittot, Dietmar Eggemann, Steven Rostedt,
Ben Segall, Mel Gorman, Valentin Schneider, K Prateek Nayak,
Chen Yu, linux-kernel, Jemmy Wong
Exec replaces the task's cache group before resetting its NUMA fault
statistics, but only the exit path subtracts the task's contribution
from the old group's footprint.
Although de_thread() removes other members of the executing task's
thread group, tasks sharing the mm through CLONE_VM without CLONE_THREAD
(e.g., vfork()) can retain the old mm and cache group. The executing
task's contribution then remains without further updates or decay,
potentially suppressing cache-aware aggregation through the LLC
capacity check.
A vfork child rarely accumulates NUMA faults before exec because NUMA
scanning starts late. The issue mainly concerns longer-lived CLONE_VM
tasks that accumulate NUMA faults and later exec. This issue was found
by code inspection, not a workload reproducer.
Factor the existing footprint subtraction into a helper and use it when
leaving a cache group on both exec and exit. Subtract before dropping the
old group reference, preserving the existing underflow protection.
Document that exec must subtract the contribution before
task_numa_free() resets p->total_numa_faults.
Fixes: b636fef85bda ("sched/cache: Introduce task_struct->sched_cache_grp to fix UAF")
Signed-off-by: Jemmy Wong <jemmywong512@gmail.com>
Reviewed-by: Tim Chen <tim.c.chen@linux.intel.com>
---
v1: https://lore.kernel.org/all/20261009151215.62878-1-jemmywong512@gmail.com/
Changes in v2:
- Mention vfork() and that a vfork child rarely accumulates NUMA faults
before exec. The leak mainly matters for longer-lived CLONE_VM tasks.
Note that this was found by code inspection.
- Add a Fixes: tag.
- Comment that footprint subtraction must stay ahead of task_numa_free()
in bprm_execve(), and that nothing enforces that order.
- Add Reviewed-by: Tim Chen.
kernel/sched/fair.c | 55 +++++++++++++++++++++++++++------------------
1 file changed, 33 insertions(+), 22 deletions(-)
diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index 57360f5cdde4..719a3fc3651f 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -1771,6 +1771,25 @@ void sched_cache_fork_cleanup(struct task_struct *p)
RCU_INIT_POINTER(p->sched_cache_grp, NULL);
}
+static void sched_cache_footprint_sub(struct sched_cache_group *grp,
+ struct task_struct *p)
+{
+#ifdef CONFIG_NUMA_BALANCING
+ /*
+ * Remove this task's contribution when it leaves the group, either
+ * through exit or exec. Other thread groups sharing the old mm can
+ * keep the group alive after exec resets this task's NUMA statistics.
+ * Unlocked for performance; clamp to avoid underflow.
+ */
+ if (grp && p->total_numa_faults) {
+ unsigned long fp = READ_ONCE(grp->footprint);
+ unsigned long sub = min(fp, p->total_numa_faults);
+
+ WRITE_ONCE(grp->footprint, fp - sub);
+ }
+#endif
+}
+
void sched_cache_exec_mmap(struct task_struct *p, struct mm_struct *mm)
{
struct sched_cache_group *old;
@@ -1778,8 +1797,13 @@ void sched_cache_exec_mmap(struct task_struct *p, struct mm_struct *mm)
/*
* Acquire the new reference before publishing the pointer, then drop
* the old one. @p is current and the only writer of its own pointer.
+ *
+ * Footprint subtraction must stay ahead of task_numa_free() in
+ * bprm_execve(), which clears p->total_numa_faults. Nothing here
+ * enforces that order.
*/
old = sched_cache_replace_grp(p, sched_cache_group_get(mm->sched_cache_grp));
+ sched_cache_footprint_sub(old, p);
sched_cache_group_put(old);
}
@@ -1787,19 +1811,7 @@ void sched_cache_exit_mm(struct task_struct *p)
{
struct sched_cache_group *grp = sched_cache_replace_grp(p, NULL);
-#ifdef CONFIG_NUMA_BALANCING
- /*
- * Subtract this task's footprint from the group before dropping the
- * reference, so the group footprint converges as its threads exit.
- * Unlocked for performance; clamp to avoid underflow.
- */
- if (grp && p->total_numa_faults) {
- unsigned long fp = READ_ONCE(grp->footprint);
- unsigned long sub = min(fp, p->total_numa_faults);
-
- WRITE_ONCE(grp->footprint, fp - sub);
- }
-#endif
+ sched_cache_footprint_sub(grp, p);
sched_cache_group_put(grp);
}
@@ -3975,16 +3987,15 @@ static void task_numa_placement(struct task_struct *p)
* sharing this mm. Acceptable since footprint is a
* heuristic and occasional lost updates are tolerable.
*
- * If a task exits, its corresponding footprint must
- * be subtracted from p->sched_cache_grp->footprint,
- * otherwise the footprint will not converge: the
- * exiting thread's footprint remains unchanged/undecayed.
- * See exit_mm().
+ * If a task leaves its cache group through exit or exec,
+ * its contribution must be subtracted from the old group's
+ * footprint. Otherwise, that contribution remains
+ * unchanged/undecayed while other tasks keep the group alive.
+ * See sched_cache_footprint_sub().
*
- * Lost updates and unsynchronized subtraction
- * in exit_mm() can cause footprint + diff to
- * go negative. Clamp to zero to prevent the
- * unsigned footprint from wrapping.
+ * Lost updates and unsynchronized subtraction on exit or
+ * exec can cause footprint + diff to go negative. Clamp
+ * to zero to prevent the unsigned footprint from wrapping.
*/
scoped_guard(rcu) {
grp = rcu_dereference(p->sched_cache_grp);
--
2.54.0 (Apple Git-157)
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: [PATCH v2] sched/cache: Remove the old cache group footprint on exec
2026-10-10 11:16 [PATCH v2] sched/cache: Remove the old cache group footprint on exec Jemmy Wong
@ 2026-10-10 15:04 ` Chen Yu
0 siblings, 0 replies; 2+ messages in thread
From: Chen Yu @ 2026-10-10 15:04 UTC (permalink / raw)
To: Jemmy Wong
Cc: Ingo Molnar, Peter Zijlstra, Tim Chen, Juri Lelli,
Vincent Guittot, Dietmar Eggemann, Steven Rostedt, Ben Segall,
Mel Gorman, Valentin Schneider, K Prateek Nayak, chen.yu,
linux-kernel
On Sat, Oct 10, 2026 at 07:16:12PM +0800, Jemmy Wong wrote:
[ ... ]
> void sched_cache_exec_mmap(struct task_struct *p, struct mm_struct *mm)
> {
> struct sched_cache_group *old;
> @@ -1778,8 +1797,13 @@ void sched_cache_exec_mmap(struct task_struct *p, struct mm_struct *mm)
> /*
> * Acquire the new reference before publishing the pointer, then drop
> * the old one. @p is current and the only writer of its own pointer.
> + *
> + * Footprint subtraction must stay ahead of task_numa_free() in
> + * bprm_execve(), which clears p->total_numa_faults. Nothing here
> + * enforces that order.
> */
> old = sched_cache_replace_grp(p, sched_cache_group_get(mm->sched_cache_grp));
> + sched_cache_footprint_sub(old, p);
Yes, it fixes the stale footprint issue for a vfork(). While looking at this fix I
realize that maybe even with this patch applied, we might still can not fix the losing
footprint, due to an existing race condition:
CPU0: execve() CPU1: CLONE_VM task
------------------------------ -------------------------------
task_numa_placement()
read footprint
sched_cache_exec_mmap()
read footprint
write footprint - own faults <-- the fix
write footprint + own diff
<-- overwrites the subtraction
So the footprint might still be lost because CPU1 overwrites it, and
there seems to be no lock protected against the race(I did not find one).
An enhanced way is to turn the raw footprint write into cmpxchg.
It reduces the race, and since numa balancing scan interval
numa_scan_period is not small, the overhead of using
atomic write is low, I'm thinking of something below(only compile tested):
diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index 57360f5cdde4..4123f0535cd9 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -1771,6 +1771,11 @@ void sched_cache_fork_cleanup(struct task_struct *p)
RCU_INIT_POINTER(p->sched_cache_grp, NULL);
}
+#ifdef CONFIG_NUMA_BALANCING
+static void sched_cache_footprint_update(struct sched_cache_group *grp,
+ unsigned long delta, bool add);
+#endif
+
void sched_cache_exec_mmap(struct task_struct *p, struct mm_struct *mm)
{
struct sched_cache_group *old;
@@ -1780,9 +1785,28 @@ void sched_cache_exec_mmap(struct task_struct *p, struct mm_struct *mm)
* the old one. @p is current and the only writer of its own pointer.
*/
old = sched_cache_replace_grp(p, sched_cache_group_get(mm->sched_cache_grp));
+#ifdef CONFIG_NUMA_BALANCING
+ if (old && p->total_numa_faults)
+ sched_cache_footprint_update(old, p->total_numa_faults, false);
+#endif
sched_cache_group_put(old);
}
+#ifdef CONFIG_NUMA_BALANCING
+static void sched_cache_footprint_update(struct sched_cache_group *grp,
+ unsigned long delta, bool add)
+{
+ unsigned long old = READ_ONCE(grp->footprint), new;
+
+ do {
+ if (add)
+ new = old + delta;
+ else
+ new = old - min(old, delta);
+ } while (!try_cmpxchg(&grp->footprint, &old, new));
+}
+#endif
+
void sched_cache_exit_mm(struct task_struct *p)
{
struct sched_cache_group *grp = sched_cache_replace_grp(p, NULL);
@@ -1793,12 +1817,8 @@ void sched_cache_exit_mm(struct task_struct *p)
* reference, so the group footprint converges as its threads exit.
* Unlocked for performance; clamp to avoid underflow.
*/
- if (grp && p->total_numa_faults) {
- unsigned long fp = READ_ONCE(grp->footprint);
- unsigned long sub = min(fp, p->total_numa_faults);
-
- WRITE_ONCE(grp->footprint, fp - sub);
- }
+ if (grp && p->total_numa_faults)
+ sched_cache_footprint_update(grp, p->total_numa_faults, false);
#endif
sched_cache_group_put(grp);
}
@@ -3890,7 +3910,7 @@ static void task_numa_placement(struct task_struct *p)
unsigned long total_faults;
u64 runtime, period;
spinlock_t *group_lock = NULL;
- long __maybe_unused new_fp;
+ long __maybe_unused fp_diff = 0;
struct numa_group *ng;
/*
@@ -3986,14 +4006,8 @@ static void task_numa_placement(struct task_struct *p)
* go negative. Clamp to zero to prevent the
* unsigned footprint from wrapping.
*/
- scoped_guard(rcu) {
- grp = rcu_dereference(p->sched_cache_grp);
-
- if (grp) {
- new_fp = (long)READ_ONCE(grp->footprint) + diff;
- WRITE_ONCE(grp->footprint, max(new_fp, 0L));
- }
- }
+ /* Update grp->footprint once, to limit the c2c cost. */
+ fp_diff += diff;
#endif
}
@@ -4008,6 +4022,18 @@ static void task_numa_placement(struct task_struct *p)
}
}
+#ifdef CONFIG_SCHED_CACHE
+ if (fp_diff) {
+ scoped_guard(rcu) {
+ grp = rcu_dereference(p->sched_cache_grp);
+
+ if (grp)
+ sched_cache_footprint_update(grp, abs(fp_diff),
+ fp_diff > 0);
+ }
+ }
+#endif
+
/* Cannot migrate task to CPU-less node */
max_nid = numa_nearest_node(max_nid, N_CPU);
--
2.25.1
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2026-10-10 15:19 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-10-10 11:16 [PATCH v2] sched/cache: Remove the old cache group footprint on exec Jemmy Wong
2026-10-10 15:04 ` Chen Yu
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®