On 9/25/2024 7:09 PM, Rameshkumar Sundaram wrote:
> During ath12k module removal, in ath12k_core_deinit(),
> ath12k_mac_destroy() un-registers ah->hw from mac80211 and frees
> the ah->hw as well as all the ar's in it. After this
> ath12k_core_soc_destroy()-> ath12k_dp_free()-> ath12k_dp_cc_cleanup()
> tries to access one of the freed ar's from pending skb.
>
> This is because during mac destroy, driver failed to flush few
> data packets, which were accessed later in ath12k_dp_cc_cleanup()
> and freed, but using ar from the packet led to this use-after-free.
>
> BUG: KASAN: use-after-free in ath12k_dp_cc_cleanup.part.0+0x5e2/0xd40 [ath12k]
> Write of size 4 at addr ffff888150bd3514 by task modprobe/8926
> CPU: 0 UID: 0 PID: 8926 Comm: modprobe Not tainted
> 6.11.0-rc2-wt-ath+ #1746
> Hardware name: Intel(R) Client Systems NUC8i7HVK/NUC8i7HVB, BIOS
> HNKBLi70.86A.0067.2021.0528.1339 05/28/2021
>
> Call Trace:
> <TASK>
> dump_stack_lvl+0x7d/0xe0
> print_address_description.constprop.0+0x33/0x3a0
> print_report+0xb5/0x260
> ? kasan_addr_to_slab+0x24/0x80
> kasan_report+0xd8/0x110
> ? ath12k_dp_cc_cleanup.part.0+0x5e2/0xd40 [ath12k]
> ? ath12k_dp_cc_cleanup.part.0+0x5e2/0xd40 [ath12k]
> kasan_check_range+0xf3/0x1a0
> __kasan_check_write+0x14/0x20
> ath12k_dp_cc_cleanup.part.0+0x5e2/0xd40 [ath12k]
> ath12k_dp_free+0x178/0x420 [ath12k]
> ath12k_core_stop+0x176/0x200 [ath12k]
> ath12k_core_deinit+0x13f/0x210 [ath12k]
> ath12k_pci_remove+0xad/0x1c0 [ath12k]
> pci_device_remove+0x9b/0x1b0
> device_remove+0xbf/0x150
> device_release_driver_internal+0x3c3/0x580
> ? __kasan_check_read+0x11/0x20
> driver_detach+0xc4/0x190
> bus_remove_driver+0x130/0x2a0
> driver_unregister+0x68/0x90
> pci_unregister_driver+0x24/0x240
> ? find_module_all+0x13e/0x1e0
> ath12k_pci_exit+0x10/0x20 [ath12k]
> __do_sys_delete_module+0x32c/0x580
> ? module_flags+0x2f0/0x2f0
> ? kmem_cache_free+0xf0/0x410
> ? __fput+0x56f/0xab0
> ? __fput+0x56f/0xab0
> ? debug_smp_processor_id+0x17/0x20
> __x64_sys_delete_module+0x4f/0x70
> x64_sys_call+0x522/0x9f0
> do_syscall_64+0x64/0x130
> entry_SYSCALL_64_after_hwframe+0x4b/0x53
> RIP: 0033:0x7f8182c6ac8b
>
> Commit 24de1b7b231c ("wifi: ath12k: fix flush failure in recovery
> scenarios") added the change to decrement the pending packets count
> in case of recovery which make sense as ah->hw as well all
> ar's in it are intact during recovery, but during core deinit there
> is no use in decrementing packets count or waking up the empty waitq
> as the module is going to be removed also ar from pending skbs
> can't be used and the packets should just be released back.
>
> To fix this, add additional vif check before trying to use the
> ar from skb->cb, because during core_deinit mac80211 will delete
> all the vifs and ath12k will remove these vif's reference
> (ath12k_mac_vif_unref()) from pending packets.
>
> Tested-on: QCN9274 hw2.0 PCI WLAN.WBE.1.1.1-00214-QCAHKSWPL_SILICONZ-1
> Tested-on: WCN7850 hw2.0 PCI WLAN.HMT.1.0.c5-00481-QCAHMTSWPL_V1.0_V2.0_SILICONZ-3
>
> Fixes: 24de1b7b231c ("wifi: ath12k: fix flush failure in recovery scenarios")
> Signed-off-by: Rameshkumar Sundaram <quic_ramess@xxxxxxxxxxx>
> ---
> drivers/net/wireless/ath/ath12k/dp.c | 11 ++++++++---
> 1 file changed, 8 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/net/wireless/ath/ath12k/dp.c b/drivers/net/wireless/ath/ath12k/dp.c
> index 61aa78d8bd8c..09363ca15701 100644
> --- a/drivers/net/wireless/ath/ath12k/dp.c
> +++ b/drivers/net/wireless/ath/ath12k/dp.c
> @@ -1203,9 +1203,14 @@ static void ath12k_dp_cc_cleanup(struct ath12k_base *ab)
> continue;
>
> skb_cb = ATH12K_SKB_CB(skb);
> - ar = skb_cb->ar;
> - if (atomic_dec_and_test(&ar->dp.num_tx_pending))
> - wake_up(&ar->dp.tx_empty_waitq);
> + /* skb_cb's ar is valid only if the skb is associated with
> + * a valid vif.
> + */
> + if (skb_cb->vif) {
> + ar = skb_cb->ar;
> + if (atomic_dec_and_test(&ar->dp.num_tx_pending))
> + wake_up(&ar->dp.tx_empty_waitq);
> + }
>
> dma_unmap_single(ab->dev, ATH12K_SKB_CB(skb)->paddr,
> skb->len, DMA_TO_DEVICE);
>
> base-commit: d35bb26e150d7fb7434959fad9fcaeaac99906e6
dropping this since there will be a v2
