Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S933286AbdIXU7i (ORCPT ); Sun, 24 Sep 2017 16:59:38 -0400 Received: from mail.linuxfoundation.org ([140.211.169.12]:60508 "EHLO mail.linuxfoundation.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S933044AbdIXUlU (ORCPT ); Sun, 24 Sep 2017 16:41:20 -0400 From: Greg Kroah-Hartman To: linux-kernel@vger.kernel.org Cc: Greg Kroah-Hartman , stable@vger.kernel.org, Bjorn Andersson , Kalle Valo Subject: [PATCH 4.13 036/109] wcn36xx: Introduce mutual exclusion of fw configuration Date: Sun, 24 Sep 2017 22:32:57 +0200 Message-Id: <20170924203354.531730235@linuxfoundation.org> X-Mailer: git-send-email 2.14.1 In-Reply-To: <20170924203353.104695385@linuxfoundation.org> References: <20170924203353.104695385@linuxfoundation.org> User-Agent: quilt/0.65 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 8376 Lines: 274 4.13-stable review patch. If anyone has any objections, please let me know. ------------------ From: Bjorn Andersson commit 39efc7cc7ccf82d1cd946580cdb70760f347305a upstream. As the association status changes the driver needs to configure the hardware. This is done based on information in the "sta" acquired by ieee80211_find_sta(), which requires the caller to ensure that the "sta" is valid while its being used; generally by entering an rcu read section. But the operations acting on the "sta" has to communicate with the firmware and may therefor sleep, resulting in the following report: [ 31.418190] BUG: sleeping function called from invalid context at kernel/locking/mutex.c:238 [ 31.425919] in_atomic(): 0, irqs_disabled(): 0, pid: 34, name: kworker/u8:1 [ 31.434609] CPU: 0 PID: 34 Comm: kworker/u8:1 Tainted: G W 4.12.0-rc4-next-20170607+ #993 [ 31.441002] Hardware name: Qualcomm Technologies, Inc. APQ 8016 SBC (DT) [ 31.450380] Workqueue: phy0 ieee80211_iface_work [ 31.457226] Call trace: [ 31.461830] [] dump_backtrace+0x0/0x260 [ 31.464004] [] show_stack+0x14/0x20 [ 31.469557] [] dump_stack+0x98/0xb8 [ 31.474592] [] ___might_sleep+0xf0/0x118 [ 31.479626] [] __might_sleep+0x50/0x88 [ 31.485010] [] mutex_lock+0x24/0x60 [ 31.490479] [] wcn36xx_smd_set_link_st+0x30/0x130 [ 31.495428] [] wcn36xx_bss_info_changed+0x148/0x448 [ 31.501504] [] ieee80211_bss_info_change_notify+0xbc/0x118 [ 31.508102] [] ieee80211_assoc_success+0x664/0x7f8 [ 31.515220] [] ieee80211_rx_mgmt_assoc_resp+0x144/0x2d8 [ 31.521555] [] ieee80211_sta_rx_queued_mgmt+0x190/0x698 [ 31.528239] [] ieee80211_iface_work+0x234/0x368 [ 31.535011] [] process_one_work+0x1cc/0x340 [ 31.541086] [] worker_thread+0x48/0x430 [ 31.546814] [] kthread+0x108/0x138 [ 31.552195] [] ret_from_fork+0x10/0x50 In order to ensure that the "sta" remains alive (and consistent) for the duration of bss_info_changed() mutual exclusion has to be ensured with sta_remove(). This is done by introducing a mutex to cover firmware configuration changes, which is made to also ensure mutual exclusion between other operations changing the state or configuration of the firmware. With this we can drop the rcu read lock. Signed-off-by: Bjorn Andersson Signed-off-by: Kalle Valo Signed-off-by: Greg Kroah-Hartman --- drivers/net/wireless/ath/wcn36xx/main.c | 52 +++++++++++++++++++++++++++-- drivers/net/wireless/ath/wcn36xx/wcn36xx.h | 3 + 2 files changed, 53 insertions(+), 2 deletions(-) --- a/drivers/net/wireless/ath/wcn36xx/main.c +++ b/drivers/net/wireless/ath/wcn36xx/main.c @@ -372,6 +372,8 @@ static int wcn36xx_config(struct ieee802 wcn36xx_dbg(WCN36XX_DBG_MAC, "mac config changed 0x%08x\n", changed); + mutex_lock(&wcn->conf_mutex); + if (changed & IEEE80211_CONF_CHANGE_CHANNEL) { int ch = WCN36XX_HW_CHANNEL(wcn); wcn36xx_dbg(WCN36XX_DBG_MAC, "wcn36xx_config channel switch=%d\n", @@ -382,6 +384,8 @@ static int wcn36xx_config(struct ieee802 } } + mutex_unlock(&wcn->conf_mutex); + return 0; } @@ -396,6 +400,8 @@ static void wcn36xx_configure_filter(str wcn36xx_dbg(WCN36XX_DBG_MAC, "mac configure filter\n"); + mutex_lock(&wcn->conf_mutex); + *total &= FIF_ALLMULTI; fp = (void *)(unsigned long)multicast; @@ -408,6 +414,8 @@ static void wcn36xx_configure_filter(str else if (NL80211_IFTYPE_STATION == vif->type && tmp->sta_assoc) wcn36xx_smd_set_mc_list(wcn, vif, fp); } + + mutex_unlock(&wcn->conf_mutex); kfree(fp); } @@ -471,6 +479,8 @@ static int wcn36xx_set_key(struct ieee80 key_conf->key, key_conf->keylen); + mutex_lock(&wcn->conf_mutex); + switch (key_conf->cipher) { case WLAN_CIPHER_SUITE_WEP40: vif_priv->encrypt_type = WCN36XX_HAL_ED_WEP40; @@ -565,6 +575,8 @@ static int wcn36xx_set_key(struct ieee80 } out: + mutex_unlock(&wcn->conf_mutex); + return ret; } @@ -725,6 +737,8 @@ static void wcn36xx_bss_info_changed(str wcn36xx_dbg(WCN36XX_DBG_MAC, "mac bss info changed vif %p changed 0x%08x\n", vif, changed); + mutex_lock(&wcn->conf_mutex); + if (changed & BSS_CHANGED_BEACON_INFO) { wcn36xx_dbg(WCN36XX_DBG_MAC, "mac bss changed dtim period %d\n", @@ -787,7 +801,13 @@ static void wcn36xx_bss_info_changed(str bss_conf->aid); vif_priv->sta_assoc = true; - rcu_read_lock(); + + /* + * Holding conf_mutex ensures mutal exclusion with + * wcn36xx_sta_remove() and as such ensures that sta + * won't be freed while we're operating on it. As such + * we do not need to hold the rcu_read_lock(). + */ sta = ieee80211_find_sta(vif, bss_conf->bssid); if (!sta) { wcn36xx_err("sta %pM is not found\n", @@ -811,7 +831,6 @@ static void wcn36xx_bss_info_changed(str * place where AID is available. */ wcn36xx_smd_config_sta(wcn, vif, sta); - rcu_read_unlock(); } else { wcn36xx_dbg(WCN36XX_DBG_MAC, "disassociated bss %pM vif %pM AID=%d\n", @@ -873,6 +892,9 @@ static void wcn36xx_bss_info_changed(str } } out: + + mutex_unlock(&wcn->conf_mutex); + return; } @@ -882,7 +904,10 @@ static int wcn36xx_set_rts_threshold(str struct wcn36xx *wcn = hw->priv; wcn36xx_dbg(WCN36XX_DBG_MAC, "mac set RTS threshold %d\n", value); + mutex_lock(&wcn->conf_mutex); wcn36xx_smd_update_cfg(wcn, WCN36XX_HAL_CFG_RTS_THRESHOLD, value); + mutex_unlock(&wcn->conf_mutex); + return 0; } @@ -893,8 +918,12 @@ static void wcn36xx_remove_interface(str struct wcn36xx_vif *vif_priv = wcn36xx_vif_to_priv(vif); wcn36xx_dbg(WCN36XX_DBG_MAC, "mac remove interface vif %p\n", vif); + mutex_lock(&wcn->conf_mutex); + list_del(&vif_priv->list); wcn36xx_smd_delete_sta_self(wcn, vif->addr); + + mutex_unlock(&wcn->conf_mutex); } static int wcn36xx_add_interface(struct ieee80211_hw *hw, @@ -915,9 +944,13 @@ static int wcn36xx_add_interface(struct return -EOPNOTSUPP; } + mutex_lock(&wcn->conf_mutex); + list_add(&vif_priv->list, &wcn->vif_list); wcn36xx_smd_add_sta_self(wcn, vif); + mutex_unlock(&wcn->conf_mutex); + return 0; } @@ -930,6 +963,8 @@ static int wcn36xx_sta_add(struct ieee80 wcn36xx_dbg(WCN36XX_DBG_MAC, "mac sta add vif %p sta %pM\n", vif, sta->addr); + mutex_lock(&wcn->conf_mutex); + spin_lock_init(&sta_priv->ampdu_lock); sta_priv->vif = vif_priv; /* @@ -941,6 +976,9 @@ static int wcn36xx_sta_add(struct ieee80 sta_priv->aid = sta->aid; wcn36xx_smd_config_sta(wcn, vif, sta); } + + mutex_unlock(&wcn->conf_mutex); + return 0; } @@ -954,8 +992,13 @@ static int wcn36xx_sta_remove(struct iee wcn36xx_dbg(WCN36XX_DBG_MAC, "mac sta remove vif %p sta %pM index %d\n", vif, sta->addr, sta_priv->sta_index); + mutex_lock(&wcn->conf_mutex); + wcn36xx_smd_delete_sta(wcn, sta_priv->sta_index); sta_priv->vif = NULL; + + mutex_unlock(&wcn->conf_mutex); + return 0; } @@ -999,6 +1042,8 @@ static int wcn36xx_ampdu_action(struct i wcn36xx_dbg(WCN36XX_DBG_MAC, "mac ampdu action action %d tid %d\n", action, tid); + mutex_lock(&wcn->conf_mutex); + switch (action) { case IEEE80211_AMPDU_RX_START: sta_priv->tid = tid; @@ -1038,6 +1083,8 @@ static int wcn36xx_ampdu_action(struct i wcn36xx_err("Unknown AMPDU action\n"); } + mutex_unlock(&wcn->conf_mutex); + return 0; } @@ -1216,6 +1263,7 @@ static int wcn36xx_probe(struct platform wcn = hw->priv; wcn->hw = hw; wcn->dev = &pdev->dev; + mutex_init(&wcn->conf_mutex); mutex_init(&wcn->hal_mutex); mutex_init(&wcn->scan_lock); --- a/drivers/net/wireless/ath/wcn36xx/wcn36xx.h +++ b/drivers/net/wireless/ath/wcn36xx/wcn36xx.h @@ -202,6 +202,9 @@ struct wcn36xx { struct qcom_smem_state *tx_rings_empty_state; unsigned tx_rings_empty_state_bit; + /* prevents concurrent FW reconfiguration */ + struct mutex conf_mutex; + /* * smd_buf must be protected with smd_mutex to garantee * that all messages are sent one after another