Received-SPF: pass (google.com: domain of linux-bluetooth-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Received-SPF: Pass (protection.outlook.com: domain of opensynergy.com
 designates 217.66.60.4 as permitted sender) receiver=protection.outlook.com;
 client-ip=217.66.60.4; helo=SR-MAIL-03.open-synergy.com; pr=C
From:   Vasyl Vavrychuk <vasyl.vavrychuk@opensynergy.com>
To:     linux-kernel@vger.kernel.org, netdev@vger.kernel.org,
        linux-bluetooth@vger.kernel.org,
        Max Krummenacher <max.oss.09@gmail.com>,
        Francesco Dolcini <francesco.dolcini@toradex.com>,
        =?UTF-8?q?Mateusz=20Jo=C5=84czyk?= <mat.jonczyk@o2.pl>,
        Jakub Kicinski <kuba@kernel.org>,
        Marcel Holtmann <marcel@holtmann.org>,
        max.krummenacher@toradex.com
Cc:     Johan Hedberg <johan.hedberg@gmail.com>,
        Luiz Augusto von Dentz <luiz.dentz@gmail.com>,
        "David S. Miller" <davem@davemloft.net>,
        Paolo Abeni <pabeni@redhat.com>,
        Eric Dumazet <edumazet@google.com>,
        Vasyl Vavrychuk <vasyl.vavrychuk@opensynergy.com>
Subject: [PATCH] Bluetooth: core: Fix deadlock due to `cancel_work_sync(&hdev->power_on)` from hci_power_on_sync.
Date:   Tue,  5 Jul 2022 15:59:31 +0300
Message-Id: <20220705125931.3601-1-vasyl.vavrychuk@opensynergy.com>
In-Reply-To: <20220614181706.26513-1-max.oss.09@gmail.com>
References: <20220614181706.26513-1-max.oss.09@gmail.com>
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
Content-Type: text/plain
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 05 Jul 2022 13:00:02.3293
 (UTC)
X-MS-Exchange-CrossTenant-Network-Message-Id: 1d595ac2-6eb6-4ea3-3f30-08da5e864645
X-MS-Exchange-CrossTenant-Id: 800fae25-9b1b-4edc-993d-c939c4e84a64
X-MS-Exchange-CrossTenant-OriginalAttributedTenantConnectingIp: TenantId=800fae25-9b1b-4edc-993d-c939c4e84a64;Ip=[217.66.60.4];Helo=[SR-MAIL-03.open-synergy.com]
X-MS-Exchange-CrossTenant-AuthSource: AM6EUR05FT055.eop-eur05.prod.protection.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Anonymous
X-MS-Exchange-CrossTenant-FromEntityHeader: HybridOnPrem
X-MS-Exchange-Transport-CrossTenantHeadersStamped: PA4PR04MB7727
X-TM-AS-ERS: 104.47.0.55-0.0.0.0
X-TMASE-Version: StarCloud-1.3-9.0.1004-26996.007
X-TMASE-Result: 10--10.482100-4.000000
X-TMASE-MatchedRID: hDt2nT7GHlPoSitJVour/f7FEhWgo0y8iK5qg1cmsr+dkHRvBVgJZmfL
        KEW7qlsHqUirzUPTZU8RXOKWV2pRUD3rUpOWwerc/kFj+75tvQwd7wYwkPJ/mh8DRANMiqtikrI
        9/WPu3jf68qduCW2JnM/NY1zQKKD4skLtspCGs70zOazjYfBb8Y0KoZnZ6F+XsMZG2pUzAfOyuh
        piqaRnanAm0uKJEVsP5erpc6fZVyrfytYpCSLCzr9A3Bl1/DcVUmlK22ARBaHCkJHfGKxhL7+Ih
        mgj6NxWiAaG2YvsCOK12HagvbwDji/7QU2czuUNA9lly13c/gFHyz3bB5kG50z5vzLEGq8DSebk
        8lenLCkkrfB4Sxx5OjKVinSQfz+2RGPq1SoWksB+yskgwrfsC30tCKdnhB58FMkUvzgg/cUqzDt
        2+ewmfO0d84ZRbF3AIAcCikR3vq/ZKrIEyWvT574TNGUN4s+AaKip7hpHodOcWpgypYPMbtdt5+
        WoqbOB
X-TMASE-XGENCLOUD: c53241e7-22e2-4b84-95f7-0e1209b093b6-0-0-200-0
X-TM-Deliver-Signature: 8AE3E450CD76EE7D70593E6C79C1DA51
DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=opensynergy.com;
        s=TM-DKIM-20210503141657; t=1657026008;
        bh=/mzkp2obN17YX5X2aYRTHkUYlkv9Lt075UpXK6TxCjo=; l=3043;
        h=From:To:Date;
        b=v8uQMikK0XKPHj2iHngFpC7c7Xrmb3DRgVP2ad8XJudMOk7z/shAMZSLeijq4dSby
         C3tZJzxHtVumf9nqr9d/u8VjW712PODZY0MIBWBxRqeNsNoTnVSGuOstAEu+iaQlKf
         qDWFlsq3e4RYrjczxtIStNk8gM/yc89H/W+Hnix/rTY0Z9W7lyhn51W9mJv5cGMBDl
         ak32o3lt5xD/zjbkPCFb+Sl+2qcsgEtG4xbHD2xHFyhpe/ivTc+ewnoZ9NlrxinJax
         sd7TfuvcUryaFus8K66+erkXKOCp/tozHDOrVL9kbJ/W8S48cteW3fldyLO87obN49
         EIuzg81MBRXuw==
X-Spam-Status: No, score=-2.8 required=5.0 tests=BAYES_00,DKIM_SIGNED,
        DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_LOW,SPF_HELO_PASS,
        SPF_PASS,T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no
        version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-bluetooth.vger.kernel.org>
X-Mailing-List: linux-bluetooth@vger.kernel.org

`cancel_work_sync(&hdev->power_on)` was moved to hci_dev_close_sync in
commit [1] to ensure that power_on work is canceled after HCI interface
down.

But, in certain cases power_on work function may call hci_dev_close_sync
itself: hci_power_on -> hci_dev_do_close -> hci_dev_close_sync ->
cancel_work_sync(&hdev->power_on), causing deadlock. In particular, this
happens when device is rfkilled on boot. To avoid deadlock, move
power_on work canceling out of hci_dev_do_close/hci_dev_close_sync.

Deadlock introduced by commit [1] was reported in [2,3] as broken
suspend. Suspend did not work because `hdev->req_lock` held as result of
`power_on` work deadlock. In fact, other BT features were not working.
It was not observed when testing [1] since it was verified without
rfkill in place.

NOTE: It is not needed to cancel power_on work from other places where
hci_dev_do_close/hci_dev_close_sync is called in case:
* Requests were serialized due to `hdev->req_workqueue`. The power_on
work is first in that workqueue.
* hci_rfkill_set_block which won't close device anyway until HCI_SETUP
is on.
* hci_sock_release which runs after hci_sock_bind which ensures
HCI_SETUP was cleared.

As result, behaviour is the same as in pre-dd06ed7 commit, except
power_on work cancel added to hci_dev_close.

[1]: commit dd06ed7ad057 ("Bluetooth: core: Fix missing power_on work cancel on HCI close")
[2]: https://lore.kernel.org/lkml/20220614181706.26513-1-max.oss.09@gmail.com/
[2]: https://lore.kernel.org/lkml/1236061d-95dd-c3ad-a38f-2dae7aae51ef@o2.pl/

Fixes: commit dd06ed7ad057 ("Bluetooth: core: Fix missing power_on work cancel on HCI close")
Signed-off-by: Vasyl Vavrychuk <vasyl.vavrychuk@opensynergy.com>
Reported-by: Max Krummenacher <max.krummenacher@toradex.com>
Reported-by: Mateusz Jonczyk <mat.jonczyk@o2.pl>
---
 net/bluetooth/hci_core.c | 3 +++
 net/bluetooth/hci_sync.c | 1 -
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index 59a5c1341c26..a0f99baafd35 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -571,6 +571,7 @@ int hci_dev_close(__u16 dev)
 		goto done;
 	}
 
+	cancel_work_sync(&hdev->power_on);
 	if (hci_dev_test_and_clear_flag(hdev, HCI_AUTO_OFF))
 		cancel_delayed_work(&hdev->power_off);
 
@@ -2675,6 +2676,8 @@ void hci_unregister_dev(struct hci_dev *hdev)
 	list_del(&hdev->list);
 	write_unlock(&hci_dev_list_lock);
 
+	cancel_work_sync(&hdev->power_on);
+
 	hci_cmd_sync_clear(hdev);
 
 	if (!test_bit(HCI_QUIRK_NO_SUSPEND_NOTIFIER, &hdev->quirks))
diff --git a/net/bluetooth/hci_sync.c b/net/bluetooth/hci_sync.c
index 286d6767f017..1739e8cb3291 100644
--- a/net/bluetooth/hci_sync.c
+++ b/net/bluetooth/hci_sync.c
@@ -4088,7 +4088,6 @@ int hci_dev_close_sync(struct hci_dev *hdev)
 
 	bt_dev_dbg(hdev, "");
 
-	cancel_work_sync(&hdev->power_on);
 	cancel_delayed_work(&hdev->power_off);
 	cancel_delayed_work(&hdev->ncmd_timer);
 
-- 
2.30.2