Re: [RESEND PATCH ath-current 2/3] wifi: ath11k: fix NULL dereference in ahb remove when QMI init incomplete

[Miaoqing Pan <miaoqing.pan@oss.qualcomm.com>]

On 8/5/2026 6:07 PM, Rameshkumar Sundaram wrote:
> On 8/3/2026 12:17 PM, Miaoqing Pan wrote:
>>
>>
>> On 7/31/2026 5:24 PM, Rameshkumar Sundaram wrote:
>>> On 7/30/2026 8:39 AM, Miaoqing Pan wrote:
>>>> On WCN6750, if QMI messages never arrive (for example when qrtr_smd
>>>> is not ready), WLAN initialization stops before the device is fully
>>>> registered. In this case ATH11K_FLAG_QMI_FAIL is not set because no
>>>> QMI event handler is executed.
>>>>
>>>> When the driver is removed, ath11k_ahb_remove() still calls
>>>> ath11k_core_deinit(), which eventually triggers
>>>> ath11k_ce_cleanup_pipes() on uninitialized CE pipes and results in a
>>>> NULL pointer dereference in ath11k_hal_srng_access_begin():
>>>>
>>>>    ath11k_hal_srng_access_begin+0x14/0x68 [ath11k]
>>>>    ath11k_ce_cleanup_pipes+0x184/0x190 [ath11k]
>>>>    ath11k_pcic_stop+0x24/0x38 [ath11k]
>>>>    ath11k_core_deinit+0xfc/0x1c0 [ath11k]
>>>>    ath11k_ahb_remove+0x38/0xa0 [ath11k_ahb]
>>>>
>>>> Fix this by invoking ath11k_ahb_remove_prepare() before the state
>>>> check and skipping ath11k_core_deinit() when either QMI initialization
>>>> failed or the device was never registered. If ATH11K_FLAG_REGISTERED
>>>> is not set, core initialization did not complete and CE pipes may
>>>> remain uninitialized, making ath11k_core_deinit() unsafe.
>>>>
>>>> Tested-on: WCN6750 hw1.0 AHB WLAN.MSL.2.0.c2-00204-QCAMSLSWPLZ-1
>>>>
>>>> Fixes: 00402f49d26f ("ath11k: Add support for WCN6750 device")
>>>> Signed-off-by: Miaoqing Pan <miaoqing.pan@oss.qualcomm.com>
>>>> ---
>>>>   drivers/net/wireless/ath/ath11k/ahb.c | 6 ++++--
>>>>   1 file changed, 4 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/drivers/net/wireless/ath/ath11k/ahb.c b/drivers/net/ 
>>>> wireless/ath/ath11k/ahb.c
>>>> index fb5640882b98..7f5f5c8d7c56 100644
>>>> --- a/drivers/net/wireless/ath/ath11k/ahb.c
>>>> +++ b/drivers/net/wireless/ath/ath11k/ahb.c
>>>> @@ -1265,14 +1265,16 @@ static void ath11k_ahb_remove(struct 
>>>> platform_device *pdev)
>>>>   {
>>>>       struct ath11k_base *ab = platform_get_drvdata(pdev);
>>>> -    if (test_bit(ATH11K_FLAG_QMI_FAIL, &ab->dev_flags)) {
>>>> +    ath11k_ahb_remove_prepare(ab);
>>>> +
>>>> +    if (test_bit(ATH11K_FLAG_QMI_FAIL, &ab->dev_flags) ||
>>>> +        !test_bit(ATH11K_FLAG_REGISTERED, &ab->dev_flags)) {
>>>
>>> Can we use ATH11K_FLAG_REGISTERED alone to decide the cleanup path? 
>>> If REGISTERED is set, ath11k_core_qmi_firmware_ready() completed at 
>>> least once and the normal ath11k_core_deinit() path should run. If 
>>> QMI failed on recovery (say before FW_READY or INIT_DONE) this should 
>>> still do core_deinit() isn't ?
>>>
>>>
>> During SSR recovery, it is possible for a QMI failure to occur after 
>> the device has already been registered, which could result in both 
>> ATH11K_FLAG_REGISTERED and ATH11K_FLAG_QMI_FAIL being set at the same 
>> time.
> 
> Yeah so in that case current code will skip core_deinit() which should 
> actually run isn't ? Though the QMI has failed on recovery the core was 
> already initialized and should be torn down.
> 

Thanks for the review.

You are right. During SSR, if QMI fails after the device is already 
registered, core_deinit() should still run for proper cleanup. Will send v2.

>>>>           ath11k_ahb_power_down(ab, false);
>>>>           ath11k_debugfs_soc_destroy(ab);
>>>>           ath11k_qmi_deinit_service(ab);
>>>>           goto qmi_fail;
>>>>       }
>>>> -    ath11k_ahb_remove_prepare(ab);
>>>>       ath11k_core_deinit(ab);
>>>>   qmi_fail:
>>>
> 
> -- 
> Ramesh