From patchwork Tue Oct 31 09:42:46 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ganapathi Bhat X-Patchwork-Id: 10033793 X-Patchwork-Delegate: kvalo@adurom.com Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 3358B60327 for ; Tue, 31 Oct 2017 09:46:18 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 1E42C28A23 for ; Tue, 31 Oct 2017 09:46:18 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 12E1E28A32; Tue, 31 Oct 2017 09:46:18 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.9 required=2.0 tests=BAYES_00,RCVD_IN_DNSWL_HI autolearn=ham version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 705C128A23 for ; Tue, 31 Oct 2017 09:46:17 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752372AbdJaJqN (ORCPT ); Tue, 31 Oct 2017 05:46:13 -0400 Received: from mx0a-0016f401.pphosted.com ([67.231.148.174]:35890 "EHLO mx0b-0016f401.pphosted.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1751691AbdJaJqL (ORCPT ); Tue, 31 Oct 2017 05:46:11 -0400 Received: from pps.filterd (m0045849.ppops.net [127.0.0.1]) by mx0a-0016f401.pphosted.com (8.16.0.21/8.16.0.21) with SMTP id v9V9jA9Q007352; Tue, 31 Oct 2017 02:46:09 -0700 Received: from sc-exch03.marvell.com ([199.233.58.183]) by mx0a-0016f401.pphosted.com with ESMTP id 2dxfpmktpr-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-SHA384 bits=256 verify=NOT); Tue, 31 Oct 2017 02:46:08 -0700 Received: from SC-EXCH03.marvell.com (10.93.176.83) by SC-EXCH03.marvell.com (10.93.176.83) with Microsoft SMTP Server (TLS) id 15.0.1210.3; Tue, 31 Oct 2017 02:46:07 -0700 Received: from maili.marvell.com (10.93.176.43) by SC-EXCH03.marvell.com (10.93.176.83) with Microsoft SMTP Server id 15.0.1210.3 via Frontend Transport; Tue, 31 Oct 2017 02:46:07 -0700 Received: from localhost.marvell.com (gbhat-thinkpad-t430.marvell.com [10.31.130.81]) by maili.marvell.com (Postfix) with ESMTP id A99093F7041; Tue, 31 Oct 2017 02:46:07 -0700 (PDT) Received: from localhost.marvell.com (localhost [127.0.0.1]) by localhost.marvell.com (8.14.4/8.14.4/Debian-4.1ubuntu1) with ESMTP id v9V9h9KV014196; Tue, 31 Oct 2017 15:13:09 +0530 Received: (from root@localhost) by localhost.marvell.com (8.14.4/8.14.4/Submit) id v9V9h9A4014195; Tue, 31 Oct 2017 15:13:09 +0530 From: Ganapathi Bhat To: CC: Brian Norris , Cathy Luo , Xinming Hu , Zhiyuan Yang , James Cao , Mangesh Malusare , Douglas Anderson , Karthik Ananthapadmanabha , Ganapathi Bhat Subject: [PATCH 2/3] mwifiex: cleanup tx_ba_stream_tbl_lock usage Date: Tue, 31 Oct 2017 15:12:46 +0530 Message-ID: <1509442967-14149-3-git-send-email-gbhat@marvell.com> X-Mailer: git-send-email 1.9.1 In-Reply-To: <1509442967-14149-1-git-send-email-gbhat@marvell.com> References: <1509442967-14149-1-git-send-email-gbhat@marvell.com> MIME-Version: 1.0 X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10432:, , definitions=2017-10-31_03:, , signatures=0 X-Proofpoint-Details: rule=outbound_notspam policy=outbound score=0 spamscore=0 suspectscore=1 malwarescore=0 lowpriorityscore=0 phishscore=0 adultscore=0 bulkscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.0.1-1707230000 definitions=main-1710310136 Sender: linux-wireless-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-wireless@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP From: Karthik Ananthapadmanabha Fix the incorrect usage of tx_ba_stream_tbl_lock spinlock: 1. We shouldn't access the fields of the elements returned by mwifiex_get_ba_status() and mwifiex_get_ba_tbl(), after we have released the spin lock. To fix this, aquire the spinlock before calling these functions and release the lock only after processing the correspnding element is complete. 2. Move tx_ba_stream_tbl_lock out of mwifiex_del_ba_tbl(), to avoid recursive spinlock. Acquire the spinlock explicitly, before calling mwifiex_del_ba_tbl(). Signed-off-by: Karthik Ananthapadmanabha Signed-off-by: Ganapathi Bhat --- drivers/net/wireless/marvell/mwifiex/11n.c | 45 +++++++++++++--------- .../net/wireless/marvell/mwifiex/11n_rxreorder.c | 3 -- 2 files changed, 26 insertions(+), 22 deletions(-) diff --git a/drivers/net/wireless/marvell/mwifiex/11n.c b/drivers/net/wireless/marvell/mwifiex/11n.c index 8772e39..38d7b48 100644 --- a/drivers/net/wireless/marvell/mwifiex/11n.c +++ b/drivers/net/wireless/marvell/mwifiex/11n.c @@ -77,24 +77,19 @@ int mwifiex_fill_cap_info(struct mwifiex_private *priv, u8 radio_type, /* * This function returns the pointer to an entry in BA Stream - * table which matches the requested BA status. + * table which matches the requested BA status. The caller + * must hold tx_ba_stream_tbl_lock before calling this function. */ static struct mwifiex_tx_ba_stream_tbl * mwifiex_get_ba_status(struct mwifiex_private *priv, enum mwifiex_ba_status ba_status) { struct mwifiex_tx_ba_stream_tbl *tx_ba_tsr_tbl; - unsigned long flags; - spin_lock_irqsave(&priv->tx_ba_stream_tbl_lock, flags); list_for_each_entry(tx_ba_tsr_tbl, &priv->tx_ba_stream_tbl_ptr, list) { - if (tx_ba_tsr_tbl->ba_status == ba_status) { - spin_unlock_irqrestore(&priv->tx_ba_stream_tbl_lock, - flags); + if (tx_ba_tsr_tbl->ba_status == ba_status) return tx_ba_tsr_tbl; - } } - spin_unlock_irqrestore(&priv->tx_ba_stream_tbl_lock, flags); return NULL; } @@ -114,9 +109,11 @@ int mwifiex_ret_11n_delba(struct mwifiex_private *priv, struct mwifiex_tx_ba_stream_tbl *tx_ba_tbl; struct host_cmd_ds_11n_delba *del_ba = &resp->params.del_ba; uint16_t del_ba_param_set = le16_to_cpu(del_ba->del_ba_param_set); + unsigned long flags; tid = del_ba_param_set >> DELBA_TID_POS; if (del_ba->del_result == BA_RESULT_SUCCESS) { + spin_lock_irqsave(&priv->tx_ba_stream_tbl_lock, flags); mwifiex_del_ba_tbl(priv, tid, del_ba->peer_mac_addr, TYPE_DELBA_SENT, INITIATOR_BIT(del_ba_param_set)); @@ -125,6 +122,7 @@ int mwifiex_ret_11n_delba(struct mwifiex_private *priv, if (tx_ba_tbl) mwifiex_send_addba(priv, tx_ba_tbl->tid, tx_ba_tbl->ra); + spin_unlock_irqrestore(&priv->tx_ba_stream_tbl_lock, flags); } else { /* * In case of failure, recreate the deleted stream in case * we initiated the ADDBA @@ -135,11 +133,13 @@ int mwifiex_ret_11n_delba(struct mwifiex_private *priv, mwifiex_create_ba_tbl(priv, del_ba->peer_mac_addr, tid, BA_SETUP_INPROGRESS); + spin_lock_irqsave(&priv->tx_ba_stream_tbl_lock, flags); tx_ba_tbl = mwifiex_get_ba_status(priv, BA_SETUP_INPROGRESS); if (tx_ba_tbl) mwifiex_del_ba_tbl(priv, tx_ba_tbl->tid, tx_ba_tbl->ra, TYPE_DELBA_SENT, true); + spin_unlock_irqrestore(&priv->tx_ba_stream_tbl_lock, flags); } return 0; @@ -160,6 +160,7 @@ int mwifiex_ret_11n_addba_req(struct mwifiex_private *priv, struct host_cmd_ds_11n_addba_rsp *add_ba_rsp = &resp->params.add_ba_rsp; struct mwifiex_tx_ba_stream_tbl *tx_ba_tbl; struct mwifiex_ra_list_tbl *ra_list; + unsigned long flags; u16 block_ack_param_set = le16_to_cpu(add_ba_rsp->block_ack_param_set); add_ba_rsp->ssn = cpu_to_le16((le16_to_cpu(add_ba_rsp->ssn)) @@ -176,14 +177,17 @@ int mwifiex_ret_11n_addba_req(struct mwifiex_private *priv, ra_list->ba_status = BA_SETUP_NONE; ra_list->amsdu_in_ampdu = false; } + spin_lock_irqsave(&priv->tx_ba_stream_tbl_lock, flags); mwifiex_del_ba_tbl(priv, tid, add_ba_rsp->peer_mac_addr, TYPE_DELBA_SENT, true); + spin_unlock_irqrestore(&priv->tx_ba_stream_tbl_lock, flags); if (add_ba_rsp->add_rsp_result != BA_RESULT_TIMEOUT) priv->aggr_prio_tbl[tid].ampdu_ap = BA_STREAM_NOT_ALLOWED; return 0; } + spin_lock_irqsave(&priv->tx_ba_stream_tbl_lock, flags); tx_ba_tbl = mwifiex_get_ba_tbl(priv, tid, add_ba_rsp->peer_mac_addr); if (tx_ba_tbl) { mwifiex_dbg(priv->adapter, EVENT, "info: BA stream complete\n"); @@ -201,6 +205,7 @@ int mwifiex_ret_11n_addba_req(struct mwifiex_private *priv, } else { mwifiex_dbg(priv->adapter, ERROR, "BA stream not created\n"); } + spin_unlock_irqrestore(&priv->tx_ba_stream_tbl_lock, flags); return 0; } @@ -501,24 +506,20 @@ void mwifiex_11n_delete_all_tx_ba_stream_tbl(struct mwifiex_private *priv) /* * This function returns the pointer to an entry in BA Stream - * table which matches the given RA/TID pair. + * table which matches the given RA/TID pair. The caller must + * hold tx_ba_stream_tbl_lock before calling this function. */ struct mwifiex_tx_ba_stream_tbl * mwifiex_get_ba_tbl(struct mwifiex_private *priv, int tid, u8 *ra) { struct mwifiex_tx_ba_stream_tbl *tx_ba_tsr_tbl; - unsigned long flags; - spin_lock_irqsave(&priv->tx_ba_stream_tbl_lock, flags); list_for_each_entry(tx_ba_tsr_tbl, &priv->tx_ba_stream_tbl_ptr, list) { if (ether_addr_equal_unaligned(tx_ba_tsr_tbl->ra, ra) && - tx_ba_tsr_tbl->tid == tid) { - spin_unlock_irqrestore(&priv->tx_ba_stream_tbl_lock, - flags); + tx_ba_tsr_tbl->tid == tid) return tx_ba_tsr_tbl; - } } - spin_unlock_irqrestore(&priv->tx_ba_stream_tbl_lock, flags); + return NULL; } @@ -534,11 +535,15 @@ void mwifiex_create_ba_tbl(struct mwifiex_private *priv, u8 *ra, int tid, unsigned long flags; int tid_down; + spin_lock_irqsave(&priv->tx_ba_stream_tbl_lock, flags); if (!mwifiex_get_ba_tbl(priv, tid, ra)) { new_node = kzalloc(sizeof(struct mwifiex_tx_ba_stream_tbl), GFP_ATOMIC); - if (!new_node) + if (!new_node) { + spin_unlock_irqrestore(&priv->tx_ba_stream_tbl_lock, + flags); return; + } tid_down = mwifiex_wmm_downgrade_tid(priv, tid); ra_list = mwifiex_wmm_get_ralist_node(priv, tid_down, ra); @@ -552,10 +557,9 @@ void mwifiex_create_ba_tbl(struct mwifiex_private *priv, u8 *ra, int tid, new_node->ba_status = ba_status; memcpy(new_node->ra, ra, ETH_ALEN); - spin_lock_irqsave(&priv->tx_ba_stream_tbl_lock, flags); list_add_tail(&new_node->list, &priv->tx_ba_stream_tbl_ptr); - spin_unlock_irqrestore(&priv->tx_ba_stream_tbl_lock, flags); } + spin_unlock_irqrestore(&priv->tx_ba_stream_tbl_lock, flags); } /* @@ -680,11 +684,14 @@ void mwifiex_11n_delete_ba_stream(struct mwifiex_private *priv, u8 *del_ba) (struct host_cmd_ds_11n_delba *) del_ba; uint16_t del_ba_param_set = le16_to_cpu(cmd_del_ba->del_ba_param_set); int tid; + unsigned long flags; tid = del_ba_param_set >> DELBA_TID_POS; + spin_lock_irqsave(&priv->tx_ba_stream_tbl_lock, flags); mwifiex_del_ba_tbl(priv, tid, cmd_del_ba->peer_mac_addr, TYPE_DELBA_RECEIVE, INITIATOR_BIT(del_ba_param_set)); + spin_unlock_irqrestore(&priv->tx_ba_stream_tbl_lock, flags); } /* diff --git a/drivers/net/wireless/marvell/mwifiex/11n_rxreorder.c b/drivers/net/wireless/marvell/mwifiex/11n_rxreorder.c index 0149c4a..4631bc2 100644 --- a/drivers/net/wireless/marvell/mwifiex/11n_rxreorder.c +++ b/drivers/net/wireless/marvell/mwifiex/11n_rxreorder.c @@ -682,7 +682,6 @@ int mwifiex_11n_rx_reorder_pkt(struct mwifiex_private *priv, struct mwifiex_tx_ba_stream_tbl *ptx_tbl; struct mwifiex_ra_list_tbl *ra_list; u8 cleanup_rx_reorder_tbl; - unsigned long flags; int tid_down; if (type == TYPE_DELBA_RECEIVE) @@ -716,9 +715,7 @@ int mwifiex_11n_rx_reorder_pkt(struct mwifiex_private *priv, ra_list->amsdu_in_ampdu = false; ra_list->ba_status = BA_SETUP_NONE; } - spin_lock_irqsave(&priv->tx_ba_stream_tbl_lock, flags); mwifiex_11n_delete_tx_ba_stream_tbl_entry(priv, ptx_tbl); - spin_unlock_irqrestore(&priv->tx_ba_stream_tbl_lock, flags); } }