linux-kernel - [PATCH 2/2] mwifiex: resolve reset vs. remove()/shutdown() deadlocks

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Fri, 12 Jan 2018 13:08:37 -0800
From:   Brian Norris <briannorris@...omium.org>
To:     Ganapathi Bhat <gbhat@...vell.com>,
        Nishant Sarmukadam <nishants@...vell.com>,
        Kalle Valo <kvalo@...eaurora.org>,
        Amitkumar Karwar <amitkarwar@...il.com>
Cc:     <linux-kernel@...r.kernel.org>, linux-wireless@...r.kernel.org,
        Ellie Reeves <ellierevves@...il.com>,
        Christoph Hellwig <hch@....de>,
        Dmitry Torokhov <dmitry.torokhov@...il.com>,
        Zhiyuan Yang <yangzy@...vell.com>,
        Tim Song <songtao@...vell.com>, Cathy Luo <cluo@...vell.com>,
        James Cao <jcao@...vell.com>,
        Brian Norris <briannorris@...omium.org>,
        stable@...r.kernel.org, Xinming Hu <huxm@...vell.com>
Subject: [PATCH 2/2] mwifiex: resolve reset vs. remove()/shutdown() deadlocks

Commit b014e96d1abb ("PCI: Protect pci_error_handlers->reset_notify()
usage with device_lock()") resolves races between driver reset and
removal, but it introduces some new deadlock problems. If we see a
timeout while we've already started suspending, removing, or shutting
down the driver, we might see:

(a) a worker thread, running mwifiex_pcie_work() ->
    mwifiex_pcie_card_reset_work() -> pci_reset_function()
(b) a removal thread, running mwifiex_pcie_remove() ->
    mwifiex_free_adapter() -> mwifiex_unregister() ->
    mwifiex_cleanup_pcie() -> cancel_work_sync(&card->work)

Unfortunately, mwifiex_pcie_remove() already holds the device lock that
pci_reset_function() is now requesting, and so we see a deadlock.

It's necessary to cancel and synchronize our outstanding work before
tearing down the driver, so we can't have this work wait indefinitely
for the lock.

It's reasonable to only "try" to reset here, since this will mostly
happen for cases where it's already difficult to reset the firmware
anyway (e.g., while we're suspending or powering off the system). And if
reset *really* needs to happen, we can always try again later.

Fixes: b014e96d1abb ("PCI: Protect pci_error_handlers->reset_notify() usage with device_lock()")
Cc: <stable@...r.kernel.org>
Cc: Xinming Hu <huxm@...vell.com>
Signed-off-by: Brian Norris <briannorris@...omium.org>
---
 drivers/net/wireless/marvell/mwifiex/pcie.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wireless/marvell/mwifiex/pcie.c b/drivers/net/wireless/marvell/mwifiex/pcie.c
index f666cb2ea7e0..97a6199692ab 100644
--- a/drivers/net/wireless/marvell/mwifiex/pcie.c
+++ b/drivers/net/wireless/marvell/mwifiex/pcie.c
@@ -2786,7 +2786,10 @@ static void mwifiex_pcie_card_reset_work(struct mwifiex_adapter *adapter)
 {
 	struct pcie_service_card *card = adapter->card;
 
-	pci_reset_function(card->dev);
+	/* We can't afford to wait here; remove() might be waiting on us. If we
+	 * can't grab the device lock, maybe we'll get another chance later.
+	 */
+	pci_try_reset_function(card->dev);
 }
 
 static void mwifiex_pcie_work(struct work_struct *work)
-- 
2.16.0.rc1.238.g530d649a79-goog