[PATCH ath-next v2] wifi: ath12k: Abort scan before removing link interface to prevent duplicate deletion

[Lingbo Kong]

On 2025/4/14 17:29, Lorenzo Stoakes wrote:
> On Mon, Apr 14, 2025 at 11:27:47AM +0800, Lingbo Kong wrote:
>>
>>
>> On 2025/4/11 18:05, Lorenzo Stoakes wrote:
>>> +cc Oleskandr who kindly pointed me at the v1 of this patch (see [0]).
>>>
>>> [0]:https://lore.kernel.org/all/20250124093352.481-1-quic_lingbok@quicinc.com/
>>>
>>> On Wed, Feb 26, 2025 at 07:31:18PM +0800, Lingbo Kong wrote:
>>>> Currently, when ath12k performs the remove link interface operation, if
>>>> there is an ongoing scan operation on the arvif, ath12k may execute the
>>>> remove link interface operation multiple times on the same arvif. This
>>>> occurs because, during the remove link operation, if a scan operation is
>>>> present on the arvif, ath12k may receive a WMI_SCAN_EVENT_COMPLETED event
>>>> from the firmware. Upon receiving this event, ath12k will continue to
>>>> execute the ath12k_scan_vdev_clean_work() function, performing the remove
>>>> link interface operation on the same arvif again.
>>>>
>>>> To address this issue, before executing the remove link interface
>>>> operation, ath12k needs to check if there is an ongoing scan operation on
>>>> the current arvif. If such an operation exists, it should be aborted.
>>>>
>>>> Tested-on: WCN7850 hw2.0 PCI WLAN.HMT.1.0.c5-00481-QCAHMTSWPL_V1.0_V2.0_SILICONZ-3
>>>>
>>>> Signed-off-by: Lingbo Kong <quic_lingbok at quicinc.com>
>>>
>>> Hey, thanks for this!
>>>
>>> Not sure on status of this, has the patch been taken for 6.15? As I don't
>>> see it in Linus's tree (not looking _that_ hard though). I don't think it's
>>> even in -next?
>>>
>>> I keep hitting issues with my X870E CARBON wifi onboard motherboard wifi -
>>> most recently I saw a null pointer deref in ath12k_mac_remove_link_interface().
>>>
>>> This occurred when I tried changing the network interface, in fact I had
>>> first clicked on 'available networks' in network manager so quite likely a
>>> concurrent scan.
>>>
>>> I rather stupidly didn't copy/paste the text of it, but you can see the
>>> report in screenshot form at [1]. Apologies for shade being case on ath12k
>>> driver but you know, frustrations :))
>>>
>>> It's difficult for me to test your patch as I am having pretty awful
>>> firmware issue with this motherboard - if I powercycle in any way that gets
>>> interrupted, or especially if a kernel issue arises, then the ath12k module
>>> will not load on next boot, or at all going forward.
>>>
>>> Updating the kernel to, I think, a recent 6.13 (and now 6.14-1 where I
>>> observed this issue), got the wifi working again, seemingly randomly.
>>>
>>> Usually I have to try to reset the CMOS state, but doing so causes other
>>> issues so I generally try to avoid (I have a network workaround involving
>>> an ethernet wifi adapater, it's pretty... yeah).
>>>
>>> So I assume some non-volatile state gets corrupted somehow, I'm not sure if
>>> you had any insights into how I might more sanely reset that?
>>>
>>> Anyway regardless thanks for your efforts, if the wifi adapter appears
>>> again then I will test this and can give T-b tags if so.
>>>
>>> Cheers, Lorenzo
>>>
>>> [1]:https://fosstodon.org/@ljs/114318530969496869
>>>
>>
>> hi, lorenzo,
>> sry for delay response.😊
>> as for as i know, this patch hasn't been accepted so far.
>> for your description, you find the ath12k_mac_remove_link_interface() have a
>> null pointer deref, i think maybe there is a concurrent scan operation.
>>
>> i have a suggestion, you can apply this patch and observe if there is still
>> a crash.
> 
> Unfortunately as I said it's really hard for me to test this since the
> motherboard wifi is permanently unavailable due to some cmos state which is
> really painful/difficult for me to restore.
> 
> This is another bug/issue specific I guess to my motherboard.
> 
> I can give it a try if/when that ever comes back, but I really can't afford
> unfortunately on this machine the time/risk to try to reset mobo state as things
> become unstable when I try.
> 
>>
>> can you offer a detailed crash dump?
> 
> Again, unfortunately, all I can offer is that screenshot I took, I stupidly
> didn't copy/paste it.
> 
> And it seems kernel I was using had symbols stripped so hard to figure out
> exactly where in function it occurred.
> 

oh, sry, i just noticed the link you posted. after looking at your 
screenshot, it seems the issue is indeed related to concurrent scan. my 
patch should be solved this problem, but i need to do further investigate.:)

i'll resent this patch rebasing the upstream code.:)

thx a lot! lorenzo.

/lingbo



>>
>> /lingbo
>>
>>
>>>> ---
>>>> 1.rebase to ath-next
>>>>
>>>>    drivers/net/wireless/ath/ath12k/mac.c | 5 +++++
>>>>    1 file changed, 5 insertions(+)
>>>>
>>>> diff --git a/drivers/net/wireless/ath/ath12k/mac.c b/drivers/net/wireless/ath/ath12k/mac.c
>>>> index 3e3afdc56fc9..551133483f44 100644
>>>> --- a/drivers/net/wireless/ath/ath12k/mac.c
>>>> +++ b/drivers/net/wireless/ath/ath12k/mac.c
>>>> @@ -9578,6 +9578,11 @@ ath12k_mac_op_unassign_vif_chanctx(struct ieee80211_hw *hw,
>>>>    	if (ahvif->vdev_type != WMI_VDEV_TYPE_MONITOR &&
>>>>    	    ar->num_started_vdevs == 1 && ar->monitor_vdev_created)
>>>>    		ath12k_mac_monitor_stop(ar);
>>>> +
>>>> +	if (ar->scan.arvif == arvif && ar->scan.state == ATH12K_SCAN_RUNNING) {
>>>> +		ath12k_scan_abort(ar);
>>>> +		ar->scan.arvif = NULL;
>>>> +	}
>>>>    }
>>>>
>>>>    static int
>>>>
>>>> base-commit: e180a01bf2c4a67db13d70d2d91410a8c6f74be3
>>>> --
>>>> 2.34.1
>>>>
>>>>