From patchwork Thu Mar 15 11:31:33 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ramon Fried X-Patchwork-Id: 131764 Delivered-To: patch@linaro.org Received: by 10.46.84.17 with SMTP id i17csp987592ljb; Thu, 15 Mar 2018 04:31:42 -0700 (PDT) X-Google-Smtp-Source: AG47ELvK/dwTsbpknBQ0Yvg9AKzDK7gwsMRMUU+10vGWZMAMtI3Mq56BV2CrdaaVg5xDIjJtke0V X-Received: by 10.99.94.197 with SMTP id s188mr6379673pgb.363.1521113502064; Thu, 15 Mar 2018 04:31:42 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1521113502; cv=none; d=google.com; s=arc-20160816; b=pQBJra2O0YT/Xz20OnoMxCPLjc7Z/Hx5QDv8QHJrNALUEMU1coQp3hmB/vpDpi0r3X YzUpHd4sat5YWKsOFi6ZhhggKJqmsDYpmDoGHgJuOoV+ALEjaG5WtonA/F0Jr2c6LZIM TR39d7fC6pybHh3oQrNogzTmlTnzCYJgQyo1aZuqfaZ8FsxkqTaSuUrR5pRw9WjXR6PW 8l0a0nR9dtZKzl7j1IzL9pVt05XNvC5cewa7MiP3puM6EL9MOL1tNTYFJg+LHK71x1yq gCLTNr2bFtgeqIwgX9d7Ep2zTcaMsDe1Apl22xfEEWggSsm/mKjfOjPVENF4P44NGHn/ 3Liw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:sender:message-id:date:subject:cc:to:from :dmarc-filter:dkim-signature:dkim-signature :arc-authentication-results; bh=tlLw2K0PXFDp3sn948OXpF7K2kmxmpQeNdbDfjeJsdE=; b=o+0sjfQFyenznUdo8wzhUVII/2Q7e59ybvigtKf3NcTKgl+VNZFRBKK03N+V9vndHu ZzjdD8UV7zHtJc+CETeKv07HTVC1sxqy3FM2VWZUbtZA9jV5vuIjBBIXjPAha98yUWsQ ObVDUTp9/GOnN2Z7yazGK7NxsJevjrraRt494mat58ZsNYeNpLzXVXq3x/eS2TSV0tZK D/gZPDJVMcc9ZsSnRgj6kcDnP8kp1msvTxImlTGu8m4dFo/SyhL/sqKresncsfPpLrPB BMkoqZ+JznBlvqZjIf0XvVc50gCBa0EuwnEo7mlqjnVW0uYW1hQZVfWbFxha3+jDMU45 mr6g== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@codeaurora.org header.s=default header.b=JaUU2PUV; dkim=pass header.i=@codeaurora.org header.s=default header.b=JaUU2PUV; spf=pass (google.com: best guess record for domain of linux-wireless-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-wireless-owner@vger.kernel.org Return-Path: Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id u197si657393pgc.15.2018.03.15.04.31.41; Thu, 15 Mar 2018 04:31:42 -0700 (PDT) Received-SPF: pass (google.com: best guess record for domain of linux-wireless-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67; Authentication-Results: mx.google.com; dkim=pass header.i=@codeaurora.org header.s=default header.b=JaUU2PUV; dkim=pass header.i=@codeaurora.org header.s=default header.b=JaUU2PUV; spf=pass (google.com: best guess record for domain of linux-wireless-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-wireless-owner@vger.kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751698AbeCOLbk (ORCPT + 2 others); Thu, 15 Mar 2018 07:31:40 -0400 Received: from smtp.codeaurora.org ([198.145.29.96]:45366 "EHLO smtp.codeaurora.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751527AbeCOLbj (ORCPT ); Thu, 15 Mar 2018 07:31:39 -0400 Received: by smtp.codeaurora.org (Postfix, from userid 1000) id C6D98600C1; Thu, 15 Mar 2018 11:31:38 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=codeaurora.org; s=default; t=1521113498; bh=yGg1eWSI6QkQzYoLv2e5fZEpHEIOmbkciXhO3YvKymM=; h=From:To:Cc:Subject:Date:From; b=JaUU2PUVNZvAKgv3RvN89BZGEV5GZ5LRf+05ewSmtRMHFjH3JjtSPtEmqMPmSGI8V Yo9QemrlFOHlNCFNQYUdOifXujU5EmLLA2yxfT/HA1dcLmECWdj+UTbQ9dXrh8+p7h zjpj1H8XshqvSFSaFFgv4NlHRhfYMdkTkWuz8oE8= X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on pdx-caf-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-2.8 required=2.0 tests=ALL_TRUSTED,BAYES_00, DKIM_SIGNED, T_DKIM_INVALID autolearn=no autolearn_force=no version=3.4.0 Received: from lx-rfried.mea.qualcomm.com (unknown [185.23.60.4]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-SHA256 (128/128 bits)) (No client certificate requested) (Authenticated sender: rfried@smtp.codeaurora.org) by smtp.codeaurora.org (Postfix) with ESMTPSA id B013D602FC; Thu, 15 Mar 2018 11:31:36 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=codeaurora.org; s=default; t=1521113498; bh=yGg1eWSI6QkQzYoLv2e5fZEpHEIOmbkciXhO3YvKymM=; h=From:To:Cc:Subject:Date:From; b=JaUU2PUVNZvAKgv3RvN89BZGEV5GZ5LRf+05ewSmtRMHFjH3JjtSPtEmqMPmSGI8V Yo9QemrlFOHlNCFNQYUdOifXujU5EmLLA2yxfT/HA1dcLmECWdj+UTbQ9dXrh8+p7h zjpj1H8XshqvSFSaFFgv4NlHRhfYMdkTkWuz8oE8= DMARC-Filter: OpenDMARC Filter v1.3.2 smtp.codeaurora.org B013D602FC Authentication-Results: pdx-caf-mail.web.codeaurora.org; dmarc=none (p=none dis=none) header.from=codeaurora.org Authentication-Results: pdx-caf-mail.web.codeaurora.org; spf=none smtp.mailfrom=rfried@codeaurora.org From: Ramon Fried To: k.eugene.e@gmail.com, kvalo@codeaurora.org, wcn36xx@lists.infradead.org, linux-wireless@vger.kernel.org Cc: Loic Poulain , Ramon Fried Subject: [PATCH] wcn36xx: Fix firmware crash due to corrupted buffer address Date: Thu, 15 Mar 2018 13:31:33 +0200 Message-Id: <20180315113133.28791-1-rfried@codeaurora.org> X-Mailer: git-send-email 2.15.1 Sender: linux-wireless-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-wireless@vger.kernel.org From: Loic Poulain wcn36xx_start_tx function retrieves the buffer descriptor from the channel control queue to start filling tx buffer information. However, nothing prevents this same buffer to be concurrently accessed in a concurent tx call, leading to potential buffer coruption and firmware crash (observed during iperf test). The channel control queue should only be accessed and updated with the channel lock. Fix this issue by using a local buffer descriptor which will be copied in the thread-safe wcn36xx_dxe_tx_frame. Note that buffer descriptor size is few bytes so the introduced copy overhead is insignificant. Moreover, this allows to keep the locked section minimal. Signed-off-by: Loic Poulain Signed-off-by: Ramon Fried --- drivers/net/wireless/ath/wcn36xx/dxe.c | 13 ++++--------- drivers/net/wireless/ath/wcn36xx/dxe.h | 3 ++- drivers/net/wireless/ath/wcn36xx/txrx.c | 32 ++++++++++---------------------- 3 files changed, 16 insertions(+), 32 deletions(-) -- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project diff --git a/drivers/net/wireless/ath/wcn36xx/dxe.c b/drivers/net/wireless/ath/wcn36xx/dxe.c index 7d5ecaf02288..2c3b899a88fa 100644 --- a/drivers/net/wireless/ath/wcn36xx/dxe.c +++ b/drivers/net/wireless/ath/wcn36xx/dxe.c @@ -27,15 +27,6 @@ #include "wcn36xx.h" #include "txrx.h" -void *wcn36xx_dxe_get_next_bd(struct wcn36xx *wcn, bool is_low) -{ - struct wcn36xx_dxe_ch *ch = is_low ? - &wcn->dxe_tx_l_ch : - &wcn->dxe_tx_h_ch; - - return ch->head_blk_ctl->bd_cpu_addr; -} - static void wcn36xx_ccu_write_register(struct wcn36xx *wcn, int addr, int data) { wcn36xx_dbg(WCN36XX_DBG_DXE, @@ -648,6 +639,7 @@ void wcn36xx_dxe_free_mem_pools(struct wcn36xx *wcn) int wcn36xx_dxe_tx_frame(struct wcn36xx *wcn, struct wcn36xx_vif *vif_priv, + struct wcn36xx_tx_bd *bd, struct sk_buff *skb, bool is_low) { @@ -681,6 +673,9 @@ int wcn36xx_dxe_tx_frame(struct wcn36xx *wcn, ctl->skb = NULL; desc = ctl->desc; + /* write buffer descriptor */ + memcpy(ctl->bd_cpu_addr, bd, sizeof(*bd)); + /* Set source address of the BD we send */ desc->src_addr_l = ctl->bd_phy_addr; diff --git a/drivers/net/wireless/ath/wcn36xx/dxe.h b/drivers/net/wireless/ath/wcn36xx/dxe.h index 2bc376c5391b..ce580960d109 100644 --- a/drivers/net/wireless/ath/wcn36xx/dxe.h +++ b/drivers/net/wireless/ath/wcn36xx/dxe.h @@ -452,6 +452,7 @@ struct wcn36xx_dxe_mem_pool { dma_addr_t phy_addr; }; +struct wcn36xx_tx_bd; struct wcn36xx_vif; int wcn36xx_dxe_allocate_mem_pools(struct wcn36xx *wcn); void wcn36xx_dxe_free_mem_pools(struct wcn36xx *wcn); @@ -463,8 +464,8 @@ void wcn36xx_dxe_deinit(struct wcn36xx *wcn); int wcn36xx_dxe_init_channels(struct wcn36xx *wcn); int wcn36xx_dxe_tx_frame(struct wcn36xx *wcn, struct wcn36xx_vif *vif_priv, + struct wcn36xx_tx_bd *bd, struct sk_buff *skb, bool is_low); void wcn36xx_dxe_tx_ack_ind(struct wcn36xx *wcn, u32 status); -void *wcn36xx_dxe_get_next_bd(struct wcn36xx *wcn, bool is_low); #endif /* _DXE_H_ */ diff --git a/drivers/net/wireless/ath/wcn36xx/txrx.c b/drivers/net/wireless/ath/wcn36xx/txrx.c index 22304edc5948..b1768ed6b0be 100644 --- a/drivers/net/wireless/ath/wcn36xx/txrx.c +++ b/drivers/net/wireless/ath/wcn36xx/txrx.c @@ -272,21 +272,9 @@ int wcn36xx_start_tx(struct wcn36xx *wcn, bool is_low = ieee80211_is_data(hdr->frame_control); bool bcast = is_broadcast_ether_addr(hdr->addr1) || is_multicast_ether_addr(hdr->addr1); - struct wcn36xx_tx_bd *bd = wcn36xx_dxe_get_next_bd(wcn, is_low); - - if (!bd) { - /* - * TX DXE are used in pairs. One for the BD and one for the - * actual frame. The BD DXE's has a preallocated buffer while - * the skb ones does not. If this isn't true something is really - * wierd. TODO: Recover from this situation - */ - - wcn36xx_err("bd address may not be NULL for BD DXE\n"); - return -EINVAL; - } + struct wcn36xx_tx_bd bd; - memset(bd, 0, sizeof(*bd)); + memset(&bd, 0, sizeof(bd)); wcn36xx_dbg(WCN36XX_DBG_TX, "tx skb %p len %d fc %04x sn %d %s %s\n", @@ -296,10 +284,10 @@ int wcn36xx_start_tx(struct wcn36xx *wcn, wcn36xx_dbg_dump(WCN36XX_DBG_TX_DUMP, "", skb->data, skb->len); - bd->dpu_rf = WCN36XX_BMU_WQ_TX; + bd.dpu_rf = WCN36XX_BMU_WQ_TX; - bd->tx_comp = !!(info->flags & IEEE80211_TX_CTL_REQ_TX_STATUS); - if (bd->tx_comp) { + bd.tx_comp = !!(info->flags & IEEE80211_TX_CTL_REQ_TX_STATUS); + if (bd.tx_comp) { wcn36xx_dbg(WCN36XX_DBG_DXE, "TX_ACK status requested\n"); spin_lock_irqsave(&wcn->dxe_lock, flags); if (wcn->tx_ack_skb) { @@ -321,13 +309,13 @@ int wcn36xx_start_tx(struct wcn36xx *wcn, /* Data frames served first*/ if (is_low) - wcn36xx_set_tx_data(bd, wcn, &vif_priv, sta_priv, skb, bcast); + wcn36xx_set_tx_data(&bd, wcn, &vif_priv, sta_priv, skb, bcast); else /* MGMT and CTRL frames are handeld here*/ - wcn36xx_set_tx_mgmt(bd, wcn, &vif_priv, skb, bcast); + wcn36xx_set_tx_mgmt(&bd, wcn, &vif_priv, skb, bcast); - buff_to_be((u32 *)bd, sizeof(*bd)/sizeof(u32)); - bd->tx_bd_sign = 0xbdbdbdbd; + buff_to_be((u32 *)&bd, sizeof(bd)/sizeof(u32)); + bd.tx_bd_sign = 0xbdbdbdbd; - return wcn36xx_dxe_tx_frame(wcn, vif_priv, skb, is_low); + return wcn36xx_dxe_tx_frame(wcn, vif_priv, &bd, skb, is_low); }