5.15-stable review patch. If anyone has any objections, please let me know.
------------------
From: Pauli Virtanen <redacted>
[ Upstream commit 9707a015fe8f3ba8ec7c270f3b2b8efb38823d6b ]
6lowpan.c has theoretically conflicting lock orderings, which lockdep
complains about:
a) rtnl_lock > hdev->workqueue
from 6lowpan.c:delete_netdev -> rtnl_lock -> device_del
-> put_device(parent) -> hci_release_dev -> destroy_workqueue
b) hdev->workqueue > l2cap_conn->lock > chan->lock > rtnl_lock
from hci_rx_work -> 6lowpan.c:chan_ready_cb
-> lowpan_register_netdev, ifup -> rtnl_lock
Actual deadlock appears not possible, as hci_rx_work is disabled and
l2cap_conn flushed already on hdev unregister. Hence, do minimal thing
to make lockdep happy by breaking chain a) by holding hdev refcount
until after netdev put in 6lowpan.c.
Fixes the lockdep complaint:
WARNING: possible circular locking dependency detected.
kworker/0:1/11 is trying to acquire lock:
ffff8880023b3940 ((wq_completion)hci0#2){+.+.}-{0:0}, at: touch_wq_lockdep_map+0x8b/0x130
but task is already holding lock:
ffffffff95e4f9c0 (rtnl_mutex){+.+.}-{4:4}, at: lowpan_unregister_netdev+0xd/0x30
Workqueue: events delete_netdev
Signed-off-by: Pauli Virtanen <redacted>
Signed-off-by: Luiz Augusto von Dentz <redacted>
Stable-dep-of: d4bfa78fd679 ("Bluetooth: L2CAP: reject accept queue add unless BT_LISTEN")
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
net/bluetooth/6lowpan.c | 25 +++++++++++++++++++++++--
1 file changed, 23 insertions(+), 2 deletions(-)
diff --git a/net/bluetooth/6lowpan.c b/net/bluetooth/6lowpan.c
index d0b1882f5b4b7..72306b8d9c290 100644
--- a/net/bluetooth/6lowpan.c
+++ b/net/bluetooth/6lowpan.c
@@ -758,13 +758,33 @@ static inline struct l2cap_chan *chan_new_conn_cb(struct l2cap_chan *pchan)
return chan;
}
+static void unregister_dev(struct lowpan_btle_dev *dev)
+{
+ struct hci_dev *hdev = READ_ONCE(dev->hdev);
+
+ /* If netdev holds last reference to hci_dev (its parent device), this
+ * leads to theoretical cyclic locking on lowpan_unregister_netdev:
+ *
+ * rtnl_lock -> put_device(parent) -> hci_release_dev ->
+ * destroy_workqueue -> hci_rx_work -> l2cap_recv_acldata ->
+ * chan_ready_cb -> ifup -> rtnl_lock
+ *
+ * However, hci_rx_work is disabled in hci_unregister_dev, so this
+ * should not occur. Make lockdep happy by postponing hdev release after
+ * netdev put.
+ */
+ hci_dev_hold(hdev);
+ lowpan_unregister_netdev(dev->netdev);
+ hci_dev_put(hdev);
+}
+
static void delete_netdev(struct work_struct *work)
{
struct lowpan_btle_dev *entry = container_of(work,
struct lowpan_btle_dev,
delete_netdev);
- lowpan_unregister_netdev(entry->netdev);
+ unregister_dev(entry);
/* The entry pointer is deleted by the netdev destructor. */
}@@ -1232,6 +1252,7 @@ static void disconnect_devices(void)
break;
new_dev->netdev = entry->netdev;
+ new_dev->hdev = entry->hdev;
INIT_LIST_HEAD(&new_dev->list);
list_add_rcu(&new_dev->list, &devices);
@@ -1243,7 +1264,7 @@ static void disconnect_devices(void)
ifdown(entry->netdev);
BT_DBG("Unregistering netdev %s %p",
entry->netdev->name, entry->netdev);
- lowpan_unregister_netdev(entry->netdev);
+ unregister_dev(entry);
kfree(entry);
}
}--
2.53.0