wifi: ath11k: fix event locking

The ath11k active pdevs are protected by RCU but the temperature event handling code calling ath11k_mac_get_ar_by_pdev_id() was not marked as a read-side critical section as reported by RCU lockdep:

	=============================
	WARNING: suspicious RCU usage
	6.6.0-rc6 #7 Not tainted
	-----------------------------
	drivers/net/wireless/ath/ath11k/mac.c:638 suspicious rcu_dereference_check() usage!

	other info that might help us debug this:

	rcu_scheduler_active = 2, debug_locks = 1
	no locks held by swapper/0/0.
	...
	Call trace:
	...
	 lockdep_rcu_suspicious+0x16c/0x22c
	 ath11k_mac_get_ar_by_pdev_id+0x194/0x1b0 [ath11k]
	 ath11k_wmi_tlv_op_rx+0xa84/0x2c1c [ath11k]
	 ath11k_htc_rx_completion_handler+0x388/0x510 [ath11k]

Mark the code in question as an RCU read-side critical section to avoid any potential use-after-free issues.

Fixes a41d10348b01 ("ath11k: add thermal sensor device support")

Cc [email protected] # 5.7

Signed-off-by Johan Hovold <[email protected]>

The ath11k active pdevs are protected by RCU but the temperature event handling code calling ath11k_mac_get_ar_by_pdev_id() was not marked as a read-side critical section as reported by RCU lockdep:

	=============================
	WARNING: suspicious RCU usage
	6.6.0-rc6 #7 Not tainted
	-----------------------------
	drivers/net/wireless/ath/ath11k/mac.c:638 suspicious rcu_dereference_check() usage!

	other info that might help us debug this:

	rcu_scheduler_active = 2, debug_locks = 1
	no locks held by swapper/0/0.
	...
	Call trace:
	...
	 lockdep_rcu_suspicious+0x16c/0x22c
	 ath11k_mac_get_ar_by_pdev_id+0x194/0x1b0 [ath11k]
	 ath11k_wmi_tlv_op_rx+0xa84/0x2c1c [ath11k]
	 ath11k_htc_rx_completion_handler+0x388/0x510 [ath11k]

Mark the code in question as an RCU read-side critical section to avoid any potential use-after-free issues.

Fixes a41d10348b01 ("ath11k: add thermal sensor device support")

Cc [email protected] # 5.7

Signed-off-by Johan Hovold <[email protected]>

Quoted 3 lines · Johan Hovold

The ath11k active pdevs are protected by RCU but the temperature event handling code calling ath11k_mac_get_ar_by_pdev_id() was not marked as a read-side critical section as reported by RCU lockdep:

On what hardware and firmware version did you test this? As there's so many different combos we use Tested-on tag to provide that information in the commit message:

https://wireless.wiki.kernel.org/en/users/drivers/ath11k/submittingpatches#tested-on_tag

I can add that if you let me know what you used.

Quoted 10 lines · Kalle Valo

Quoted 3 lines · Johan Hovold

The ath11k active pdevs are protected by RCU but the temperature event handling code calling ath11k_mac_get_ar_by_pdev_id() was not marked as a read-side critical section as reported by RCU lockdep:

On what hardware and firmware version did you test this? As there's so many different combos we use Tested-on tag to provide that information in the commit message:

Quoted 3 lines

https://wireless.wiki.kernel.org/en/users/drivers/ath11k/submittingpatches#tested-on_tag

I can add that if you let me know what you used.

I hit this on the Lenovo Thinkpad X13s and I guess the tag should be:

Tested-on: QCNFA765 hw2.1 WLAN.HSP.1.1-03125-QCAHSPSWPL_V1_V2_SILICONZ_LITE-3.6510.23

Note that I've only been able to test the ath11k fixes (not the corresponding ath12k) and I only tested this particular patch fully

Quoted 3 lines · Johan Hovold

I hit this on the Lenovo Thinkpad X13s and I guess the tag should be:

Tested-on: QCNFA765 hw2.1 WLAN.HSP.1.1-03125-QCAHSPSWPL_V1_V2_SILICONZ_LITE-3.6510.23

From ath11k point of view QCNFA765 is WCN6855 so I used this one:

Tested-on: WCN6855 hw2.1 PCI WLAN.HSP.1.1-03125-QCAHSPSWPL_V1_V2_SILICONZ_LITE-3.6510.23

Quoted 2 lines

Note that I've only been able to test the ath11k fixes (not the corresponding ath12k) and I only tested this particular patch fully

Thanks, I added Tested-on to this patch 1 and for the rest I added "compile tested only".

The ath11k active pdevs are protected by RCU but the temperature event handling code calling ath11k_mac_get_ar_by_pdev_id() was not marked as a read-side critical section as reported by RCU lockdep:

        =============================
        WARNING: suspicious RCU usage
        6.6.0-rc6 #7 Not tainted
        -----------------------------
        drivers/net/wireless/ath/ath11k/mac.c:638 suspicious rcu_dereference_check() usage!

        other info that might help us debug this:

        rcu_scheduler_active = 2, debug_locks = 1
        no locks held by swapper/0/0.
        ...
        Call trace:
        ...
         lockdep_rcu_suspicious+0x16c/0x22c
         ath11k_mac_get_ar_by_pdev_id+0x194/0x1b0 [ath11k]
         ath11k_wmi_tlv_op_rx+0xa84/0x2c1c [ath11k]
         ath11k_htc_rx_completion_handler+0x388/0x510 [ath11k]

Mark the code in question as an RCU read-side critical section to avoid any potential use-after-free issues.

Tested-on: WCN6855 hw2.1 PCI WLAN.HSP.1.1-03125-QCAHSPSWPL_V1_V2_SILICONZ_LITE-3.6510.23

Fixes a41d10348b01 ("ath11k: add thermal sensor device support")

Cc [email protected] # 5.7

Signed-off-by Johan Hovold <[email protected]>

Acked-by Jeff Johnson <[email protected]>

Signed-off-by Kalle Valo <[email protected]>

RCU lockdep reported suspicious RCU usage when accessing the temperature sensor. Inspection revealed that the DFS radar event code was also missing the required RCU read-side critical section marking.

Johan

Changes in v2
- add the missing rcu_read_unlock() to an ath11k_wmi_pdev_temperature_event() error path as noticed by Jeff

Johan Hovold (2):
wifi: ath11k: fix temperature event locking
wifi: ath11k: fix dfs radar event locking

Quoted 12 lines · Johan Hovold

RCU lockdep reported suspicious RCU usage when accessing the temperature sensor. Inspection revealed that the DFS radar event code was also missing the required RCU read-side critical section marking.

Johan

Changes in v2
- add the missing rcu_read_unlock() to an ath11k_wmi_pdev_temperature_event() error path as noticed by Jeff

Quoted 3 lines

Johan Hovold (2):
wifi: ath11k: fix temperature event locking
wifi: ath11k: fix dfs radar event locking

Thanks for the fixes. I really like using lockdep_assert_held() to document if a function requires some lock held, is there anything similar for RCU?

Quoted 14 lines · Kalle Valo

Quoted 12 lines · Johan Hovold

RCU lockdep reported suspicious RCU usage when accessing the temperature sensor. Inspection revealed that the DFS radar event code was also missing the required RCU read-side critical section marking.

Johan

Changes in v2
- add the missing rcu_read_unlock() to an ath11k_wmi_pdev_temperature_event() error path as noticed by Jeff

Quoted 7 lines

Quoted 3 lines

Johan Hovold (2):
wifi: ath11k: fix temperature event locking
wifi: ath11k: fix dfs radar event locking

Thanks for the fixes. I really like using lockdep_assert_held() to document if a function requires some lock held, is there anything similar for RCU?

Not really, but the checking is instead built into the primitives like rcu_dereference() and enabled whenever CONFIG_PROVE_RCU is set.

For some special cases, we have open-coded checks like:

	RCU_LOCKDEP_WARN(!rcu_read_lock_held());

which similarly depend on CONFIG_PROVE_RCU or simply

	WARN_ON_ONCE(!rcu_read_lock_held());

(old discussion, changing title)

Quoted 15 lines · Johan Hovold

Quoted 3 lines · Kalle Valo

Thanks for the fixes. I really like using lockdep_assert_held() to document if a function requires some lock held, is there anything similar for RCU?

Not really, but the checking is instead built into the primitives like rcu_dereference() and enabled whenever CONFIG_PROVE_RCU is set.

For some special cases, we have open-coded checks like:

	RCU_LOCKDEP_WARN(!rcu_read_lock_held());

which similarly depend on CONFIG_PROVE_RCU or simply

Quoted 1 line

	WARN_ON_ONCE(!rcu_read_lock_held());

I just found out that sparse has __must_hold():

https://lore.kernel.org/linux-wireless/[email protected]/

That looks promising, should we start using that in ath11k and ath12k to check our RCU usage?

Quoted 2 lines · Kalle Valo

I just found out that sparse has __must_hold():

Quoted 4 lines

https://lore.kernel.org/linux-wireless/[email protected]/

That looks promising, should we start using that in ath11k and ath12k to check our RCU usage?

I see that Johannes already commented on this in the thread above.

I'm pretty sure smatch can't be used for this.

Quoted 6 lines · Johan Hovold

Quoted 4 lines · Kalle Valo

I just found out that sparse has __must_hold():

https://lore.kernel.org/linux-wireless/[email protected]/

Quoted 6 lines

Quoted 2 lines

That looks promising, should we start using that in ath11k and ath12k to check our RCU usage?

I see that Johannes already commented on this in the thread above.

I'm pretty sure smatch can't be used for this.

I meant "sparse"...