From patchwork Tue Aug 14 04:40:33 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Vasanthakumar Thiagarajan X-Patchwork-Id: 1317991 Return-Path: X-Original-To: patchwork-linux-wireless@patchwork.kernel.org Delivered-To: patchwork-process-083081@patchwork2.kernel.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by patchwork2.kernel.org (Postfix) with ESMTP id C8EFBDF266 for ; Tue, 14 Aug 2012 04:41:00 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751394Ab2HNEkd (ORCPT ); Tue, 14 Aug 2012 00:40:33 -0400 Received: from wolverine01.qualcomm.com ([199.106.114.254]:38745 "EHLO wolverine01.qualcomm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751029Ab2HNEkc (ORCPT ); Tue, 14 Aug 2012 00:40:32 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=qca.qualcomm.com; i=@qca.qualcomm.com; q=dns/txt; s=qcdkim; t=1344919233; x=1376455233; h=from:to:cc:subject:date:message-id:mime-version; bh=Ao74Wd7M7MTO1g5l4Mv2RAoSahvifueFmh/q6EPHLnY=; b=j02gwhl7O+2/MzMjxjKkCgo2acrRBHr8srppt9pf2H+AMzPOFgozyvgZ LELRtTV0w8P0uKzYqP013h9w/rqPAz9+uX8hiPQbGCPkNdzOFHs/Brk2i aEetRpRyLcZ+HBNnsD9c0tMuWWfjXopRGdkRB3pmlzSOfAkC/XOiXzQbG k=; X-IronPort-AV: E=McAfee;i="5400,1158,6802"; a="224990251" Received: from ironmsg02-r.qualcomm.com ([172.30.46.16]) by wolverine01.qualcomm.com with ESMTP; 13 Aug 2012 21:40:29 -0700 X-IronPort-AV: E=Sophos;i="4.77,764,1336374000"; d="scan'208";a="160485066" Received: from nasanexhc11.na.qualcomm.com ([172.30.39.6]) by ironmsg02-R.qualcomm.com with ESMTP/TLS/RC4-SHA; 13 Aug 2012 21:40:29 -0700 Received: from nasanexhc05.na.qualcomm.com (172.30.48.2) by nasanexhc11.na.qualcomm.com (172.30.39.6) with Microsoft SMTP Server (TLS) id 14.2.309.2; Mon, 13 Aug 2012 21:40:28 -0700 Received: from qcamail1.qualcomm.com (172.30.48.1) by qcmail1.qualcomm.com (172.30.48.2) with Microsoft SMTP Server (TLS) id 14.2.309.2; Mon, 13 Aug 2012 21:40:26 -0700 Received: by qcamail1.qualcomm.com (sSMTP sendmail emulation); Tue, 14 Aug 2012 10:10:34 +0530 From: Vasanthakumar Thiagarajan To: CC: , Subject: [PATCH V2 1/2] ath6kl: Fix potential skb double free in ath6kl_wmi_sync_point() Date: Tue, 14 Aug 2012 10:10:33 +0530 Message-ID: <1344919234-5012-1-git-send-email-vthiagar@qca.qualcomm.com> X-Mailer: git-send-email 1.7.0.4 MIME-Version: 1.0 X-Originating-IP: [172.30.48.1] Sender: linux-wireless-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-wireless@vger.kernel.org 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. Signed-off-by: Vasanthakumar Thiagarajan --- drivers/net/wireless/ath/ath6kl/txrx.c | 4 ++- drivers/net/wireless/ath/ath6kl/wmi.c | 35 +++++++++++++++++-------------- 2 files changed, 22 insertions(+), 17 deletions(-) diff --git a/drivers/net/wireless/ath/ath6kl/txrx.c b/drivers/net/wireless/ath/ath6kl/txrx.c index 7dfa0fd..aab8251 100644 --- a/drivers/net/wireless/ath/ath6kl/txrx.c +++ b/drivers/net/wireless/ath/ath6kl/txrx.c @@ -288,8 +288,10 @@ int ath6kl_control_tx(void *devt, struct sk_buff *skb, int status = 0; struct ath6kl_cookie *cookie = NULL; - if (WARN_ON_ONCE(ar->state == ATH6KL_STATE_WOW)) + if (WARN_ON_ONCE(ar->state == ATH6KL_STATE_WOW)) { + dev_kfree_skb(skb); return -EACCES; + } spin_lock_bh(&ar->lock); diff --git a/drivers/net/wireless/ath/ath6kl/wmi.c b/drivers/net/wireless/ath/ath6kl/wmi.c index cf91348..039f668 100644 --- a/drivers/net/wireless/ath/ath6kl/wmi.c +++ b/drivers/net/wireless/ath/ath6kl/wmi.c @@ -1739,8 +1739,11 @@ int ath6kl_wmi_cmd_send(struct wmi *wmi, u8 if_idx, struct sk_buff *skb, 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 @@ static int ath6kl_wmi_data_sync_send(struct wmi *wmi, struct sk_buff *skb, 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 @@ static int ath6kl_wmi_sync_point(struct wmi *wmi, u8 if_idx) 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 @@ static int ath6kl_wmi_sync_point(struct wmi *wmi, u8 if_idx) * 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 @@ static int ath6kl_wmi_sync_point(struct wmi *wmi, u8 if_idx) 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 @@ static int ath6kl_wmi_sync_point(struct wmi *wmi, u8 if_idx) 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].