[tip: sched/urgent] sched/fair: Clear ->h_load_next when unregistering a cgroup

[tip: sched/urgent] sched/fair: Clear ->h_load_next when unregistering a cgroup

[tip-bot2 for Peng Wang]

The following commit has been merged into the sched/urgent branch of tip:

Commit-ID:     f85fd15b0da76f08e2d5bb4179c54993f7e07853
Gitweb:        https://git.kernel.org/tip/f85fd15b0da76f08e2d5bb4179c54993f7e07853
Author:        Peng Wang <peng_wang@linux.alibaba.com>
AuthorDate:    Fri, 24 Oct 2025 15:23:16 +08:00
Committer:     Ingo Molnar <mingo@kernel.org>
CommitterDate: Wed, 03 Dec 2025 17:44:01 +01:00

sched/fair: Clear ->h_load_next when unregistering a cgroup

An invalid pointer dereference bug was reported on ARM64 CPUs, and has
not yet been seen on x86. A partial oops looks like:

 Call trace:
  update_cfs_rq_h_load+0x80/0xb0
  wake_affine+0x158/0x168
  select_task_rq_fair+0x364/0x3a8
  try_to_wake_up+0x154/0x648
  wake_up_q+0x68/0xd0
  futex_wake_op+0x280/0x4c8
  do_futex+0x198/0x1c0
  __arm64_sys_futex+0x11c/0x198

See: https://lore.kernel.org/all/20251013071820.1531295-1-CruzZhao@linux.alibaba.com/

We found that the task_group corresponding to the problematic se
is not in the parent task_group’s children list, indicating that
h_load_next points to an invalid address. Consider the following
cgroup and task hierarchy:

         A
        / \
       /   \
      B     E
     / \    |
    /   \   t2
   C     D
   |     |
   t0    t1

Here follows a timing sequence that may be responsible for triggering
the problem:

  CPU X                   CPU Y                   CPU Z
  wakeup t0
  set list A->B->C
  traverse A->B->C
  t0 exits
  destroy C
                          wakeup t2
                          set list A->E           wakeup t1
                                                  set list A->B->D
                          traverse A->B->C
                          panic

CPU Z sets ->h_load_next list to A->B->D, but due to weaker memory
ordering on ARM64, Y may observe A->B before it sees B->D, then in this
time window, it can traverse A->B->C and reach an invalid se.

We can avoid stale pointer accesses by clearing ->h_load_next when
unregistering a cgroup.

Fixes: 685207963be9 ("sched: Move h_load calculation to task_h_load()")
Suggested-by: Vincent Guittot <vincent.guittot@linaro.org>
Co-developed-by: Cruz Zhao <CruzZhao@linux.alibaba.com>
Signed-off-by: Cruz Zhao <CruzZhao@linux.alibaba.com>
Signed-off-by: Peng Wang <peng_wang@linux.alibaba.com>
Signed-off-by: Ingo Molnar <mingo@kernel.org>
Reviewed-by: Vincent Guittot <vincent.guittot@linaro.org>
Link: https://patch.msgid.link/bf93d41ff9f2da19ef2c1cfb505362e0b48c39de.1761290330.git.peng_wang@linux.alibaba.com
---
 kernel/sched/fair.c |  9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index 769d7b7..00a32c9 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -13687,6 +13687,8 @@ void unregister_fair_sched_group(struct task_group *tg)
 		struct rq *rq = cpu_rq(cpu);
 
 		if (se) {
+			struct cfs_rq *parent_cfs_rq = cfs_rq_of(se);
+
 			if (se->sched_delayed) {
 				guard(rq_lock_irqsave)(rq);
 				if (se->sched_delayed) {
@@ -13696,6 +13698,13 @@ void unregister_fair_sched_group(struct task_group *tg)
 				list_del_leaf_cfs_rq(cfs_rq);
 			}
 			remove_entity_load_avg(se);
+
+			/*
+			 * Clear parent's h_load_next if it points to the
+			 * sched_entity being freed, to avoid stale pointer.
+			 */
+			if (READ_ONCE(parent_cfs_rq->h_load_next) == se)
+				WRITE_ONCE(parent_cfs_rq->h_load_next, NULL);
 		}
 
 		/*

Re: [tip:sched/urgent] sched/fair: Clear ->h_load_next when unregistering a cgroup

[Cruz Zhao]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=Y, Size: 415 bytes --]

Hi Ingo/Peter/all,

I noticed that the following patch has been queued in the
tip:sched/urgent branch for some time but hasn't yet made
it into mainline: 
https://lore.kernel.org/all/176478073513.498.15089394378873483436.tip-bot2@tip-bot2/

Could you please check if there's anything blocking its
merge? I wanted to ensure it doesn’t get overlooked.

Thanks for your time and reviewing! 

Best regards,
Cruz Zhao

Re: [tip:sched/urgent] sched/fair: Clear ->h_load_next when unregistering a cgroup

[Vincent Guittot]

Hi Cruz,

On Mon, 29 Dec 2025 at 13:51, Cruz Zhao <CruzZhao@linux.alibaba.com> wrote:
>
> Hi Ingo/Peter/all,
>
> I noticed that the following patch has been queued in the
> tip:sched/urgent branch for some time but hasn't yet made
> it into mainline:
> https://lore.kernel.org/all/176478073513.498.15089394378873483436.tip-bot2@tip-bot2/
>
> Could you please check if there's anything blocking its
> merge? I wanted to ensure it doesn’t get overlooked.

From an off list discussion w/ Peter, we need to check that this patch
is not hiding the root cause that task_h_load is not called in the
right context i.e. with rcu_read_lock(). Peter pointed out one place
in numa [1]

[1] https://lore.kernel.org/all/20251015124422.GD3419281@noisy.programming.kicks-ass.net/

Vincent

>
> Thanks for your time and reviewing!
>
> Best regards,
> Cruz Zhao

Re: [tip:sched/urgent] sched/fair: Clear ->h_load_next when unregistering a cgroup

[Krister Johansen]

Hi Vincent,

On Mon, Dec 29, 2025 at 02:58:16PM +0100, Vincent Guittot wrote:
> On Mon, 29 Dec 2025 at 13:51, Cruz Zhao <CruzZhao@linux.alibaba.com> wrote:
> > I noticed that the following patch has been queued in the
> > tip:sched/urgent branch for some time but hasn't yet made
> > it into mainline:
> > https://lore.kernel.org/all/176478073513.498.15089394378873483436.tip-bot2@tip-bot2/
> >
> > Could you please check if there's anything blocking its
> > merge? I wanted to ensure it doesn’t get overlooked.
> 
> From an off list discussion w/ Peter, we need to check that this patch
> is not hiding the root cause that task_h_load is not called in the
> right context i.e. with rcu_read_lock(). Peter pointed out one place
> in numa [1]
> 
> [1] https://lore.kernel.org/all/20251015124422.GD3419281@noisy.programming.kicks-ass.net/

If it helps, I've double-checked this code a few times.  When I looked,
there were 7 different callers of task_h_load(), and they decompose into
3 cases.

1. rcu_read_lock is held as we expect
2. the numa balancing cases Peter already identified
3. tick related invocations, where the caller is in interrupt context

For 3, there's an edge case where deferred work is scheduled if the
target cpu is in full nohz mode and has stopped.

In the cases where I'm hitting this bug, the systems aren't using numa
balancing and aren't using nohz. 90% of ones I've analyzed are in a
futex wakeup and are holding the rcu_read_lock.

This seems like just a case of the pointer continuing to reference
memory that was already free'd.  If the task group's sched entity is
freed, but the parent cfs_rq still has a pointer to that sched_entity in
h_load_next, then it may end up accessing that memory accidentally if we
do not clear it.

Put another way, even if all of these callers used rcu_read_lock, there
would still be a need to ensure that the parent's h_load_next doesn't
point to a sched entity that is free'd once the RCU read-side critical
section is exited, because the child is getting free'd and not the
parent.  The (freed) child is still discoverable from the parent's
h_load_next after the critical section because the delete code does not
clear h_load_next and order that write before the free.

-K

Re: [tip:sched/urgent] sched/fair: Clear ->h_load_next when unregistering a cgroup

[Vincent Guittot]

On Sat, 3 Jan 2026 at 02:47, Krister Johansen <kjlx@templeofstupid.com> wrote:
>
> Hi Vincent,
>
> On Mon, Dec 29, 2025 at 02:58:16PM +0100, Vincent Guittot wrote:
> > On Mon, 29 Dec 2025 at 13:51, Cruz Zhao <CruzZhao@linux.alibaba.com> wrote:
> > > I noticed that the following patch has been queued in the
> > > tip:sched/urgent branch for some time but hasn't yet made
> > > it into mainline:
> > > https://lore.kernel.org/all/176478073513.498.15089394378873483436.tip-bot2@tip-bot2/
> > >
> > > Could you please check if there's anything blocking its
> > > merge? I wanted to ensure it doesn’t get overlooked.
> >
> > From an off list discussion w/ Peter, we need to check that this patch
> > is not hiding the root cause that task_h_load is not called in the
> > right context i.e. with rcu_read_lock(). Peter pointed out one place
> > in numa [1]
> >
> > [1] https://lore.kernel.org/all/20251015124422.GD3419281@noisy.programming.kicks-ass.net/
>
> If it helps, I've double-checked this code a few times.  When I looked,
> there were 7 different callers of task_h_load(), and they decompose into
> 3 cases.
>
> 1. rcu_read_lock is held as we expect
> 2. the numa balancing cases Peter already identified
> 3. tick related invocations, where the caller is in interrupt context
>
> For 3, there's an edge case where deferred work is scheduled if the
> target cpu is in full nohz mode and has stopped.

Thanks for this analysis. I'm aligned with your conclusion that we
have 2 calls which are not protected
-task_numa_find_cpu
-sched_tick_remote

>
> In the cases where I'm hitting this bug, the systems aren't using numa
> balancing and aren't using nohz. 90% of ones I've analyzed are in a
> futex wakeup and are holding the rcu_read_lock.

Do you have a simple way to reproduce it ?

>
> This seems like just a case of the pointer continuing to reference
> memory that was already free'd.  If the task group's sched entity is
> freed, but the parent cfs_rq still has a pointer to that sched_entity in
> h_load_next, then it may end up accessing that memory accidentally if we
> do not clear it.

I agree that rcu protection does not prevent cfs_rq->h_load_next from
holding a ref to the freed sched_entity after the grace period and
until update_cfs_rq_h_load() overwrites it with another child. I just
wonder how we can really end up with
 traverse A->B->C because we do set B->D then set A->B when doing set
list A->B->D

Vincent




>
> Put another way, even if all of these callers used rcu_read_lock, there
> would still be a need to ensure that the parent's h_load_next doesn't
> point to a sched entity that is free'd once the RCU read-side critical
> section is exited, because the child is getting free'd and not the
> parent.  The (freed) child is still discoverable from the parent's
> h_load_next after the critical section because the delete code does not
> clear h_load_next and order that write before the free.
>
> -K

Re: [tip:sched/urgent] sched/fair: Clear ->h_load_next when unregistering a cgroup

[Krister Johansen]

Hi Vincent,
Thanks for the response.  I sat down to write you an e-mail about why
this was the right thing to do, but after working through the problem
now I'm less certain.

On Mon, Jan 12, 2026 at 06:32:44PM +0100, Vincent Guittot wrote:
> On Sat, 3 Jan 2026 at 02:47, Krister Johansen <kjlx@templeofstupid.com> wrote:
> > In the cases where I'm hitting this bug, the systems aren't using numa
> > balancing and aren't using nohz. 90% of ones I've analyzed are in a
> > futex wakeup and are holding the rcu_read_lock.
> 
> Do you have a simple way to reproduce it ?

I haven't come up with a minimal reproducer yet, but will send something
along if I can find a way to do this.

I do have lots of kdumps of this happening so if there's something you'd
like me to look for there, I'd be happy to do some additional digging.

The team that's running into this is only hitting it in a production
environment and so far has not been willing to let me drop in any kind
of test kernel.

> > This seems like just a case of the pointer continuing to reference
> > memory that was already free'd.  If the task group's sched entity is
> > freed, but the parent cfs_rq still has a pointer to that sched_entity in
> > h_load_next, then it may end up accessing that memory accidentally if we
> > do not clear it.
> 
> I agree that rcu protection does not prevent cfs_rq->h_load_next from
> holding a ref to the freed sched_entity after the grace period and
> until update_cfs_rq_h_load() overwrites it with another child. I just
> wonder how we can really end up with
>  traverse A->B->C because we do set B->D then set A->B when doing set
> list A->B->D

I spent some additional time analyzing a kdump that was collected on
the 12th.  The crash was caused by a write to read-only area of kernel
memory. The address turned out to be inside of css_free_rwork_fn.

The panic task was on CPU 4 trying to wake another thread in its process
that was sleeping in on futex, which last ran on CPU 2.  Another task was
already running on CPU 2.  The futex wake on CPU 4 didn't get far enough
to make a migration decision.

There are 8 cpus on this machine, 5 idle.  Cpu 0 running a metrics
collector, cpu 2 running the workload, and cpu 4 running a security
agent.  (Other tasks also trigger this panic, so I don't suspect any
particular one as the culprit).  The cgroup hierarchy on this machine
has enough nodes that a multi-node swap is possible.

I don't have details on whether other tasks that last ran on CPU 2 have
been awoken and relocated.

After the crash, the kdump does not show h_load_next list walks as
containing any errant pointers.  I manually checked these in crash, and
then when paranoia got the better of me, I wrote a drgn script that
walked up the se->parent pointers and back down the cfs_rq->h_load_next
pointers like task_h_load does, just looking to see if anything with an
address near the bad pointer was reachable.  No hits.

To fallback to the diagram from the patch:

               A
              / \
             /   \
            B     E
           / \    |
          /   \   t2
         C     D
         |     |
         t0    t1
   
C is presumed to be the dead and deleted leaf cgroup and t0 has exited.

I think it would have been helpful if the original submitters were more
concrete about which weak ordering rules were playing a role in the
problem. I had to look at [1],[2],[3] to get some sense of what the
rules are.  The writes issued by a thread may be observed in a different
order by other threads, but for writes to the same address all threads
must observe the writes in the same order and they must be visible at
the same time.

Without barriers remote cpus may see the writes in a different order
than the cpu that issued these writes, but they all must have the same
order when they're visible to the remote cpus.

With this in mind, the wakeup on CPU 4 could find the h_load_next list
in the following order:

    A -> B -> C

And issue stores:

   e->h_load_next = NULL
   a->h_load_next = e

The disassembly shows load/store dependencies for the executing thread,
but depending on how those dependencies are resolved the writes might
not be visibile to other threads in the same order.  Consequently, if
there's another waker it could issue:

  d->h_load_next = NULL
  b->h_load_next = d
  a->h_load_next = b

and there's at least a possibility of the cpu handling the wakeup of t2
seeing the store of b to a->h_load_next in an order that is before b ->
d, possibly because the dependent loads for a->h_load_next were cached
and could be retired sooner. For this to happen the stores have to be
after the a->e store, but the subsequent a->b store has to be visible
sooner and seen by cpu 4's subsequent load.

I assumed that part of the reason why this patch worked was that it's
implicitly depending on the write barrier in the rq_lock_irqsave that's
in unregister_fair_sched_group.  On 5.10, where the author originally
encountered this, the spin lock is taken unconditionally on each cpu in
turn. In 6.19, this is only taken for the se->sched_delayed and
cfs_rq->on_list cases.  I'm curious if the authors managed to reproduce
the problem on 6.19 as part of validating the fix.

[1] https://www.cl.cam.ac.uk/~pes20/armv8-mca/armv8-mca-draft.pdf
[2] https://www.cl.cam.ac.uk/~pes20/armv8-mca/full_flat.pdf
[3] https://www.cl.cam.ac.uk/~pes20/ppc-supplemental/test7.pdf

-K