Re: [PATCH v4 1/5] sched/debug: Protect lockless rq->rd access in print_dl_rq()

On Mon, Aug 10, 2026 at 03:53:38PM +0200, Daniel Vacek wrote:
> On Mon, 10 Aug 2026 at 03:58, Aaron Tomlin <[email protected]> wrote:
> > In print_dl_rq(), cpu_rq(cpu)->rd is dereferenced locklessly to display
> > deadline bandwidth statistics.
> >
> > During CPU hot-unplug or cgroup cpuset repartitioning events,
> > partition_sched_domains() calls cpu_attach_domain(), which executes
> > rq_attach_root() to detach the CPU from its root_domain. When the
> > reference count of the detached root_domain drops to zero,
> > rq_attach_root() calls call_rcu(&old_rd->rcu, free_rootdomain) to
> > schedule memory teardown after an RCU grace period.
> >
> > However, rq_attach_root() previously updated rq->rd using a plain C store
> > without an RCU publication barrier (i.e., rcu_assign_pointer()). Without a
> > release memory barrier on the writer side, CPU or compiler reordering could
> > allow the new rq->rd pointer store to become visible to other CPUs before
> > the initialization writes to rd->dl_bw are committed.
> >
> > Furthermore, because print_dl_rq() did not hold an RCU read lock while
> > dereferencing cpu_rq(cpu)->rd, an RCU grace period could elapse
> > concurrently while debugfs is reading the file, allowing
> > free_rootdomain() to execute kfree(old_rd) and causing a use-after-free
> > race condition when print_dl_rq() reads dl_bw->bw.
> >
> > Resolve this by using rcu_assign_pointer(rq->rd, rd) in rq_attach_root() to
> > guarantee a release memory barrier when publishing a root_domain.
> > Finally, fetch rq->rd using READ_ONCE() inside an RCU read-side critical
> > section in print_dl_rq().
> >
> > Fixes: 02968ccf7b80 ("sched: add /proc/sched_debug file")
> > Reported-by: sashiko-bot <[email protected]>
> > Signed-off-by: Aaron Tomlin <[email protected]>
> > ---
> >  kernel/sched/debug.c    | 12 +++++++++---
> >  kernel/sched/topology.c |  2 +-
> >  2 files changed, 10 insertions(+), 4 deletions(-)
> >
> > diff --git a/kernel/sched/debug.c b/kernel/sched/debug.c
> > index 40584b27ea0c..2c2156dfab00 100644
> > --- a/kernel/sched/debug.c
> > +++ b/kernel/sched/debug.c
> > @@ -1081,6 +1081,7 @@ void print_rt_rq(struct seq_file *m, int cpu, struct rt_rq *rt_rq)
> >  void print_dl_rq(struct seq_file *m, int cpu, struct dl_rq *dl_rq)
> >  {
> >         struct dl_bw *dl_bw;
> > +       struct root_domain *rd;
> >
> >         SEQ_printf(m, "\n");
> >         SEQ_printf(m, "dl_rq[%d]:\n", cpu);
> > @@ -1089,9 +1090,14 @@ void print_dl_rq(struct seq_file *m, int cpu, struct dl_rq *dl_rq)
> >         SEQ_printf(m, "  .%-30s: %lu\n", #x, (unsigned long)(dl_rq->x))
> >
> >         PU(dl_nr_running);
> > -       dl_bw = &cpu_rq(cpu)->rd->dl_bw;
> > -       SEQ_printf(m, "  .%-30s: %lld\n", "dl_bw->bw", dl_bw->bw);
> > -       SEQ_printf(m, "  .%-30s: %lld\n", "dl_bw->total_bw", dl_bw->total_bw);
> > +       rcu_read_lock();
> > +       rd = READ_ONCE(cpu_rq(cpu)->rd);
> > +       if (rd) {
> > +               dl_bw = &rd->dl_bw;
> > +               SEQ_printf(m, "  .%-30s: %lld\n", "dl_bw->bw", dl_bw->bw);
> > +               SEQ_printf(m, "  .%-30s: %lld\n", "dl_bw->total_bw", dl_bw->total_bw);
> > +       }
> > +       rcu_read_unlock();
> 
> Hey Aaron. How about simple
> 
> @@ -1089,7 +1089,8 @@ void print_dl_rq(struct seq_file *m, int cpu,
> struct dl_rq *dl_rq)
>         SEQ_printf(m, "  .%-30s: %lu\n", #x, (unsigned long)(dl_rq->x))
> 
>         PU(dl_nr_running);
> -       dl_bw = &cpu_rq(cpu)->rd->dl_bw;
> +       guard(rcu)();
> +       dl_bw = &rcu_dereference(cpu_rq(cpu)->rd)->dl_bw;
>         SEQ_printf(m, "  .%-30s: %lld\n", "dl_bw->bw", dl_bw->bw);
>         SEQ_printf(m, "  .%-30s: %lld\n", "dl_bw->total_bw", dl_bw->total_bw);

Hey Daniel,

Very elegant!

As mentioned here [1], since I introduced rcu_assign_pointer(rq->rd, rd) in
rq_attach_root(), I will now annotate 'rd' of struct rq with __rcu.

[1]: https://lore.kernel.org/lkml/7nn7bjzwovzu67ke6zlmmg4bv3utc6jv2sbzpklzumr3zt7csy@ip5icrcz6b35/


> >
> >  #undef PU
> >  }
> > diff --git a/kernel/sched/topology.c b/kernel/sched/topology.c
> > index 622e2e01974c..b411cc00029c 100644
> > --- a/kernel/sched/topology.c
> > +++ b/kernel/sched/topology.c
> > @@ -496,7 +496,7 @@ void rq_attach_root(struct rq *rq, struct root_domain *rd)
> >         }
> >
> >         atomic_inc(&rd->refcount);
> > -       rq->rd = rd;
> > +       rcu_assign_pointer(rq->rd, rd);
> >
> >         cpumask_set_cpu(rq->cpu, rd->span);
> >         if (cpumask_test_cpu(rq->cpu, cpu_active_mask))
> > --
> > 2.55.0
> >


Kind regards,
-- 
Aaron Tomlin