From patchwork Tue Mar 1 17:18:14 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Maya Erez X-Patchwork-Id: 8467731 X-Patchwork-Delegate: kvalo@adurom.com Return-Path: X-Original-To: patchwork-linux-wireless@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork1.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork1.web.kernel.org (Postfix) with ESMTP id 1D4329F2F0 for ; Tue, 1 Mar 2016 17:18:51 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 08A512014A for ; Tue, 1 Mar 2016 17:18:50 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id DD3BD202FF for ; Tue, 1 Mar 2016 17:18:48 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752940AbcCARSp (ORCPT ); Tue, 1 Mar 2016 12:18:45 -0500 Received: from wolverine02.qualcomm.com ([199.106.114.251]:31443 "EHLO wolverine02.qualcomm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752895AbcCARSn (ORCPT ); Tue, 1 Mar 2016 12:18:43 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=qca.qualcomm.com; i=@qca.qualcomm.com; q=dns/txt; s=qcdkim; t=1456852723; x=1488388723; h=from:to:cc:subject:date:message-id:in-reply-to: references; bh=bD+VWGsQhDxk4H9yk4nWUkMSC2jlGnybdOOLwMHCDMo=; b=Eg5TIYhRnI3Fvr7I/fhXCVQNeR0dLpACIDI/4ZRsc98NI1Bvxx0M39Jr RD5eCiWiRZkPGMdnDla4zKCN+Iu6Nk9Rnbphmvs/c4Zbe94yqiZNPYPbq +HWnc/zkGlUMuXN2WVE8gO1qGjSLsVRXOEIsETVw+wV7CiIohpL1JYisZ I=; X-IronPort-AV: E=Sophos;i="5.22,524,1449561600"; d="scan'208";a="267574475" Received: from ironmsg02-l-new.qualcomm.com (HELO ironmsg02-L.qualcomm.com) ([10.53.140.109]) by wolverine02.qualcomm.com with ESMTP/TLS/DHE-RSA-AES256-SHA; 01 Mar 2016 09:18:43 -0800 X-IronPort-AV: E=McAfee;i="5700,7163,8090"; a="648104162" Received: from lx-merez.mea.qualcomm.com ([10.18.177.171]) by ironmsg02-L.qualcomm.com with ESMTP; 01 Mar 2016 09:18:41 -0800 From: Maya Erez To: Kalle Valo Cc: Lior David , linux-wireless@vger.kernel.org, wil6210@qca.qualcomm.com, Maya Erez Subject: [PATCH v2 11/15] wil6210: fix race conditions in p2p listen and search Date: Tue, 1 Mar 2016 19:18:14 +0200 Message-Id: <1456852698-26808-12-git-send-email-qca_merez@qca.qualcomm.com> X-Mailer: git-send-email 1.8.5.2 In-Reply-To: <1456852698-26808-1-git-send-email-qca_merez@qca.qualcomm.com> References: <1456852698-26808-1-git-send-email-qca_merez@qca.qualcomm.com> Sender: linux-wireless-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-wireless@vger.kernel.org X-Spam-Status: No, score=-6.8 required=5.0 tests=BAYES_00,DKIM_SIGNED, RCVD_IN_DNSWL_HI,RP_MATCHES_RCVD,T_DKIM_INVALID,UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP From: Lior David Fix 2 race conditions found during test runs of P2P discovery: 1. Because wil_p2p_cancel_listen was not protected, user space could start a new P2P listen/search before wmi_stop_discovery completed. This caused a crash in the firmware. 2. In P2P listen, when listen timer expires and user space calls cancel_remain_on_channel at the same time, code could send the cfg80211_remain_on_channel_expired notification twice. Added protections with wil->mutex to several places that call wmi_stop_discovery. Signed-off-by: Lior David Signed-off-by: Maya Erez --- drivers/net/wireless/ath/wil6210/cfg80211.c | 9 +++-- drivers/net/wireless/ath/wil6210/main.c | 2 +- drivers/net/wireless/ath/wil6210/p2p.c | 63 ++++++++++++++++++++--------- drivers/net/wireless/ath/wil6210/wil6210.h | 4 +- 4 files changed, 53 insertions(+), 25 deletions(-) diff --git a/drivers/net/wireless/ath/wil6210/cfg80211.c b/drivers/net/wireless/ath/wil6210/cfg80211.c index 24f9829..e867c76 100644 --- a/drivers/net/wireless/ath/wil6210/cfg80211.c +++ b/drivers/net/wireless/ath/wil6210/cfg80211.c @@ -387,7 +387,7 @@ static int wil_cfg80211_scan(struct wiphy *wiphy, return rc; } - wil_p2p_stop_discovery(wil); + (void)wil_p2p_stop_discovery(wil); wil_dbg_misc(wil, "Start scan_request 0x%p\n", request); wil_dbg_misc(wil, "SSID count: %d", request->n_ssids); @@ -868,6 +868,9 @@ static int wil_cfg80211_set_default_key(struct wiphy *wiphy, u8 key_index, bool unicast, bool multicast) { + struct wil6210_priv *wil = wiphy_to_wil(wiphy); + + wil_dbg_misc(wil, "%s: entered\n", __func__); return 0; } @@ -903,9 +906,7 @@ static int wil_cancel_remain_on_channel(struct wiphy *wiphy, wil_dbg_misc(wil, "%s()\n", __func__); - wil_p2p_cancel_listen(wil, cookie); - - return 0; + return wil_p2p_cancel_listen(wil, cookie); } /** diff --git a/drivers/net/wireless/ath/wil6210/main.c b/drivers/net/wireless/ath/wil6210/main.c index e09e9bb..05a4ae7 100644 --- a/drivers/net/wireless/ath/wil6210/main.c +++ b/drivers/net/wireless/ath/wil6210/main.c @@ -984,7 +984,7 @@ int __wil_down(struct wil6210_priv *wil) } wil_enable_irq(wil); - wil_p2p_stop_discovery(wil); + (void)wil_p2p_stop_discovery(wil); if (wil->scan_request) { wil_dbg_misc(wil, "Abort scan_request 0x%p\n", diff --git a/drivers/net/wireless/ath/wil6210/p2p.c b/drivers/net/wireless/ath/wil6210/p2p.c index d223648..2c1b895 100644 --- a/drivers/net/wireless/ath/wil6210/p2p.c +++ b/drivers/net/wireless/ath/wil6210/p2p.c @@ -156,26 +156,42 @@ out: return rc; } -void wil_p2p_stop_discovery(struct wil6210_priv *wil) +u8 wil_p2p_stop_discovery(struct wil6210_priv *wil) { struct wil_p2p_info *p2p = &wil->p2p; + u8 started = p2p->discovery_started; if (p2p->discovery_started) { del_timer_sync(&p2p->discovery_timer); p2p->discovery_started = 0; wmi_stop_discovery(wil); } + + return started; } -void wil_p2p_cancel_listen(struct wil6210_priv *wil, u64 cookie) +int wil_p2p_cancel_listen(struct wil6210_priv *wil, u64 cookie) { struct wil_p2p_info *p2p = &wil->p2p; + u8 started; + + mutex_lock(&wil->mutex); - if (cookie != p2p->cookie) + if (cookie != p2p->cookie) { wil_info(wil, "%s: Cookie mismatch: 0x%016llx vs. 0x%016llx\n", __func__, p2p->cookie, cookie); + mutex_unlock(&wil->mutex); + return -ENOENT; + } + + started = wil_p2p_stop_discovery(wil); + + mutex_unlock(&wil->mutex); - wil_p2p_stop_discovery(wil); + if (!started) { + wil_err(wil, "%s: listen not started\n", __func__); + return -ENOENT; + } mutex_lock(&wil->p2p_wdev_mutex); cfg80211_remain_on_channel_expired(wil->radio_wdev, @@ -184,6 +200,7 @@ void wil_p2p_cancel_listen(struct wil6210_priv *wil, u64 cookie) GFP_KERNEL); wil->radio_wdev = wil->wdev; mutex_unlock(&wil->p2p_wdev_mutex); + return 0; } void wil_p2p_listen_expired(struct work_struct *work) @@ -192,18 +209,23 @@ void wil_p2p_listen_expired(struct work_struct *work) struct wil_p2p_info, discovery_expired_work); struct wil6210_priv *wil = container_of(p2p, struct wil6210_priv, p2p); + u8 started; wil_dbg_misc(wil, "%s()\n", __func__); - wil_p2p_stop_discovery(wil); + mutex_lock(&wil->mutex); + started = wil_p2p_stop_discovery(wil); + mutex_unlock(&wil->mutex); - mutex_lock(&wil->p2p_wdev_mutex); - cfg80211_remain_on_channel_expired(wil->radio_wdev, - p2p->cookie, - &p2p->listen_chan, - GFP_KERNEL); - wil->radio_wdev = wil->wdev; - mutex_unlock(&wil->p2p_wdev_mutex); + if (started) { + mutex_lock(&wil->p2p_wdev_mutex); + cfg80211_remain_on_channel_expired(wil->radio_wdev, + p2p->cookie, + &p2p->listen_chan, + GFP_KERNEL); + wil->radio_wdev = wil->wdev; + mutex_unlock(&wil->p2p_wdev_mutex); + } } @@ -213,14 +235,19 @@ void wil_p2p_search_expired(struct work_struct *work) struct wil_p2p_info, discovery_expired_work); struct wil6210_priv *wil = container_of(p2p, struct wil6210_priv, p2p); + u8 started; wil_dbg_misc(wil, "%s()\n", __func__); - wil_p2p_stop_discovery(wil); + mutex_lock(&wil->mutex); + started = wil_p2p_stop_discovery(wil); + mutex_unlock(&wil->mutex); - mutex_lock(&wil->p2p_wdev_mutex); - cfg80211_scan_done(wil->scan_request, 0); - wil->scan_request = NULL; - wil->radio_wdev = wil->wdev; - mutex_unlock(&wil->p2p_wdev_mutex); + if (started) { + mutex_lock(&wil->p2p_wdev_mutex); + cfg80211_scan_done(wil->scan_request, 0); + wil->scan_request = NULL; + wil->radio_wdev = wil->wdev; + mutex_unlock(&wil->p2p_wdev_mutex); + } } diff --git a/drivers/net/wireless/ath/wil6210/wil6210.h b/drivers/net/wireless/ath/wil6210/wil6210.h index 9b77a08..68f60ea 100644 --- a/drivers/net/wireless/ath/wil6210/wil6210.h +++ b/drivers/net/wireless/ath/wil6210/wil6210.h @@ -775,8 +775,8 @@ int wil_p2p_search(struct wil6210_priv *wil, struct cfg80211_scan_request *request); int wil_p2p_listen(struct wil6210_priv *wil, unsigned int duration, struct ieee80211_channel *chan, u64 *cookie); -void wil_p2p_stop_discovery(struct wil6210_priv *wil); -void wil_p2p_cancel_listen(struct wil6210_priv *wil, u64 cookie); +u8 wil_p2p_stop_discovery(struct wil6210_priv *wil); +int wil_p2p_cancel_listen(struct wil6210_priv *wil, u64 cookie); void wil_p2p_listen_expired(struct work_struct *work); void wil_p2p_search_expired(struct work_struct *work);