| From c9a1294703890957c31dd28a06194bb29e39e92d Mon Sep 17 00:00:00 2001 |
| From: Sasha Levin <sashal@kernel.org> |
| Date: Mon, 5 Apr 2021 09:51:05 -0700 |
| Subject: rcu: Reject RCU_LOCKDEP_WARN() false positives |
| |
| From: Paul E. McKenney <paulmck@kernel.org> |
| |
| [ Upstream commit 3066820034b5dd4e89bd74a7739c51c2d6f5e554 ] |
| |
| If another lockdep report runs concurrently with an RCU lockdep report |
| from RCU_LOCKDEP_WARN(), the following sequence of events can occur: |
| |
| 1. debug_lockdep_rcu_enabled() sees that lockdep is enabled |
| when called from (say) synchronize_rcu(). |
| |
| 2. Lockdep is disabled by a concurrent lockdep report. |
| |
| 3. debug_lockdep_rcu_enabled() evaluates its lockdep-expression |
| argument, for example, lock_is_held(&rcu_bh_lock_map). |
| |
| 4. Because lockdep is now disabled, lock_is_held() plays it safe and |
| returns the constant 1. |
| |
| 5. But in this case, the constant 1 is not safe, because invoking |
| synchronize_rcu() under rcu_read_lock_bh() is disallowed. |
| |
| 6. debug_lockdep_rcu_enabled() wrongly invokes lockdep_rcu_suspicious(), |
| resulting in a false-positive splat. |
| |
| This commit therefore changes RCU_LOCKDEP_WARN() to check |
| debug_lockdep_rcu_enabled() after checking the lockdep expression, |
| so that any "safe" returns from lock_is_held() are rejected by |
| debug_lockdep_rcu_enabled(). This requires memory ordering, which is |
| supplied by READ_ONCE(debug_locks). The resulting volatile accesses |
| prevent the compiler from reordering and the fact that only one variable |
| is being accessed prevents the underlying hardware from reordering. |
| The combination works for IA64, which can reorder reads to the same |
| location, but this is defeated by the volatile accesses, which compile |
| to load instructions that provide ordering. |
| |
| Reported-by: syzbot+dde0cc33951735441301@syzkaller.appspotmail.com |
| Reported-by: Matthew Wilcox <willy@infradead.org> |
| Reported-by: syzbot+88e4f02896967fe1ab0d@syzkaller.appspotmail.com |
| Reported-by: Thomas Gleixner <tglx@linutronix.de> |
| Suggested-by: Boqun Feng <boqun.feng@gmail.com> |
| Reviewed-by: Boqun Feng <boqun.feng@gmail.com> |
| Signed-off-by: Paul E. McKenney <paulmck@kernel.org> |
| Signed-off-by: Sasha Levin <sashal@kernel.org> |
| --- |
| include/linux/rcupdate.h | 2 +- |
| kernel/rcu/update.c | 2 +- |
| 2 files changed, 2 insertions(+), 2 deletions(-) |
| |
| diff --git a/include/linux/rcupdate.h b/include/linux/rcupdate.h |
| index bd04f722714f..d11bee5d9347 100644 |
| --- a/include/linux/rcupdate.h |
| +++ b/include/linux/rcupdate.h |
| @@ -315,7 +315,7 @@ static inline int rcu_read_lock_any_held(void) |
| #define RCU_LOCKDEP_WARN(c, s) \ |
| do { \ |
| static bool __section(".data.unlikely") __warned; \ |
| - if (debug_lockdep_rcu_enabled() && !__warned && (c)) { \ |
| + if ((c) && debug_lockdep_rcu_enabled() && !__warned) { \ |
| __warned = true; \ |
| lockdep_rcu_suspicious(__FILE__, __LINE__, s); \ |
| } \ |
| diff --git a/kernel/rcu/update.c b/kernel/rcu/update.c |
| index b95ae86c40a7..dd94a602a6d2 100644 |
| --- a/kernel/rcu/update.c |
| +++ b/kernel/rcu/update.c |
| @@ -277,7 +277,7 @@ EXPORT_SYMBOL_GPL(rcu_callback_map); |
| |
| noinstr int notrace debug_lockdep_rcu_enabled(void) |
| { |
| - return rcu_scheduler_active != RCU_SCHEDULER_INACTIVE && debug_locks && |
| + return rcu_scheduler_active != RCU_SCHEDULER_INACTIVE && READ_ONCE(debug_locks) && |
| current->lockdep_recursion == 0; |
| } |
| EXPORT_SYMBOL_GPL(debug_lockdep_rcu_enabled); |
| -- |
| 2.30.2 |
| |