Re: [PATCH] wifi: ath12k: fix NULL dereference in hw scan cleanup path

[Rameshkumar Sundaram <rameshkumar.sundaram@oss.qualcomm.com>]

On 9/2/2026 6:31 AM, Chia-Lin Kao (AceLan) wrote:
> ath12k_mac_op_hw_scan() walks ahvif->links_map in its abort path to
> tear down the scan vdevs it may have created. It skips link vifs which
> are no longer present:
> 
> 	arvif = wiphy_dereference(hw->wiphy, ahvif->link[link_id]);
> 	if (!arvif)
> 		continue;
> 
> 	ar = arvif->ar;
> 	if (ar->scan.arvif == arvif) {
> 
> but it does not check arvif->ar, which is cleared in several teardown
> paths while the link remains set in links_map. When firmware crashes
> and the reset worker detaches the link vifs from their radio, a scan


Thanks for fixing this. I'm trying to understand the recovery path that 
detaches the link vif while leaving links_map unchanged. Could you share 
more details on that?

> request arriving in that window fails, enters the abort path and
> dereferences a NULL ar:
> 
>    BUG: kernel NULL pointer dereference, address: 0000000000001508
>    RIP: 0010:ath12k_mac_op_hw_scan+0x170/0x8a0 [ath12k]
>    Call Trace:
>     drv_hw_scan+0xa0/0x160 [mac80211]
>     __ieee80211_start_scan+0x305/0x7b0 [mac80211]
>     ieee80211_request_scan+0xe/0x20 [mac80211]
>     nl80211_trigger_scan+0x610/0xa20 [cfg80211]
> 
> 0x1508 is the offset of scan.arvif within struct ath12k, reached from
> a NULL base.
> 
> The oops happens in a task holding wiphy and rtnl locks, so it takes
> down the rest of the networking stack with it: subsequent scan, netns
> and NetworkManager operations block indefinitely in D state.
> 
> Check the radio before using it, matching the existing arvif check
> directly above and the arvif->ar check already used elsewhere in this
> file.
> 


One sequence that seems to leave the link vif assigned is:

ath12k_mac_assign_vif_to_vdev() :

assign_chanctx -> ath12k_mac_assign_vif_to_vdev
       -> ath12k_mac_assign_link_vif: links_map |= BIT(0)
       -> ath12k_mac_vdev_create: arvif->ar = ar0, WMI fails as FW 
crashed, returns clearing arvif->ar
       -> returns ar (NULL), but still links_map has BIT(0) and link is 
assigned

At that point, links_map still contains BIT(0), and a subsequent 
ath12k_mac_op_hw_scan() may de-reference link0's arvif->ar.

Is this the sequence you're hitting?
May be error messages from ath12k_mac_assign_vif_to_vdev() help confirm 
that?


> Fixes: feed05f1526e8 ("wifi: ath12k: Split scan request for split band device")
> Signed-off-by: Chia-Lin Kao (AceLan) <acelan.kao@canonical.com>
> ---
>   drivers/net/wireless/ath/ath12k/mac.c | 10 ++++++++++
>   1 file changed, 10 insertions(+)
> 
> diff --git a/drivers/net/wireless/ath/ath12k/mac.c b/drivers/net/wireless/ath/ath12k/mac.c
> index 99bf5cf79d102..64a636f3ae3c6 100644
> --- a/drivers/net/wireless/ath/ath12k/mac.c
> +++ b/drivers/net/wireless/ath/ath12k/mac.c
> @@ -5821,6 +5821,16 @@ int ath12k_mac_op_hw_scan(struct ieee80211_hw *hw,
>   				continue;
>   
>   			ar = arvif->ar;
> +
> +			/* The link vif may have been detached from its radio
> +			 * while this scan request was being processed, for
> +			 * example by a firmware recovery running concurrently.
> +			 * The link is still set in links_map in that case, so
> +			 * the radio has to be checked before it is used.
> +			 */
> +			if (!ar)
> +				continue;
> +
>   			if (ar->scan.arvif == arvif) {
>   				wiphy_work_cancel(hw->wiphy, &ar->scan.vdev_clean_wk);
>   				spin_lock_bh(&ar->data_lock);


if above is the scenario, along with this change, would it make sense to 
call ath12k_mac_unassign_link_vif(arvif) when ath12k_mac_vdev_create() 
fails in ath12k_mac_assign_vif_to_vdev() ?




-- 
Ramesh