Re: [PATCH ath-next] wifi: ath9k: use rcu_dereference_bh() for sta->rates in ath_merge_ratetbl()

[Toke Høiland-Jørgensen]

Jeff Johnson <jeff.johnson@xxxxxxxxxxxxxxxx> writes:

> Commit c4f518736472 ("wifi: ath9k: Remove redundant rcu_read_lock/unlock()
> in spin_lock") removed the explicit rcu_read_lock()/rcu_read_unlock() pair
> from ath_txq_schedule(), relying on spin_lock_bh() to provide an implicit
> RCU read-side critical section.
>
> That is correct on kernels without CONFIG_DEBUG_LOCK_ALLOC, where
> rcu_read_lock_any_held() falls back to !preemptible() as a proxy, and
> spin_lock_bh() disables preemption.
>
> However, rcu_dereference() in ath_merge_ratetbl() checks
> rcu_read_lock_held(), which under CONFIG_DEBUG_LOCK_ALLOC only returns
> true when lock_is_held(&rcu_lock_map) — set exclusively by an explicit
> rcu_read_lock().  A spin_lock_bh() does not set rcu_lock_map, so on a
> CONFIG_DEBUG_LOCK_ALLOC + CONFIG_PROVE_RCU kernel, any call path that
> reaches ath_merge_ratetbl() with a non-NULL sta will produce a lockdep
> splat.
>
> The correct fix is to use rcu_dereference_bh(), whose validity check
> calls rcu_read_lock_bh_held(), which returns true whenever BH is disabled
> (in_softirq() || irqs_disabled()).  This matches the actual protection
> at all callers of ath_set_rates() that pass a non-NULL sta: they all hold
> a spin_lock_bh() (either sc->chan_lock or txq->axq_lock).
>
> Fixes: c4f518736472 ("wifi: ath9k: Remove redundant rcu_read_lock/unlock() in spin_lock")
> Assisted-by: Claude:claude-sonnet-4-6
> Signed-off-by: Jeff Johnson <jeff.johnson@xxxxxxxxxxxxxxxx>

Acked-by: Toke Høiland-Jørgensen <toke@xxxxxxx>