ath6kl: Fix potential skb double free in ath6kl_wmi_sync_point()
skb given to ath6kl_control_tx() is owned by ath6kl_control_tx().
Calling function should not free the skb for error cases.
This is found during code review.
kvalo: fix a checkpatch warning in ath6kl_wmi_cmd_send()
Signed-off-by: Vasanthakumar Thiagarajan <vthiagar@qca.qualcomm.com>
Signed-off-by: Kalle Valo <kvalo@qca.qualcomm.com>
diff --git a/drivers/net/wireless/ath/ath6kl/wmi.c b/drivers/net/wireless/ath/ath6kl/wmi.c
index d485a7c..373751f 100644
--- a/drivers/net/wireless/ath/ath6kl/wmi.c
+++ b/drivers/net/wireless/ath/ath6kl/wmi.c
@@ -1739,8 +1739,11 @@
int ret;
u16 info1;
- if (WARN_ON(skb == NULL || (if_idx > (wmi->parent_dev->vif_max - 1))))
+ if (WARN_ON(skb == NULL ||
+ (if_idx > (wmi->parent_dev->vif_max - 1)))) {
+ dev_kfree_skb(skb);
return -EINVAL;
+ }
ath6kl_dbg(ATH6KL_DBG_WMI, "wmi tx id %d len %d flag %d\n",
cmd_id, skb->len, sync_flag);
@@ -2352,8 +2355,10 @@
struct wmi_data_hdr *data_hdr;
int ret;
- if (WARN_ON(skb == NULL || ep_id == wmi->ep_id))
+ if (WARN_ON(skb == NULL || ep_id == wmi->ep_id)) {
+ dev_kfree_skb(skb);
return -EINVAL;
+ }
skb_push(skb, sizeof(struct wmi_data_hdr));
@@ -2390,10 +2395,8 @@
spin_unlock_bh(&wmi->lock);
skb = ath6kl_wmi_get_new_buf(sizeof(*cmd));
- if (!skb) {
- ret = -ENOMEM;
- goto free_skb;
- }
+ if (!skb)
+ return -ENOMEM;
cmd = (struct wmi_sync_cmd *) skb->data;
@@ -2416,7 +2419,7 @@
* then do not send the Synchronize cmd on the control ep
*/
if (ret)
- goto free_skb;
+ goto free_cmd_skb;
/*
* Send sync cmd followed by sync data messages on all
@@ -2426,15 +2429,12 @@
NO_SYNC_WMIFLAG);
if (ret)
- goto free_skb;
-
- /* cmd buffer sent, we no longer own it */
- skb = NULL;
+ goto free_data_skb;
for (index = 0; index < num_pri_streams; index++) {
if (WARN_ON(!data_sync_bufs[index].skb))
- break;
+ goto free_data_skb;
ep_id = ath6kl_ac2_endpoint_id(wmi->parent_dev,
data_sync_bufs[index].
@@ -2443,17 +2443,20 @@
ath6kl_wmi_data_sync_send(wmi, data_sync_bufs[index].skb,
ep_id, if_idx);
- if (ret)
- break;
-
data_sync_bufs[index].skb = NULL;
+
+ if (ret)
+ goto free_data_skb;
}
-free_skb:
+ return 0;
+
+free_cmd_skb:
/* free up any resources left over (possibly due to an error) */
if (skb)
dev_kfree_skb(skb);
+free_data_skb:
for (index = 0; index < num_pri_streams; index++) {
if (data_sync_bufs[index].skb != NULL) {
dev_kfree_skb((struct sk_buff *)data_sync_bufs[index].