[PATCH v2] srcu: Fix WARN_ON() for rcu_segcblist_n_cbs() in cleanup_srcu_struct()

[Sunho Park]

The WARN_ON() added by commit 78a38cbf6f20 ("srcu: Queue sdp->work
when the delay timer is successfully deleted") uses rcu_segcblist_n_cbs()
to detect callbacks that srcu_barrier() failed to wait for. However, the
->len counter is decremented only at the end of srcu_invoke_callbacks(),
after the invoking loop has finished. Since srcu_barrier() can return
right after the barrier callback is invoked, cleanup_srcu_struct() can see
a non-zero n_cbs even though the cblist is already physically empty,
falsely triggering the WARN_ON() together with a still-pending delay_work
timer.

This can be triggered as follows, as seen in the syzbot report against
kvm_destroy_vm() -> cleanup_srcu_struct(&kvm->srcu):

1. call_srcu(&kvm->srcu, &bus->rcu, __free_bus) starts SRCU grace
   period GP1.

2. GP1 ends: a delay timer is armed and sdp->work is queued, but
   sdp->work has not run yet.

3. Another call_srcu(&kvm->srcu, &bus->rcu, __free_bus) call invokes
   srcu_segcblist_advance(), which moves the GP1 callback to
   RCU_DONE_TAIL, and starts SRCU grace period GP2.

4. srcu_barrier() is called. It queues its barrier callback after the
   GP2 callback and waits for srcu_invoke_callbacks() to invoke it.

5. GP2 ends: another delay timer is armed, and the sdp->work queued in
   step 2 begins to run. Its srcu_invoke_callbacks() call invokes
   srcu_segcblist_advance() again, moving the GP2 and barrier callbacks
   to RCU_DONE_TAIL as well, and then invokes all of them. However,
   rcu_segcblist_add_len(), which updates srcu_cblist's ->len, has not
   run yet at this point.

6. srcu_barrier() returns once its callback has been invoked, and
   cleanup_srcu_struct() starts running. It finds the delay timer
   armed in step 5 still pending and srcu_cblist's ->len still
   non-zero (because step 5 has not reached rcu_segcblist_add_len()
   yet), and WARN_ON() fires even though every callback has actually
   been invoked.

Use rcu_segcblist_empty(), which checks the actual head of the cblist,
instead of rcu_segcblist_n_cbs(), which checks the racy ->len counter.
Callbacks that have genuinely not been invoked yet still leave the list
non-empty, so the WARN_ON() still catches callers that skip srcu_barrier()
or queue callbacks after it.

Link: https://lore.kernel.org/rcu/e6350377085ddd85d6ef00d8e9a67bd50c762d3c@xxxxxxxxx/T/#t
Reported-by: syzbot+d4faf7db59e11f6fd1ab@xxxxxxxxxxxxxxxxxxxxxxxxx
Closes: https://syzkaller.appspot.com/bug?extid=d4faf7db59e11f6fd1ab
Fixes: 78a38cbf6f20 ("srcu: Queue sdp->work when the delay timer is successfully deleted")
Suggested-by: Zqiang <qiang.zhang@xxxxxxxxx>
Signed-off-by: Sunho Park <shpark061104@xxxxxxxxx>
---
v2: Describe the exact interleaving of call_srcu()/srcu_barrier()/
    srcu_invoke_callbacks() that triggers the false-positive WARN_ON(),
    per Zqiang. No code change from v1.
Link: https://lore.kernel.org/rcu/1b9d3ddaa26ba6a8ac2ad4c586c2d19a5c1f927e@xxxxxxxxx/T/#t
---

 kernel/rcu/srcutree.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/rcu/srcutree.c b/kernel/rcu/srcutree.c
index ed204b3f4b84..ad27880dd690 100644
--- a/kernel/rcu/srcutree.c
+++ b/kernel/rcu/srcutree.c
@@ -704,7 +704,7 @@ void cleanup_srcu_struct(struct srcu_struct *ssp)
 		// Call srcu_barrier() before this cleanup_srcu_struct()
 		// to avoid triggering this WARN_ON().
 		if (WARN_ON(timer_delete_sync(&sdp->delay_work) &&
-			    rcu_segcblist_n_cbs(&sdp->srcu_cblist)) &&
+			    !rcu_segcblist_empty(&sdp->srcu_cblist)) &&
 		    rcu_cpu_beenfullyonline(sdp->cpu))
 			queue_work_on(sdp->cpu, rcu_gp_wq, &sdp->work);
 		flush_work(&sdp->work);
-- 
2.43.0