netdev - [PATCH] Revert "Bluetooth: Shutdown controller after workqueues are flushed or cancelled"

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-next>] [day] [month] [year] [list]

Message-Id: <20210816231150.1478727-1-dmitry.baryshkov@linaro.org>
Date:   Tue, 17 Aug 2021 02:11:50 +0300
From:   Dmitry Baryshkov <dmitry.baryshkov@...aro.org>
To:     Marcel Holtmann <marcel@...tmann.org>,
        Johan Hedberg <johan.hedberg@...il.com>,
        Luiz Augusto von Dentz <luiz.dentz@...il.com>
Cc:     "David S. Miller" <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>,
        linux-bluetooth@...r.kernel.org, netdev@...r.kernel.org,
        Kai-Heng Feng <kai.heng.feng@...onical.com>
Subject: [PATCH] Revert "Bluetooth: Shutdown controller after workqueues are flushed or cancelled"

This reverts commit 0ea9fd001a14ebc294f112b0361a4e601551d508. It moved
calling shutdown callback after flushing the queues. In doing so it
disabled calling the shutdown hook completely: shutdown condition
tests for HCI_UP in hdev->flags, which gets cleared now before checking
this condition (see test_and_clear_bit(HCI_UP, ...) call). Thus shutdown
hook was never called.

This would not be a problem itself and could fixed with just removing
the HCI_UP condition (since if we are this point, we already know that
the HCI device was up before calling hci_dev_do_close(). However the
fact that shutdown hook was not called hid the fact that it is not
proper to call shutdown hook so late in the sequence. The hook would
usually call __hci_cmd_sync()/__hci_cmd_sync_ev(), which would timeout
without running queues.

Thus I think it is more proper at this moment to revert the commit and
look for a better solution.

Fixes: 0ea9fd001a14 ("Bluetooth: Shutdown controller after workqueues are flushed or cancelled")
Cc: Kai-Heng Feng <kai.heng.feng@...onical.com>
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@...aro.org>
---
 net/bluetooth/hci_core.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index e1a545c8a69f..677d880068a4 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -1721,6 +1721,14 @@ int hci_dev_do_close(struct hci_dev *hdev)

 	BT_DBG("%s %p", hdev->name, hdev);

+	if (!hci_dev_test_flag(hdev, HCI_UNREGISTER) &&
+	    !hci_dev_test_flag(hdev, HCI_USER_CHANNEL) &&
+	    test_bit(HCI_UP, &hdev->flags)) {
+		/* Execute vendor specific shutdown routine */
+		if (hdev->shutdown)
+			hdev->shutdown(hdev);
+	}
+
 	cancel_delayed_work(&hdev->power_off);
 	cancel_delayed_work(&hdev->ncmd_timer);

@@ -1798,14 +1806,6 @@ int hci_dev_do_close(struct hci_dev *hdev)
 		clear_bit(HCI_INIT, &hdev->flags);
 	}

-	if (!hci_dev_test_flag(hdev, HCI_UNREGISTER) &&
-	    !hci_dev_test_flag(hdev, HCI_USER_CHANNEL) &&
-	    test_bit(HCI_UP, &hdev->flags)) {
-		/* Execute vendor specific shutdown routine */
-		if (hdev->shutdown)
-			hdev->shutdown(hdev);
-	}
-
 	/* flush cmd  work */
 	flush_work(&hdev->cmd_work);

-- 
2.30.2