[PATCH v2] sched/cache: Remove the old cache group footprint on exec

[PATCH v2] sched/cache: Remove the old cache group footprint on exec

[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)

Re: [PATCH v2] sched/cache: Remove the old cache group footprint on exec

[Chen Yu]

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

Re: [PATCH v2] sched/cache: Remove the old cache group footprint on exec

[Jemmy Wong]

Hi Chenyu,

Thanks for the review and for spotting the race.

> On Oct 10, 2026, at 11:04 PM, Chen Yu <yu.c.chen@intel.com> wrote:
> 
> 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).

Right, and there is no lock. grp->footprint is only ever written by plain
READ_ONCE()/WRITE_ONCE() in task_numa_placement(), exit and (with v2) exec.

> 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):

This looks right to me, including applying the placement diff once.
I've taken it into my local tree with a few tweaks, explained
inline below. Feel free to push back on any of them.

> 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);
> }

I folded it in without the forward declaration by ordering the helpers ahead
of the callers. The condition lives in sched_cache_footprint_drop(), with an
empty stub when CONFIG_NUMA_BALANCING is off, so exec and exit share one call:

	old = sched_cache_replace_grp(p, sched_cache_group_get(mm->sched_cache_grp));
	sched_cache_footprint_drop(old, p);
	sched_cache_group_put(old);

Moving the check into the helper costs no call: I confirmed in the
generated code that sched_cache_footprint_drop() is inlined into both
exec and exit, so the early-out test still sits in the caller.

The "must stay ahead of task_numa_free()" note moved onto that helper.
Nothing enforces the order; sched_cache_exec_mmap() just remains before
task_numa_free(current, false) in bprm_execve().

> +#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

#ifdef CONFIG_NUMA_BALANCING
static void sched_cache_footprint_update(struct sched_cache_group *grp, long diff)
{
	unsigned long old = READ_ONCE(grp->footprint), new;

	do {
		if (diff < 0)
			new = old - min(old, -(unsigned long)diff);
		else
			new = old + diff;
	} while (!try_cmpxchg_relaxed(&grp->footprint, &old, new));
}

static void sched_cache_footprint_drop(struct sched_cache_group *grp,
				     struct task_struct *p)
{
	if (grp && p->total_numa_faults)
		sched_cache_footprint_update(grp, -(long)p->total_numa_faults);
}
#else
static inline void sched_cache_footprint_drop(struct sched_cache_group *grp,
					    struct task_struct *p) { }
#endif

Two changes in that helper:
- try_cmpxchg_relaxed() rather than try_cmpxchg().
  try_cmpxchg() is fully ordered on success; on arm64 with LSE that
  is casal. Its release half keeps the CAS from being observed before
  any earlier access from this CPU, and its acquire half keeps later
  accesses from being observed before the CAS. In exec_mmap() those
  earlier accesses include e.g. the tsk->mm and tsk->active_mm stores
  just above, which need no ordering against grp->footprint.

- Signed @diff rather than (delta, bool add), so the sign is not split
  across an abs()/">0" pair at the call site. Subtraction still clamps,
  with the negate done in unsigned arithmetic to keep the clamp in one
  type.

One question about the clamp: shouldn't atomic updates balance each
task's placement diffs against its exit/exec subtraction? Assuming no
overflow, old < delta would suggest an accounting bug that clamping
hides. Is there a legitimate path where the contributions do not balance?

If not, we could use atomic_long_t and atomic_long_add(diff,
&grp->footprint) instead of the CAS loop, provided the footprint fits
in a signed long. atomic_long_add() is already unordered; readers would
use atomic_long_read().

> 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);
> + }

grp is declared inside the guard, and the signed diff is passed through:

	if (fp_diff) {
		scoped_guard(rcu) {
			struct sched_cache_group *grp;

			grp = rcu_dereference(p->sched_cache_grp);
			if (grp)
				sched_cache_footprint_update(grp, fp_diff);
		}
	}

> + }
> +#endif
> +
> /* Cannot migrate task to CPU-less node */
> max_nid = numa_nearest_node(max_nid, N_CPU);
> 
> -- 
> 2.25.1

Thanks,
Jemmy