From patchwork Sun Oct 21 10:52:06 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Julia Lawall X-Patchwork-Id: 1622391 Return-Path: X-Original-To: patchwork-linux-wireless@patchwork.kernel.org Delivered-To: patchwork-process-083081@patchwork1.kernel.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by patchwork1.kernel.org (Postfix) with ESMTP id 4B32E400E9 for ; Sun, 21 Oct 2012 10:53:27 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752907Ab2JUKwV (ORCPT ); Sun, 21 Oct 2012 06:52:21 -0400 Received: from mail4-relais-sop.national.inria.fr ([192.134.164.105]:16977 "EHLO mail4-relais-sop.national.inria.fr" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752481Ab2JUKwP (ORCPT ); Sun, 21 Oct 2012 06:52:15 -0400 X-IronPort-AV: E=Sophos;i="4.80,625,1344204000"; d="scan'208";a="159799683" Received: from palace.lip6.fr (HELO localhost.localdomain) ([132.227.105.202]) by mail4-relais-sop.national.inria.fr with ESMTP/TLS/DHE-RSA-AES256-SHA; 21 Oct 2012 12:52:10 +0200 From: Julia Lawall To: Kalle Valo Cc: kernel-janitors@vger.kernel.org, "John W. Linville" , linux-wireless@vger.kernel.org, netdev@vger.kernel.org, linux-kernel@vger.kernel.org Subject: [PATCH 4/5] ath6kl/wmi.c: eliminate possible double free Date: Sun, 21 Oct 2012 12:52:06 +0200 Message-Id: <1350816727-1381-5-git-send-email-Julia.Lawall@lip6.fr> X-Mailer: git-send-email 1.7.8.6 In-Reply-To: <1350816727-1381-1-git-send-email-Julia.Lawall@lip6.fr> References: <1350816727-1381-1-git-send-email-Julia.Lawall@lip6.fr> Sender: linux-wireless-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-wireless@vger.kernel.org From: Julia Lawall This makes two changes. In ath6kl_wmi_cmd_send, a call to dev_kfree_skb on the skb argument is added to the initial sanity check to more completely establish the invariant that ath6kl_wmi_cmd_send owns its skb argument. Then, in ath6kl_wmi_sync_point, on failure of the call to ath6kl_wmi_cmd_send, the clearing of the local skb variable is moved up, so that the error-handling code at the end of the function does not free it again. A simplified version of the semantic match that finds this problem is as follows: (http://coccinelle.lip6.fr/) // @r@ identifier f,free,a; parameter list[n] ps; type T; expression e; @@ f(ps,T a,...) { ... when any when != a = e if(...) { ... free(a); ... return ...; } ... when any } @@ identifier r.f,r.free; expression x,a; expression list[r.n] xs; @@ * x = f(xs,a,...); if (...) { ... free(a); ... return ...; } // Signed-off-by: Julia Lawall --- Not tested. drivers/net/wireless/ath/ath6kl/wmi.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html diff --git a/drivers/net/wireless/ath/ath6kl/wmi.c b/drivers/net/wireless/ath/ath6kl/wmi.c index c30ab4b..50f50e4 100644 --- a/drivers/net/wireless/ath/ath6kl/wmi.c +++ b/drivers/net/wireless/ath/ath6kl/wmi.c @@ -1677,8 +1677,10 @@ 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); @@ -2348,12 +2350,12 @@ static int ath6kl_wmi_sync_point(struct wmi *wmi, u8 if_idx) ret = ath6kl_wmi_cmd_send(wmi, if_idx, skb, WMI_SYNCHRONIZE_CMDID, NO_SYNC_WMIFLAG); - if (ret) - goto free_skb; - /* cmd buffer sent, we no longer own it */ skb = NULL; + if (ret) + goto free_skb; + for (index = 0; index < num_pri_streams; index++) { if (WARN_ON(!data_sync_bufs[index].skb))