From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To: linux-kernel@vger.kernel.org
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
stable@vger.kernel.org,
syzbot <syzbot+a5df189917e79d5e59c9@syzkaller.appspotmail.com>,
Linus Torvalds <torvalds@linux-foundation.org>,
Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>,
Luiz Augusto von Dentz <luiz.von.dentz@intel.com>,
Sasha Levin <sashal@kernel.org>
Subject: [PATCH 4.19 20/54] Bluetooth: defer cleanup of resources in hci_unregister_dev()
Date: Tue, 10 Aug 2021 19:30:14 +0200 [thread overview]
Message-ID: <20210810172944.842148853@linuxfoundation.org> (raw)
In-Reply-To: <20210810172944.179901509@linuxfoundation.org>
From: Tetsuo Handa <penguin-kernel@i-love.sakura.ne.jp>
[ Upstream commit e04480920d1eec9c061841399aa6f35b6f987d8b ]
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 b40df5743ee8 ("[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 4ce61d1c7a8e ("[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 the
sleep in atomic context warning.
Then, commit 4b5dd696f81b ("Bluetooth: Remove local_bh_disable() from
hci_sock.c") in 3.3-rc1 removed local_bh_disable().
Then, commit e305509e678b ("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_cleanup_dev() which is called by bt_host_release() when all
references to this unregistered device (which is a kobject) are gone.
Since hci_sock_dev_event(HCI_DEV_UNREG) no longer resets
hci_pi(sk)->hdev, we need to check whether this device was unregistered
and return an error based on HCI_UNREGISTER flag. There might be subtle
behavioral difference in "monitor the hdev" functionality; please report
if you found something went wrong due to this patch.
Link: https://syzkaller.appspot.com/bug?extid=a5df189917e79d5e59c9 [1]
Reported-by: syzbot <syzbot+a5df189917e79d5e59c9@syzkaller.appspotmail.com>
Suggested-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
Fixes: e305509e678b ("Bluetooth: use correct lock to prevent UAF of hdev object")
Acked-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
include/net/bluetooth/hci_core.h | 1 +
net/bluetooth/hci_core.c | 16 +++++------
net/bluetooth/hci_sock.c | 49 +++++++++++++++++++++-----------
net/bluetooth/hci_sysfs.c | 3 ++
4 files changed, 45 insertions(+), 24 deletions(-)
diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index 6a61faf0cc79..75d892dc7796 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -1042,6 +1042,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_cleanup_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);
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index 219cdbb476fb..7a85f215da45 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -3261,14 +3261,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);
@@ -3297,7 +3293,14 @@ void hci_unregister_dev(struct hci_dev *hdev)
}
device_del(&hdev->dev);
+ /* Actual cleanup is deferred until hci_cleanup_dev(). */
+ hci_dev_put(hdev);
+}
+EXPORT_SYMBOL(hci_unregister_dev);
+/* Cleanup HCI device */
+void hci_cleanup_dev(struct hci_dev *hdev)
+{
debugfs_remove_recursive(hdev->debugfs);
kfree_const(hdev->hw_info);
kfree_const(hdev->fw_info);
@@ -3320,11 +3323,8 @@ void hci_unregister_dev(struct hci_dev *hdev)
hci_discovery_filter_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);
}
-EXPORT_SYMBOL(hci_unregister_dev);
/* Suspend HCI device */
int hci_suspend_dev(struct hci_dev *hdev)
diff --git a/net/bluetooth/hci_sock.c b/net/bluetooth/hci_sock.c
index 06156de24c50..3ba0c6df73ce 100644
--- a/net/bluetooth/hci_sock.c
+++ b/net/bluetooth/hci_sock.c
@@ -59,6 +59,17 @@ struct hci_pinfo {
char comm[TASK_COMM_LEN];
};
+static struct hci_dev *hci_hdev_from_sock(struct sock *sk)
+{
+ struct hci_dev *hdev = hci_pi(sk)->hdev;
+
+ if (!hdev)
+ return ERR_PTR(-EBADFD);
+ if (hci_dev_test_flag(hdev, HCI_UNREGISTER))
+ return ERR_PTR(-EPIPE);
+ return hdev;
+}
+
void hci_sock_set_flag(struct sock *sk, int nr)
{
set_bit(nr, &hci_pi(sk)->flags);
@@ -752,19 +763,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);
}
@@ -923,10 +928,10 @@ static int hci_sock_blacklist_del(struct hci_dev *hdev, void __user *arg)
static int hci_sock_bound_ioctl(struct sock *sk, unsigned int cmd,
unsigned long arg)
{
- struct hci_dev *hdev = hci_pi(sk)->hdev;
+ struct hci_dev *hdev = hci_hdev_from_sock(sk);
- if (!hdev)
- return -EBADFD;
+ if (IS_ERR(hdev))
+ return PTR_ERR(hdev);
if (hci_dev_test_flag(hdev, HCI_USER_CHANNEL))
return -EBUSY;
@@ -1080,6 +1085,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;
@@ -1356,9 +1373,9 @@ static int hci_sock_getname(struct socket *sock, struct sockaddr *addr,
lock_sock(sk);
- hdev = hci_pi(sk)->hdev;
- if (!hdev) {
- err = -EBADFD;
+ hdev = hci_hdev_from_sock(sk);
+ if (IS_ERR(hdev)) {
+ err = PTR_ERR(hdev);
goto done;
}
@@ -1718,9 +1735,9 @@ static int hci_sock_sendmsg(struct socket *sock, struct msghdr *msg,
goto done;
}
- hdev = hci_pi(sk)->hdev;
- if (!hdev) {
- err = -EBADFD;
+ hdev = hci_hdev_from_sock(sk);
+ if (IS_ERR(hdev)) {
+ err = PTR_ERR(hdev);
goto done;
}
diff --git a/net/bluetooth/hci_sysfs.c b/net/bluetooth/hci_sysfs.c
index 9874844a95a9..b69d88b88d2e 100644
--- a/net/bluetooth/hci_sysfs.c
+++ b/net/bluetooth/hci_sysfs.c
@@ -83,6 +83,9 @@ 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);
+
+ if (hci_dev_test_flag(hdev, HCI_UNREGISTER))
+ hci_cleanup_dev(hdev);
kfree(hdev);
module_put(THIS_MODULE);
}
--
2.30.2
next prev parent reply other threads:[~2021-08-10 17:32 UTC|newest]
Thread overview: 64+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-08-10 17:29 [PATCH 4.19 00/54] 4.19.203-rc1 review Greg Kroah-Hartman
2021-08-10 17:29 ` [PATCH 4.19 01/54] Revert "ACPICA: Fix memory leak caused by _CID repair function" Greg Kroah-Hartman
2021-08-10 17:29 ` [PATCH 4.19 02/54] ALSA: seq: Fix racy deletion of subscriber Greg Kroah-Hartman
2021-08-10 17:29 ` [PATCH 4.19 03/54] ARM: imx: add missing iounmap() Greg Kroah-Hartman
2021-08-10 17:29 ` [PATCH 4.19 04/54] ARM: dts: colibri-imx6ull: limit SDIO clock to 25MHz Greg Kroah-Hartman
2021-08-10 17:29 ` [PATCH 4.19 05/54] ALSA: usb-audio: fix incorrect clock source setting Greg Kroah-Hartman
2021-08-10 17:30 ` [PATCH 4.19 06/54] clk: stm32f4: fix post divisor setup for I2S/SAI PLLs Greg Kroah-Hartman
2021-08-10 17:30 ` [PATCH 4.19 07/54] omap5-board-common: remove not physically existing vdds_1v8_main fixed-regulator Greg Kroah-Hartman
2021-08-10 17:30 ` [PATCH 4.19 08/54] scsi: sr: Return correct event when media event code is 3 Greg Kroah-Hartman
2021-08-10 17:30 ` [PATCH 4.19 09/54] media: videobuf2-core: dequeue if start_streaming fails Greg Kroah-Hartman
2021-08-10 17:30 ` [PATCH 4.19 10/54] net: natsemi: Fix missing pci_disable_device() in probe and remove Greg Kroah-Hartman
2021-08-10 17:30 ` [PATCH 4.19 11/54] sctp: move the active_key update after sh_keys is added Greg Kroah-Hartman
2021-08-10 17:30 ` [PATCH 4.19 12/54] nfp: update ethtool reporting of pauseframe control Greg Kroah-Hartman
2021-08-10 17:30 ` [PATCH 4.19 13/54] net: ipv6: fix returned variable type in ip6_skb_dst_mtu Greg Kroah-Hartman
2021-08-10 17:30 ` [PATCH 4.19 14/54] mips: Fix non-POSIX regexp Greg Kroah-Hartman
2021-08-10 17:30 ` [PATCH 4.19 15/54] bnx2x: fix an error code in bnx2x_nic_load() Greg Kroah-Hartman
2021-08-10 17:30 ` [PATCH 4.19 16/54] net: pegasus: fix uninit-value in get_interrupt_interval Greg Kroah-Hartman
2021-08-10 17:30 ` [PATCH 4.19 17/54] net: fec: fix use-after-free in fec_drv_remove Greg Kroah-Hartman
2021-08-10 17:30 ` [PATCH 4.19 18/54] net: vxge: fix use-after-free in vxge_device_unregister Greg Kroah-Hartman
2021-08-10 17:30 ` [PATCH 4.19 19/54] blk-iolatency: error out if blk_get_queue() failed in iolatency_set_limit() Greg Kroah-Hartman
2021-08-10 17:30 ` Greg Kroah-Hartman [this message]
2021-08-10 17:30 ` [PATCH 4.19 21/54] USB: usbtmc: Fix RCU stall warning Greg Kroah-Hartman
2021-08-10 17:30 ` [PATCH 4.19 22/54] USB: serial: option: add Telit FD980 composition 0x1056 Greg Kroah-Hartman
2021-08-10 17:30 ` [PATCH 4.19 23/54] USB: serial: ch341: fix character loss at high transfer rates Greg Kroah-Hartman
2021-08-10 17:30 ` [PATCH 4.19 24/54] USB: serial: ftdi_sio: add device ID for Auto-M3 OP-COM v2 Greg Kroah-Hartman
2021-08-10 17:30 ` [PATCH 4.19 25/54] firmware_loader: use -ETIMEDOUT instead of -EAGAIN in fw_load_sysfs_fallback Greg Kroah-Hartman
2021-08-10 17:30 ` [PATCH 4.19 26/54] firmware_loader: fix use-after-free in firmware_fallback_sysfs Greg Kroah-Hartman
2021-08-10 17:30 ` [PATCH 4.19 27/54] ALSA: usb-audio: Add registration quirk for JBL Quantum 600 Greg Kroah-Hartman
2021-08-10 17:30 ` [PATCH 4.19 28/54] usb: gadget: f_hid: added GET_IDLE and SET_IDLE handlers Greg Kroah-Hartman
2021-08-10 17:30 ` [PATCH 4.19 29/54] usb: gadget: f_hid: fixed NULL pointer dereference Greg Kroah-Hartman
2021-08-10 17:30 ` [PATCH 4.19 30/54] usb: gadget: f_hid: idle uses the highest byte for duration Greg Kroah-Hartman
2021-08-10 17:30 ` [PATCH 4.19 31/54] usb: otg-fsm: Fix hrtimer list corruption Greg Kroah-Hartman
2021-08-10 17:30 ` [PATCH 4.19 32/54] scripts/tracing: fix the bug that cant parse raw_trace_func Greg Kroah-Hartman
2021-08-10 17:30 ` [PATCH 4.19 33/54] tracing / histogram: Give calculation hist_fields a size Greg Kroah-Hartman
2021-08-10 17:30 ` [PATCH 4.19 34/54] tracing/histogram: Rename "cpu" to "common_cpu" Greg Kroah-Hartman
2021-08-10 17:30 ` [PATCH 4.19 35/54] optee: Clear stale cache entries during initialization Greg Kroah-Hartman
2021-08-10 17:30 ` [PATCH 4.19 36/54] tee: add tee_shm_alloc_kernel_buf() Greg Kroah-Hartman
2021-08-11 7:24 ` Pavel Machek
2021-08-11 13:34 ` Tyler Hicks
2021-08-12 10:56 ` Greg Kroah-Hartman
2021-08-10 17:30 ` [PATCH 4.19 37/54] staging: rtl8723bs: Fix a resource leak in sd_int_dpc Greg Kroah-Hartman
2021-08-10 17:30 ` [PATCH 4.19 38/54] media: rtl28xxu: fix zero-length control request Greg Kroah-Hartman
2021-08-10 17:30 ` [PATCH 4.19 39/54] pipe: increase minimum default pipe size to 2 pages Greg Kroah-Hartman
2021-08-10 17:30 ` [PATCH 4.19 40/54] ext4: fix potential htree corruption when growing large_dir directories Greg Kroah-Hartman
2021-08-10 17:30 ` [PATCH 4.19 41/54] serial: 8250: Mask out floating 16/32-bit bus bits Greg Kroah-Hartman
2021-08-10 17:30 ` [PATCH 4.19 42/54] MIPS: Malta: Do not byte-swap accesses to the CBUS UART Greg Kroah-Hartman
2021-08-10 17:30 ` [PATCH 4.19 43/54] pcmcia: i82092: fix a null pointer dereference bug Greg Kroah-Hartman
2021-08-10 17:30 ` [PATCH 4.19 44/54] KVM: x86: accept userspace interrupt only if no event is injected Greg Kroah-Hartman
2021-08-10 17:30 ` [PATCH 4.19 45/54] KVM: x86/mmu: Fix per-cpu counter corruption on 32-bit builds Greg Kroah-Hartman
2021-08-10 17:30 ` [PATCH 4.19 46/54] spi: meson-spicc: fix memory leak in meson_spicc_remove Greg Kroah-Hartman
2021-08-10 17:30 ` [PATCH 4.19 47/54] perf/x86/amd: Dont touch the AMD64_EVENTSEL_HOSTONLY bit inside the guest Greg Kroah-Hartman
2021-08-10 17:30 ` [PATCH 4.19 48/54] qmi_wwan: add network device usage statistics for qmimux devices Greg Kroah-Hartman
2021-08-10 17:30 ` [PATCH 4.19 49/54] libata: fix ata_pio_sector for CONFIG_HIGHMEM Greg Kroah-Hartman
2021-08-10 17:30 ` [PATCH 4.19 50/54] reiserfs: add check for root_inode in reiserfs_fill_super Greg Kroah-Hartman
2021-08-10 17:30 ` [PATCH 4.19 51/54] reiserfs: check directory items on read from disk Greg Kroah-Hartman
2021-08-10 17:30 ` [PATCH 4.19 52/54] alpha: Send stop IPI to send to online CPUs Greg Kroah-Hartman
2021-08-10 17:30 ` [PATCH 4.19 53/54] net/qla3xxx: fix schedule while atomic in ql_wait_for_drvr_lock and ql_adapter_reset Greg Kroah-Hartman
2021-08-10 17:30 ` [PATCH 4.19 54/54] ARM: imx: add mmdc ipg clock operation for mmdc Greg Kroah-Hartman
2021-08-11 1:07 ` [PATCH 4.19 00/54] 4.19.203-rc1 review Samuel Zou
2021-08-11 9:42 ` Sudip Mukherjee
2021-08-11 17:13 ` Naresh Kamboju
2021-08-11 19:58 ` Guenter Roeck
2021-08-11 22:22 ` Shuah Khan
2021-08-12 19:20 ` Pavel Machek
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20210810172944.842148853@linuxfoundation.org \
--to=gregkh@linuxfoundation.org \
--cc=linux-kernel@vger.kernel.org \
--cc=luiz.von.dentz@intel.com \
--cc=penguin-kernel@I-love.SAKURA.ne.jp \
--cc=sashal@kernel.org \
--cc=stable@vger.kernel.org \
--cc=syzbot+a5df189917e79d5e59c9@syzkaller.appspotmail.com \
--cc=torvalds@linux-foundation.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).