[PATCH v3 3/4] sched/fair: Use list_for_each_entry_rcu() in print_cfs_stats()

[Aaron Tomlin]

In print_cfs_stats(), rq->leaf_cfs_rq_list is traversed using
for_each_leaf_cfs_rq_safe(), which expands to list_for_each_entry_safe().
Although rq->leaf_cfs_rq_list is RCU-protected, list_for_each_entry_safe()
is a non-RCU iteration macro. It dereferences pointer links without
READ_ONCE() and pre-fetches the next pointer.

When a writer concurrently adds a new cfs_rq to the list using
list_add_rcu(), a reader traversing with list_for_each_entry_safe()
lacks READ_ONCE() protection. Without READ_ONCE(), the compiler is free
to re-fetch pointers or reorder instructions. As a result, the reader
can observe a newly inserted cfs_rq's pointer before its internal fields
are fully visible, leading to reading uninitialised data or
dereferencing invalid pointers. This is illustrated below:

           CPU 0                                        CPU 1
    Writer (under rq->lock)                      Reader (print_cfs_stats)
   ---------------------------                  --------------------------
    1. Initialise cfs_rq                         rcu_read_lock()
         cfs_rq->tg = tg
         cfs_rq->load = 1024
    2. list_add_rcu(&cfs_rq->list, ...):         3. for_each_leaf_cfs_rq_safe():
         smp_store_release()                          Reads prev->next (cfs_rq)
         prev->next = cfs_rq           ----->         Missing READ_ONCE()
                                                 4. Reads cfs_rq fields; sees
                                                    garbage data
                                                      cfs_rq->tg
                                                      cfs_rq->load

Fix this by introducing for_each_leaf_cfs_rq_rcu(), which expands to
list_for_each_entry_rcu(). This uses READ_ONCE() during list traversal.

Fixes: 0601267ca673 ("sched/fair: Rewrite list_add_leaf_cfs_rq()")
Reported-by: sashiko-bot <sashiko-bot@xxxxxxxxxx>
Signed-off-by: Aaron Tomlin <atomlin@xxxxxxxxxxx>
---
 kernel/sched/fair.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index d78467ec6ee1..25f941ad637a 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -412,6 +412,10 @@ static inline void assert_list_leaf_cfs_rq(struct rq *rq)
 	list_for_each_entry_safe(cfs_rq, pos, &rq->leaf_cfs_rq_list,	\
 				 leaf_cfs_rq_list)
 
+#define for_each_leaf_cfs_rq_rcu(rq, cfs_rq)				\
+	list_for_each_entry_rcu(cfs_rq, &(rq)->leaf_cfs_rq_list,	\
+				leaf_cfs_rq_list)
+
 /* Do the two (enqueued) entities belong to the same group ? */
 static inline struct cfs_rq *
 is_same_group(struct sched_entity *se, struct sched_entity *pse)
@@ -497,6 +501,9 @@ static inline void assert_list_leaf_cfs_rq(struct rq *rq)
 #define for_each_leaf_cfs_rq_safe(rq, cfs_rq, pos)	\
 		for (cfs_rq = &rq->cfs, pos = NULL; cfs_rq; cfs_rq = pos)
 
+#define for_each_leaf_cfs_rq_rcu(rq, cfs_rq)	\
+		for (cfs_rq = &rq->cfs; cfs_rq; cfs_rq = NULL)
+
 static inline struct sched_entity *parent_entity(struct sched_entity *se)
 {
 	return NULL;
@@ -15401,10 +15408,10 @@ DEFINE_SCHED_CLASS(fair) = {
 
 void print_cfs_stats(struct seq_file *m, int cpu)
 {
-	struct cfs_rq *cfs_rq, *pos;
+	struct cfs_rq *cfs_rq;
 
 	rcu_read_lock();
-	for_each_leaf_cfs_rq_safe(cpu_rq(cpu), cfs_rq, pos)
+	for_each_leaf_cfs_rq_rcu(cpu_rq(cpu), cfs_rq)
 		print_cfs_rq(m, cpu, cfs_rq);
 	rcu_read_unlock();
 }
-- 
2.55.0