FAILED: patch "[PATCH] Bluetooth: defer cleanup of resources in hci_unregister_dev()" failed to apply to 4.14-stable tree
Subsystems (2)
[email protected]
maintainerThe patch below does not apply to the 4.14-stable tree.
If someone wants it applied there, or to any other stable or longterm tree, then please email the backport, including the original git commit id to <[email protected]>.
thanks,
greg k-h
------------------ original commit in Linus's tree ------------------
From 58ce6d5b271ab25fb2056f84a8e5546945eb5fc9 Mon Sep 17 00:00:00 2001
From: Tetsuo Handa <[email protected]>
Date: Tue, 27 Jul 2021 06:12:04 +0900
Subject: [PATCH] Bluetooth: defer cleanup of resources in hci_unregister_dev()
syzbot is hitting might_sleep() warning at hci_sock_dev_event() due to calling lock_sock() with rw spinlock held [1].
It seems that history of this locking problem is a trial and error.
Commit b40df5743ee8aed8 ("[PATCH] bluetooth: fix socket locking in hci_sock_dev_event()") in 2.6.21-rc4 changed bh_lock_sock() to lock_sock() as an attempt to fix lockdep warning.
Then, commit 4ce61d1c7a8ef4c1 ("[BLUETOOTH]: Fix locking in hci_sock_dev_event().") in 2.6.22-rc2 changed lock_sock() to local_bh_disable() + bh_lock_sock_nested() as an attempt to fix sleep in atomic context warning.
Then, commit 4b5dd696f81b210c ("Bluetooth: Remove local_bh_disable() from hci_sock.c") in 3.3-rc1 removed local_bh_disable().
Then, commit e305509e678b3a4a ("Bluetooth: use correct lock to prevent UAF of hdev object") in 5.13-rc5 again changed bh_lock_sock_nested() to lock_sock() as an attempt to fix CVE-2021-3573.
This difficulty comes from current implementation that hci_sock_dev_event(HCI_DEV_UNREG) is responsible for dropping all references from sockets because hci_unregister_dev() immediately reclaims resources as soon as returning from hci_sock_dev_event(HCI_DEV_UNREG). But the history suggests that hci_sock_dev_event(HCI_DEV_UNREG) was not doing what it should do.
Therefore, instead of trying to detach sockets from device, let's accept not detaching sockets from device at hci_sock_dev_event(HCI_DEV_UNREG), by moving actual cleanup of resources from hci_unregister_dev() to hci_release_dev() which is called by bt_host_release when all references to this unregistered device (which is a kobject) are gone.
include/net/bluetooth/hci_core.h +1 -0
@@ -1230,6 +1230,7 @@ struct hci_dev *hci_alloc_dev(void);void hci_free_dev (struct hci_dev *hdev);int hci_register_dev (struct hci_dev *hdev);void hci_unregister_dev (struct hci_dev *hdev); +void hci_release_dev (struct hci_dev *hdev);int hci_suspend_dev (struct hci_dev *hdev);int hci_resume_dev (struct hci_dev *hdev);int hci_reset_dev (struct hci_dev *hdev);
net/bluetooth/hci_core.c +9 -8
@@ -3996,14 +3996,10 @@ EXPORT_SYMBOL(hci_register_dev);/* Unregister HCI device */ void hci_unregister_dev (struct hci_dev *hdev) { -int id; -BT_DBG ("%p name %s bus %d" , hdev, hdev->name, hdev->bus);hci_dev_set_flag (hdev,HCI_UNREGISTER ); - id = hdev->id; -write_lock (&hci_dev_list_lock);list_del (&hdev->list);write_unlock (&hci_dev_list_lock);
@@ -4038,7 +4034,13 @@ void hci_unregister_dev(struct hci_dev *hdev) }device_del (&hdev->dev); +hci_dev_put (hdev); +} +EXPORT_SYMBOL (hci_unregister_dev); +/* Release HCI device */ +void hci_release_dev (struct hci_dev *hdev) +{debugfs_remove_recursive (hdev->debugfs);kfree_const (hdev->hw_info);kfree_const (hdev->fw_info);
@@ -4063,11 +4065,10 @@ void hci_unregister_dev(struct hci_dev *hdev)hci_blocked_keys_clear (hdev);hci_dev_unlock (hdev); -hci_dev_put (hdev); - -ida_simple_remove (&hci_index_ida, id); +ida_simple_remove (&hci_index_ida, hdev->id); +kfree (hdev); } -EXPORT_SYMBOL (hci_unregister_dev); +EXPORT_SYMBOL (hci_release_dev);/* Suspend HCI device */ int hci_suspend_dev (struct hci_dev *hdev)
net/bluetooth/hci_sock.c +13 -7
@@ -759,19 +759,13 @@ void hci_sock_dev_event(struct hci_dev *hdev, int event)if (event ==HCI_DEV_UNREG ) {struct sock *sk; -/* Detach sockets from device */ +/* Wake up sockets using this dead device */ read_lock (&hci_sk_list.lock);sk_for_each (sk, &hci_sk_list.head) { -lock_sock (sk);if (hci_pi (sk)->hdev == hdev) { -hci_pi (sk)->hdev =NULL ; sk->sk_err =EPIPE ; - sk->sk_state =BT_OPEN ; sk->sk_state_change (sk); - -hci_dev_put (hdev); } -release_sock (sk); }read_unlock (&hci_sk_list.lock); }
@@ -1103,6 +1097,18 @@ static int hci_sock_bind(struct socket *sock, struct sockaddr *addr,lock_sock (sk); +/* Allow detaching from dead device and attaching to alive device, if +* the caller wants to re-bind (instead of close) this socket in +* response to hci_sock_dev_event(HCI_DEV_UNREG) notification. +*/ + hdev =hci_pi (sk)->hdev; +if (hdev &&hci_dev_test_flag (hdev,HCI_UNREGISTER )) { +hci_pi (sk)->hdev =NULL ; + sk->sk_state =BT_OPEN ; +hci_dev_put (hdev); + } + hdev =NULL ; +if (sk->sk_state ==BT_BOUND ) { err = -EALREADY ; goto done;
net/bluetooth/hci_sysfs.c +1 -1
@@ -83,7 +83,7 @@ void hci_conn_del_sysfs(struct hci_conn *conn)static void bt_host_release (struct device *dev) {struct hci_dev *hdev =to_hci_dev (dev); -kfree (hdev); +hci_release_dev (hdev);module_put (THIS_MODULE ); }