Re: [PATCH 2/2] wifi: ath11k: fix error path leaks in some WMI calls

["Nicolas Escande" <nico.escande@gmail.com>]

On Tue May 5, 2026 at 7:23 PM CEST, Rameshkumar Sundaram wrote:
> On 5/2/2026 12:44 PM, Nicolas Escande wrote:
>> This is the same pattern that was previously identified as problematic:
>> direct 'return ath11k_wmi_cmd_send(...)' will leak the skb in the error
>> path if it is not explicitly handled.
>> 
>> Fixes: c417b247ba04 ("ath11k: implement hardware data filter")
>> Fixes: 9cbd7fc9be82 ("ath11k: support MAC address randomization in scan")
>> Fixes: ba9177fcef21 ("ath11k: Add basic WoW functionalities")
>> Fixes: fec4b898f369 ("ath11k: Add WoW net-detect functionality")
>> Fixes: c3c36bfe998b ("ath11k: support ARP and NS offload")
>> Fixes: a16d9b50cfba ("ath11k: support GTK rekey offload")
>> Fixes: 652f69ed9c1b ("ath11k: Add support for SAR")
>> Fixes: 0f84a156aa3b ("ath11k: Handle keepalive during WoWLAN suspend and resume")
>> Signed-off-by: Nicolas Escande <nico.escande@gmail.com>
>> ---
>>   drivers/net/wireless/ath/ath11k/wmi.c | 112 ++++++++++++++++++++++----
>>   1 file changed, 96 insertions(+), 16 deletions(-)
>> 
>> diff --git a/drivers/net/wireless/ath/ath11k/wmi.c b/drivers/net/wireless/ath/ath11k/wmi.c
>> index 024c2aad9fb4..dca6e011cc40 100644
>> --- a/drivers/net/wireless/ath/ath11k/wmi.c
>> +++ b/drivers/net/wireless/ath/ath11k/wmi.c
>> @@ -9299,7 +9299,7 @@ int ath11k_wmi_hw_data_filter_cmd(struct ath11k *ar, u32 vdev_id,
>>   {
>>   	struct wmi_hw_data_filter_cmd *cmd;
>>   	struct sk_buff *skb;
>> -	int len;
>> +	int ret, len;
>>   
>>   	len = sizeof(*cmd);
>>   	skb = ath11k_wmi_alloc_skb(ar->wmi->wmi_ab, len);
>> @@ -9324,7 +9324,13 @@ int ath11k_wmi_hw_data_filter_cmd(struct ath11k *ar, u32 vdev_id,
>>   		   "hw data filter enable %d filter_bitmap 0x%x\n",
>>   		   enable, filter_bitmap);
>>   
>> -	return ath11k_wmi_cmd_send(ar->wmi, skb, WMI_HW_DATA_FILTER_CMDID);
>> +	ret = ath11k_wmi_cmd_send(ar->wmi, skb, WMI_HW_DATA_FILTER_CMDID);
>> +	if (ret) {
>> +		ath11k_warn(ar->ab, "failed to send WMI_HW_DATA_FILTER_CMDID\n");
>> +		dev_kfree_skb(skb);
>> +	}
>> +
>> +	return ret;
>>   }
>>
>
> { .. }
>
>
>> @@ -10053,7 +10127,13 @@ int ath11k_wmi_sta_keepalive(struct ath11k *ar,
>>   		   "sta keepalive vdev %d enabled %d method %d interval %d\n",
>>   		   arg->vdev_id, arg->enabled, arg->method, arg->interval);
>>   
>> -	return ath11k_wmi_cmd_send(wmi, skb, WMI_STA_KEEPALIVE_CMDID);
>> +	ret = ath11k_wmi_cmd_send(wmi, skb, WMI_STA_KEEPALIVE_CMDID);
>> +	if (ret) {
>> +		ath11k_warn(ar->ab, "failed to send WMI_STA_KEEPALIVE_CMDID\n");
>> +		dev_kfree_skb(skb);
>> +	}
>> +
>> +	return ret;
>>   }
>>   
>>   bool ath11k_wmi_supports_6ghz_cc_ext(struct ath11k *ar)
>
>
> Thanks for fixing these. One more instance of the same pattern remains 
> in ath11k_tm_cmd_wmi_ftm().
>
Ha nice catch, I originally skipped it because I saw there was a 'if (ret)' and
assumed it had the proper error handling.

> Please add dev_kfree_skb(skb) before goto out, matching 
> ath11k_tm_cmd_wmi() above.
Yes seems it needs it's own patch and fixes tag. I'll respin the series then.

Thanks