Fix CVE-2024-41062
Edward Adam Davis (1): bluetooth/l2cap: sync sock recv cb and release
Luiz Augusto von Dentz (2): Bluetooth: L2CAP: Fix deadlock Bluetooth: Fix usage of __hci_cmd_sync_status
Marcel Holtmann (1): Bluetooth: Add helper for serialized HCI command execution
include/net/bluetooth/hci_core.h | 11 +- include/net/bluetooth/hci_sync.h | 44 ++++ net/bluetooth/Makefile | 2 +- net/bluetooth/hci_core.c | 92 ++------- net/bluetooth/hci_request.c | 68 ------ net/bluetooth/hci_request.h | 4 + net/bluetooth/hci_sync.c | 343 +++++++++++++++++++++++++++++++ net/bluetooth/l2cap_core.c | 3 + net/bluetooth/l2cap_sock.c | 10 +- 9 files changed, 427 insertions(+), 150 deletions(-) create mode 100644 include/net/bluetooth/hci_sync.h create mode 100644 net/bluetooth/hci_sync.c
From: Edward Adam Davis eadavis@qq.com
mainline inclusion from mainline-v6.10-rc7 commit 89e856e124f9ae548572c56b1b70c2255705f8fe category: bugfix bugzilla: https://gitee.com/src-openeuler/kernel/issues/IAGEK1 CVE: CVE-2024-41062
Reference: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?i...
-------------------------------------------
The problem occurs between the system call to close the sock and hci_rx_work, where the former releases the sock and the latter accesses it without lock protection.
CPU0 CPU1 ---- ---- sock_close hci_rx_work l2cap_sock_release hci_acldata_packet l2cap_sock_kill l2cap_recv_frame sk_free l2cap_conless_channel l2cap_sock_recv_cb
If hci_rx_work processes the data that needs to be received before the sock is closed, then everything is normal; Otherwise, the work thread may access the released sock when receiving data.
Add a chan mutex in the rx callback of the sock to achieve synchronization between the sock release and recv cb.
Sock is dead, so set chan data to NULL, avoid others use invalid sock pointer.
Reported-and-tested-by: syzbot+b7f6f8c9303466e16c8a@syzkaller.appspotmail.com Signed-off-by: Edward Adam Davis eadavis@qq.com Signed-off-by: Luiz Augusto von Dentz luiz.von.dentz@intel.com
Conflicts: net/bluetooth/l2cap_sock.c [The conflict occurs because the commit 89e856e124f9("bluetooth/l2cap: sync sock recv cb and release") is not merged] Signed-off-by: Zhengchao Shao shaozhengchao@huawei.com --- net/bluetooth/l2cap_sock.c | 20 +++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-)
diff --git a/net/bluetooth/l2cap_sock.c b/net/bluetooth/l2cap_sock.c index c91f4c190bbc..c7217d3ecd8b 100644 --- a/net/bluetooth/l2cap_sock.c +++ b/net/bluetooth/l2cap_sock.c @@ -1238,6 +1238,10 @@ static void l2cap_sock_kill(struct sock *sk)
BT_DBG("sk %p state %s", sk, state_to_string(sk->sk_state));
+ /* Sock is dead, so set chan data to NULL, avoid other task use invalid + * sock pointer. + */ + l2cap_pi(sk)->chan->data = NULL; /* Kill poor orphan */
l2cap_chan_put(l2cap_pi(sk)->chan); @@ -1480,9 +1484,21 @@ static struct l2cap_chan *l2cap_sock_new_connection_cb(struct l2cap_chan *chan)
static int l2cap_sock_recv_cb(struct l2cap_chan *chan, struct sk_buff *skb) { - struct sock *sk = chan->data; + struct sock *sk; int err;
+ /* To avoid race with sock_release, a chan lock needs to be added here + * to synchronize the sock. + */ + l2cap_chan_hold(chan); + l2cap_chan_lock(chan); + sk = chan->data; + if (!sk) { + l2cap_chan_unlock(chan); + l2cap_chan_put(chan); + return -ENXIO; + } + lock_sock(sk);
if (l2cap_pi(sk)->rx_busy_skb) { @@ -1519,6 +1535,8 @@ static int l2cap_sock_recv_cb(struct l2cap_chan *chan, struct sk_buff *skb)
done: release_sock(sk); + l2cap_chan_unlock(chan); + l2cap_chan_put(chan);
return err; }
From: Marcel Holtmann marcel@holtmann.org
mainline inclusion from mainline-v5.17-rc1 commit 6a98e3836fa2077b169f10a35c2ca9952d53f987 category: bugfix bugzilla: https://gitee.com/src-openeuler/kernel/issues/IAGEK1 CVE: CVE-2024-41062
Reference: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?i...
-------------------------------------------
The usage of __hci_cmd_sync() within the hdev->setup() callback allows for a nice and simple serialized execution of HCI commands. More importantly it allows for result processing before issueing the next command.
With the current usage of hci_req_run() it is possible to batch up commands and execute them, but it is impossible to react to their results or errors.
This is an attempt to generalize the hdev->setup() handling and provide a simple way of running multiple HCI commands from a single function context.
There are multiple struct work that are decdicated to certain tasks already used right now. It is add a lot of bloat to hci_dev struct and extra handling code. So it might be possible to put all of these behind a common HCI command infrastructure and just execute the HCI commands from the same work context in a serialized fashion.
For example updating the white list and resolving list can be done now without having to know the list size ahead of time. Also preparing for suspend or resume shouldn't require a state machine anymore. There are other tasks that should be simplified as well.
Signed-off-by: Marcel Holtmann marcel@holtmann.org Signed-off-by: Luiz Augusto von Dentz luiz.von.dentz@intel.com Signed-off-by: Marcel Holtmann marcel@holtmann.org
Conflicts: net/bluetooth/Makefile net/bluetooth/hci_core.c net/bluetooth/hci_request.c [The conflict occurs because the commit 01ce70b0a274("Bluetooth: eir: Move EIR/Adv Data functions to its own file") is not merged] Signed-off-by: Zhengchao Shao shaozhengchao@huawei.com --- include/net/bluetooth/hci_core.h | 11 +- include/net/bluetooth/hci_sync.h | 42 ++++ net/bluetooth/Makefile | 2 +- net/bluetooth/hci_core.c | 23 +-- net/bluetooth/hci_request.c | 68 ------- net/bluetooth/hci_request.h | 4 + net/bluetooth/hci_sync.c | 330 +++++++++++++++++++++++++++++++ 7 files changed, 385 insertions(+), 95 deletions(-) create mode 100644 include/net/bluetooth/hci_sync.h create mode 100644 net/bluetooth/hci_sync.c
diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h index 5d8b6bd87495..622de75b66d4 100644 --- a/include/net/bluetooth/hci_core.h +++ b/include/net/bluetooth/hci_core.h @@ -30,6 +30,7 @@ #include <linux/rculist.h>
#include <net/bluetooth/hci.h> +#include <net/bluetooth/hci_sync.h> #include <net/bluetooth/hci_sock.h>
/* HCI priority */ @@ -441,6 +442,9 @@ struct hci_dev { struct work_struct power_on; struct delayed_work power_off; struct work_struct error_reset; + struct work_struct cmd_sync_work; + struct list_head cmd_sync_work_list; + struct mutex cmd_sync_work_lock;
__u16 discov_timeout; struct delayed_work discov_off; @@ -1641,10 +1645,6 @@ static inline int hci_check_conn_params(u16 min, u16 max, u16 latency, int hci_register_cb(struct hci_cb *hcb); int hci_unregister_cb(struct hci_cb *hcb);
-struct sk_buff *__hci_cmd_sync(struct hci_dev *hdev, u16 opcode, u32 plen, - const void *param, u32 timeout); -struct sk_buff *__hci_cmd_sync_ev(struct hci_dev *hdev, u16 opcode, u32 plen, - const void *param, u8 event, u32 timeout); int __hci_cmd_send(struct hci_dev *hdev, u16 opcode, u32 plen, const void *param);
@@ -1655,9 +1655,6 @@ void hci_send_sco(struct hci_conn *conn, struct sk_buff *skb);
void *hci_sent_cmd_data(struct hci_dev *hdev, __u16 opcode);
-struct sk_buff *hci_cmd_sync(struct hci_dev *hdev, u16 opcode, u32 plen, - const void *param, u32 timeout); - u32 hci_conn_get_phy(struct hci_conn *conn);
/* ----- HCI Sockets ----- */ diff --git a/include/net/bluetooth/hci_sync.h b/include/net/bluetooth/hci_sync.h new file mode 100644 index 000000000000..fcfdeb3cbd7c --- /dev/null +++ b/include/net/bluetooth/hci_sync.h @@ -0,0 +1,42 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +/* + * BlueZ - Bluetooth protocol stack for Linux + * + * Copyright (C) 2021 Intel Corporation + */ + +typedef int (*hci_cmd_sync_work_func_t)(struct hci_dev *hdev, void *data); +typedef void (*hci_cmd_sync_work_destroy_t)(struct hci_dev *hdev, void *data, + int err); + +struct hci_cmd_sync_work_entry { + struct list_head list; + hci_cmd_sync_work_func_t func; + void *data; + hci_cmd_sync_work_destroy_t destroy; +}; + +/* Function with sync suffix shall not be called with hdev->lock held as they + * wait the command to complete and in the meantime an event could be received + * which could attempt to acquire hdev->lock causing a deadlock. + */ +struct sk_buff *__hci_cmd_sync(struct hci_dev *hdev, u16 opcode, u32 plen, + const void *param, u32 timeout); +struct sk_buff *hci_cmd_sync(struct hci_dev *hdev, u16 opcode, u32 plen, + const void *param, u32 timeout); +struct sk_buff *__hci_cmd_sync_ev(struct hci_dev *hdev, u16 opcode, u32 plen, + const void *param, u8 event, u32 timeout); +struct sk_buff *__hci_cmd_sync_sk(struct hci_dev *hdev, u16 opcode, u32 plen, + const void *param, u8 event, u32 timeout, + struct sock *sk); +int __hci_cmd_sync_status(struct hci_dev *hdev, u16 opcode, u32 plen, + const void *param, u32 timeout); +int __hci_cmd_sync_status_sk(struct hci_dev *hdev, u16 opcode, u32 plen, + const void *param, u8 event, u32 timeout, + struct sock *sk); + +void hci_cmd_sync_init(struct hci_dev *hdev); +void hci_cmd_sync_clear(struct hci_dev *hdev); + +int hci_cmd_sync_queue(struct hci_dev *hdev, hci_cmd_sync_work_func_t func, + void *data, hci_cmd_sync_work_destroy_t destroy); diff --git a/net/bluetooth/Makefile b/net/bluetooth/Makefile index 1c645fba8c49..00a82271663a 100644 --- a/net/bluetooth/Makefile +++ b/net/bluetooth/Makefile @@ -14,7 +14,7 @@ bluetooth_6lowpan-y := 6lowpan.o
bluetooth-y := af_bluetooth.o hci_core.o hci_conn.o hci_event.o mgmt.o \ hci_sock.o hci_sysfs.o l2cap_core.o l2cap_sock.o smp.o lib.o \ - ecdh_helper.o hci_request.o mgmt_util.o mgmt_config.o + ecdh_helper.o hci_request.o mgmt_util.o mgmt_config.o hci_sync.o
bluetooth-$(CONFIG_BT_BREDR) += sco.o bluetooth-$(CONFIG_BT_HS) += a2mp.o amp.o diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c index dedb939dec2e..55cd6df091df 100644 --- a/net/bluetooth/hci_core.c +++ b/net/bluetooth/hci_core.c @@ -3681,6 +3681,8 @@ struct hci_dev *hci_alloc_dev(void) INIT_WORK(&hdev->error_reset, hci_error_reset); INIT_WORK(&hdev->suspend_prepare, hci_prepare_suspend);
+ hci_cmd_sync_init(hdev); + INIT_DELAYED_WORK(&hdev->power_off, hci_power_off);
skb_queue_head_init(&hdev->rx_q); @@ -3841,6 +3843,8 @@ void hci_unregister_dev(struct hci_dev *hdev) cancel_work_sync(&hdev->power_on); cancel_work_sync(&hdev->error_reset);
+ hci_cmd_sync_clear(hdev); + if (!test_bit(HCI_QUIRK_NO_SUSPEND_NOTIFIER, &hdev->quirks)) { hci_suspend_clear_tasks(hdev); unregister_pm_notifier(&hdev->suspend_notifier); @@ -4140,25 +4144,6 @@ void *hci_sent_cmd_data(struct hci_dev *hdev, __u16 opcode) return hdev->sent_cmd->data + HCI_COMMAND_HDR_SIZE; }
-/* Send HCI command and wait for command commplete event */ -struct sk_buff *hci_cmd_sync(struct hci_dev *hdev, u16 opcode, u32 plen, - const void *param, u32 timeout) -{ - struct sk_buff *skb; - - if (!test_bit(HCI_UP, &hdev->flags)) - return ERR_PTR(-ENETDOWN); - - bt_dev_dbg(hdev, "opcode 0x%4.4x plen %d", opcode, plen); - - hci_req_sync_lock(hdev); - skb = __hci_cmd_sync(hdev, opcode, plen, param, timeout); - hci_req_sync_unlock(hdev); - - return skb; -} -EXPORT_SYMBOL(hci_cmd_sync); - /* Send ACL data */ static void hci_add_acl_hdr(struct sk_buff *skb, __u16 handle, __u16 flags) { diff --git a/net/bluetooth/hci_request.c b/net/bluetooth/hci_request.c index 7ce6db1ac558..2dc8b72be775 100644 --- a/net/bluetooth/hci_request.c +++ b/net/bluetooth/hci_request.c @@ -30,10 +30,6 @@ #include "smp.h" #include "hci_request.h"
-#define HCI_REQ_DONE 0 -#define HCI_REQ_PEND 1 -#define HCI_REQ_CANCELED 2 - void hci_req_init(struct hci_request *req, struct hci_dev *hdev) { skb_queue_head_init(&req->cmd_q); @@ -126,70 +122,6 @@ void hci_req_sync_cancel(struct hci_dev *hdev, int err) } }
-struct sk_buff *__hci_cmd_sync_ev(struct hci_dev *hdev, u16 opcode, u32 plen, - const void *param, u8 event, u32 timeout) -{ - struct hci_request req; - struct sk_buff *skb; - int err = 0; - - BT_DBG("%s", hdev->name); - - hci_req_init(&req, hdev); - - hci_req_add_ev(&req, opcode, plen, param, event); - - hdev->req_status = HCI_REQ_PEND; - - err = hci_req_run_skb(&req, hci_req_sync_complete); - if (err < 0) - return ERR_PTR(err); - - err = wait_event_interruptible_timeout(hdev->req_wait_q, - hdev->req_status != HCI_REQ_PEND, timeout); - - if (err == -ERESTARTSYS) - return ERR_PTR(-EINTR); - - switch (hdev->req_status) { - case HCI_REQ_DONE: - err = -bt_to_errno(hdev->req_result); - break; - - case HCI_REQ_CANCELED: - err = -hdev->req_result; - break; - - default: - err = -ETIMEDOUT; - break; - } - - hdev->req_status = hdev->req_result = 0; - skb = hdev->req_skb; - hdev->req_skb = NULL; - - BT_DBG("%s end: err %d", hdev->name, err); - - if (err < 0) { - kfree_skb(skb); - return ERR_PTR(err); - } - - if (!skb) - return ERR_PTR(-ENODATA); - - return skb; -} -EXPORT_SYMBOL(__hci_cmd_sync_ev); - -struct sk_buff *__hci_cmd_sync(struct hci_dev *hdev, u16 opcode, u32 plen, - const void *param, u32 timeout) -{ - return __hci_cmd_sync_ev(hdev, opcode, plen, param, 0, timeout); -} -EXPORT_SYMBOL(__hci_cmd_sync); - /* Execute request and wait for completion. */ int __hci_req_sync(struct hci_dev *hdev, int (*func)(struct hci_request *req, unsigned long opt), diff --git a/net/bluetooth/hci_request.h b/net/bluetooth/hci_request.h index 6a12e84c66c4..5a6ed954b22b 100644 --- a/net/bluetooth/hci_request.h +++ b/net/bluetooth/hci_request.h @@ -25,6 +25,10 @@ #define hci_req_sync_lock(hdev) mutex_lock(&hdev->req_lock) #define hci_req_sync_unlock(hdev) mutex_unlock(&hdev->req_lock)
+#define HCI_REQ_DONE 0 +#define HCI_REQ_PEND 1 +#define HCI_REQ_CANCELED 2 + struct hci_request { struct hci_dev *hdev; struct sk_buff_head cmd_q; diff --git a/net/bluetooth/hci_sync.c b/net/bluetooth/hci_sync.c new file mode 100644 index 000000000000..b2048287fe90 --- /dev/null +++ b/net/bluetooth/hci_sync.c @@ -0,0 +1,330 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * BlueZ - Bluetooth protocol stack for Linux + * + * Copyright (C) 2021 Intel Corporation + */ + +#include <net/bluetooth/bluetooth.h> +#include <net/bluetooth/hci_core.h> +#include <net/bluetooth/mgmt.h> + +#include "hci_request.h" +#include "smp.h" + +static void hci_cmd_sync_complete(struct hci_dev *hdev, u8 result, u16 opcode, + struct sk_buff *skb) +{ + bt_dev_dbg(hdev, "result 0x%2.2x", result); + + if (hdev->req_status != HCI_REQ_PEND) + return; + + hdev->req_result = result; + hdev->req_status = HCI_REQ_DONE; + + wake_up_interruptible(&hdev->req_wait_q); +} + +static struct sk_buff *hci_cmd_sync_alloc(struct hci_dev *hdev, u16 opcode, + u32 plen, const void *param, + struct sock *sk) +{ + int len = HCI_COMMAND_HDR_SIZE + plen; + struct hci_command_hdr *hdr; + struct sk_buff *skb; + + skb = bt_skb_alloc(len, GFP_ATOMIC); + if (!skb) + return NULL; + + hdr = skb_put(skb, HCI_COMMAND_HDR_SIZE); + hdr->opcode = cpu_to_le16(opcode); + hdr->plen = plen; + + if (plen) + skb_put_data(skb, param, plen); + + bt_dev_dbg(hdev, "skb len %d", skb->len); + + hci_skb_pkt_type(skb) = HCI_COMMAND_PKT; + hci_skb_opcode(skb) = opcode; + + return skb; +} + +static void hci_cmd_sync_add(struct hci_request *req, u16 opcode, u32 plen, + const void *param, u8 event, struct sock *sk) +{ + struct hci_dev *hdev = req->hdev; + struct sk_buff *skb; + + bt_dev_dbg(hdev, "opcode 0x%4.4x plen %d", opcode, plen); + + /* If an error occurred during request building, there is no point in + * queueing the HCI command. We can simply return. + */ + if (req->err) + return; + + skb = hci_cmd_sync_alloc(hdev, opcode, plen, param, sk); + if (!skb) { + bt_dev_err(hdev, "no memory for command (opcode 0x%4.4x)", + opcode); + req->err = -ENOMEM; + return; + } + + if (skb_queue_empty(&req->cmd_q)) + bt_cb(skb)->hci.req_flags |= HCI_REQ_START; + + bt_cb(skb)->hci.req_event = event; + + skb_queue_tail(&req->cmd_q, skb); +} + +static int hci_cmd_sync_run(struct hci_request *req) +{ + struct hci_dev *hdev = req->hdev; + struct sk_buff *skb; + unsigned long flags; + + bt_dev_dbg(hdev, "length %u", skb_queue_len(&req->cmd_q)); + + /* If an error occurred during request building, remove all HCI + * commands queued on the HCI request queue. + */ + if (req->err) { + skb_queue_purge(&req->cmd_q); + return req->err; + } + + /* Do not allow empty requests */ + if (skb_queue_empty(&req->cmd_q)) + return -ENODATA; + + skb = skb_peek_tail(&req->cmd_q); + bt_cb(skb)->hci.req_complete_skb = hci_cmd_sync_complete; + bt_cb(skb)->hci.req_flags |= HCI_REQ_SKB; + + spin_lock_irqsave(&hdev->cmd_q.lock, flags); + skb_queue_splice_tail(&req->cmd_q, &hdev->cmd_q); + spin_unlock_irqrestore(&hdev->cmd_q.lock, flags); + + queue_work(hdev->workqueue, &hdev->cmd_work); + + return 0; +} + +/* This function requires the caller holds hdev->req_lock. */ +struct sk_buff *__hci_cmd_sync_sk(struct hci_dev *hdev, u16 opcode, u32 plen, + const void *param, u8 event, u32 timeout, + struct sock *sk) +{ + struct hci_request req; + struct sk_buff *skb; + int err = 0; + + bt_dev_dbg(hdev, ""); + + hci_req_init(&req, hdev); + + hci_cmd_sync_add(&req, opcode, plen, param, event, sk); + + hdev->req_status = HCI_REQ_PEND; + + err = hci_cmd_sync_run(&req); + if (err < 0) + return ERR_PTR(err); + + err = wait_event_interruptible_timeout(hdev->req_wait_q, + hdev->req_status != HCI_REQ_PEND, + timeout); + + if (err == -ERESTARTSYS) + return ERR_PTR(-EINTR); + + switch (hdev->req_status) { + case HCI_REQ_DONE: + err = -bt_to_errno(hdev->req_result); + break; + + case HCI_REQ_CANCELED: + err = -hdev->req_result; + break; + + default: + err = -ETIMEDOUT; + break; + } + + hdev->req_status = 0; + hdev->req_result = 0; + skb = hdev->req_skb; + hdev->req_skb = NULL; + + bt_dev_dbg(hdev, "end: err %d", err); + + if (err < 0) { + kfree_skb(skb); + return ERR_PTR(err); + } + + if (!skb) + return ERR_PTR(-ENODATA); + + return skb; +} +EXPORT_SYMBOL(__hci_cmd_sync_sk); + +/* This function requires the caller holds hdev->req_lock. */ +struct sk_buff *__hci_cmd_sync(struct hci_dev *hdev, u16 opcode, u32 plen, + const void *param, u32 timeout) +{ + return __hci_cmd_sync_sk(hdev, opcode, plen, param, 0, timeout, NULL); +} +EXPORT_SYMBOL(__hci_cmd_sync); + +/* Send HCI command and wait for command complete event */ +struct sk_buff *hci_cmd_sync(struct hci_dev *hdev, u16 opcode, u32 plen, + const void *param, u32 timeout) +{ + struct sk_buff *skb; + + if (!test_bit(HCI_UP, &hdev->flags)) + return ERR_PTR(-ENETDOWN); + + bt_dev_dbg(hdev, "opcode 0x%4.4x plen %d", opcode, plen); + + hci_req_sync_lock(hdev); + skb = __hci_cmd_sync(hdev, opcode, plen, param, timeout); + hci_req_sync_unlock(hdev); + + return skb; +} +EXPORT_SYMBOL(hci_cmd_sync); + +/* This function requires the caller holds hdev->req_lock. */ +struct sk_buff *__hci_cmd_sync_ev(struct hci_dev *hdev, u16 opcode, u32 plen, + const void *param, u8 event, u32 timeout) +{ + return __hci_cmd_sync_sk(hdev, opcode, plen, param, event, timeout, + NULL); +} +EXPORT_SYMBOL(__hci_cmd_sync_ev); + +/* This function requires the caller holds hdev->req_lock. */ +int __hci_cmd_sync_status_sk(struct hci_dev *hdev, u16 opcode, u32 plen, + const void *param, u8 event, u32 timeout, + struct sock *sk) +{ + struct sk_buff *skb; + u8 status; + + skb = __hci_cmd_sync_sk(hdev, opcode, plen, param, event, timeout, sk); + if (IS_ERR_OR_NULL(skb)) { + bt_dev_err(hdev, "Opcode 0x%4x failed: %ld", opcode, + PTR_ERR(skb)); + return PTR_ERR(skb); + } + + status = skb->data[0]; + + kfree_skb(skb); + + return status; +} +EXPORT_SYMBOL(__hci_cmd_sync_status_sk); + +int __hci_cmd_sync_status(struct hci_dev *hdev, u16 opcode, u32 plen, + const void *param, u32 timeout) +{ + return __hci_cmd_sync_status_sk(hdev, opcode, plen, param, 0, timeout, + NULL); +} +EXPORT_SYMBOL(__hci_cmd_sync_status); + +static void hci_cmd_sync_work(struct work_struct *work) +{ + struct hci_dev *hdev = container_of(work, struct hci_dev, cmd_sync_work); + struct hci_cmd_sync_work_entry *entry; + hci_cmd_sync_work_func_t func; + hci_cmd_sync_work_destroy_t destroy; + void *data; + + bt_dev_dbg(hdev, ""); + + mutex_lock(&hdev->cmd_sync_work_lock); + entry = list_first_entry(&hdev->cmd_sync_work_list, + struct hci_cmd_sync_work_entry, list); + if (entry) { + list_del(&entry->list); + func = entry->func; + data = entry->data; + destroy = entry->destroy; + kfree(entry); + } else { + func = NULL; + data = NULL; + destroy = NULL; + } + mutex_unlock(&hdev->cmd_sync_work_lock); + + if (func) { + int err; + + hci_req_sync_lock(hdev); + + err = func(hdev, data); + + if (destroy) + destroy(hdev, data, err); + + hci_req_sync_unlock(hdev); + } +} + +void hci_cmd_sync_init(struct hci_dev *hdev) +{ + INIT_WORK(&hdev->cmd_sync_work, hci_cmd_sync_work); + INIT_LIST_HEAD(&hdev->cmd_sync_work_list); + mutex_init(&hdev->cmd_sync_work_lock); +} + +void hci_cmd_sync_clear(struct hci_dev *hdev) +{ + struct hci_cmd_sync_work_entry *entry, *tmp; + + cancel_work_sync(&hdev->cmd_sync_work); + + list_for_each_entry_safe(entry, tmp, &hdev->cmd_sync_work_list, list) { + if (entry->destroy) + entry->destroy(hdev, entry->data, -ECANCELED); + + list_del(&entry->list); + kfree(entry); + } +} + +int hci_cmd_sync_queue(struct hci_dev *hdev, hci_cmd_sync_work_func_t func, + void *data, hci_cmd_sync_work_destroy_t destroy) +{ + struct hci_cmd_sync_work_entry *entry; + + entry = kmalloc(sizeof(*entry), GFP_KERNEL); + if (!entry) + return -ENOMEM; + + entry->func = func; + entry->data = data; + entry->destroy = destroy; + + mutex_lock(&hdev->cmd_sync_work_lock); + list_add_tail(&entry->list, &hdev->cmd_sync_work_list); + mutex_unlock(&hdev->cmd_sync_work_lock); + + queue_work(hdev->req_workqueue, &hdev->cmd_sync_work); + + return 0; +} +EXPORT_SYMBOL(hci_cmd_sync_queue);
From: Luiz Augusto von Dentz luiz.von.dentz@intel.com
mainline inclusion from mainline-v6.10-rc7 commit f1a8f402f13f94263cf349216c257b2985100927 category: bugfix bugzilla: https://gitee.com/src-openeuler/kernel/issues/IAGEK1 CVE: CVE-2024-41062
Reference: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?i...
-------------------------------------------
This fixes the following deadlock introduced by 39a92a55be13 ("bluetooth/l2cap: sync sock recv cb and release")
============================================ WARNING: possible recursive locking detected 6.10.0-rc3-g4029dba6b6f1 #6823 Not tainted -------------------------------------------- kworker/u5:0/35 is trying to acquire lock: ffff888002ec2510 (&chan->lock#2/1){+.+.}-{3:3}, at: l2cap_sock_recv_cb+0x44/0x1e0
but task is already holding lock: ffff888002ec2510 (&chan->lock#2/1){+.+.}-{3:3}, at: l2cap_get_chan_by_scid+0xaf/0xd0
other info that might help us debug this: Possible unsafe locking scenario:
CPU0 ---- lock(&chan->lock#2/1); lock(&chan->lock#2/1);
*** DEADLOCK ***
May be due to missing lock nesting notation
3 locks held by kworker/u5:0/35: #0: ffff888002b8a940 ((wq_completion)hci0#2){+.+.}-{0:0}, at: process_one_work+0x750/0x930 #1: ffff888002c67dd0 ((work_completion)(&hdev->rx_work)){+.+.}-{0:0}, at: process_one_work+0x44e/0x930 #2: ffff888002ec2510 (&chan->lock#2/1){+.+.}-{3:3}, at: l2cap_get_chan_by_scid+0xaf/0xd0
To fix the original problem this introduces l2cap_chan_lock at l2cap_conless_channel to ensure that l2cap_sock_recv_cb is called with chan->lock held.
Fixes: 89e856e124f9 ("bluetooth/l2cap: sync sock recv cb and release") Signed-off-by: Luiz Augusto von Dentz luiz.von.dentz@intel.com
Conflicts: net/bluetooth/hci_core.c net/bluetooth/l2cap_sock.c [The conflict occurs because the commit 89e856e124f9("bluetooth/l2cap: sync sock recv cb and release") is not merged] Signed-off-by: Zhengchao Shao shaozhengchao@huawei.com --- include/net/bluetooth/hci_sync.h | 2 + net/bluetooth/hci_core.c | 72 ++++++++------------------------ net/bluetooth/hci_sync.c | 13 ++++++ net/bluetooth/l2cap_core.c | 3 ++ net/bluetooth/l2cap_sock.c | 12 +----- 5 files changed, 37 insertions(+), 65 deletions(-)
diff --git a/include/net/bluetooth/hci_sync.h b/include/net/bluetooth/hci_sync.h index fcfdeb3cbd7c..fd656c0d3abd 100644 --- a/include/net/bluetooth/hci_sync.h +++ b/include/net/bluetooth/hci_sync.h @@ -34,6 +34,8 @@ int __hci_cmd_sync_status(struct hci_dev *hdev, u16 opcode, u32 plen, int __hci_cmd_sync_status_sk(struct hci_dev *hdev, u16 opcode, u32 plen, const void *param, u8 event, u32 timeout, struct sock *sk); +int hci_cmd_sync_status(struct hci_dev *hdev, u16 opcode, u32 plen, + const void *param, u32 timeout);
void hci_cmd_sync_init(struct hci_dev *hdev); void hci_cmd_sync_clear(struct hci_dev *hdev); diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c index 55cd6df091df..c24b7c7dddbc 100644 --- a/net/bluetooth/hci_core.c +++ b/net/bluetooth/hci_core.c @@ -994,50 +994,6 @@ static int __hci_unconf_init(struct hci_dev *hdev) return 0; }
-static int hci_scan_req(struct hci_request *req, unsigned long opt) -{ - __u8 scan = opt; - - BT_DBG("%s %x", req->hdev->name, scan); - - /* Inquiry and Page scans */ - hci_req_add(req, HCI_OP_WRITE_SCAN_ENABLE, 1, &scan); - return 0; -} - -static int hci_auth_req(struct hci_request *req, unsigned long opt) -{ - __u8 auth = opt; - - BT_DBG("%s %x", req->hdev->name, auth); - - /* Authentication */ - hci_req_add(req, HCI_OP_WRITE_AUTH_ENABLE, 1, &auth); - return 0; -} - -static int hci_encrypt_req(struct hci_request *req, unsigned long opt) -{ - __u8 encrypt = opt; - - BT_DBG("%s %x", req->hdev->name, encrypt); - - /* Encryption */ - hci_req_add(req, HCI_OP_WRITE_ENCRYPT_MODE, 1, &encrypt); - return 0; -} - -static int hci_linkpol_req(struct hci_request *req, unsigned long opt) -{ - __le16 policy = cpu_to_le16(opt); - - BT_DBG("%s %x", req->hdev->name, policy); - - /* Default link policy */ - hci_req_add(req, HCI_OP_WRITE_DEF_LINK_POLICY, 2, &policy); - return 0; -} - /* Get HCI device by index. * Device is held on return. */ struct hci_dev *hci_dev_get(int index) @@ -1999,6 +1955,7 @@ int hci_dev_cmd(unsigned int cmd, void __user *arg) { struct hci_dev *hdev; struct hci_dev_req dr; + __le16 policy; int err = 0;
if (copy_from_user(&dr, arg, sizeof(dr))) @@ -2030,8 +1987,8 @@ int hci_dev_cmd(unsigned int cmd, void __user *arg)
switch (cmd) { case HCISETAUTH: - err = hci_req_sync(hdev, hci_auth_req, dr.dev_opt, - HCI_INIT_TIMEOUT, NULL); + err = __hci_cmd_sync_status(hdev, HCI_OP_WRITE_AUTH_ENABLE, + 1, &dr.dev_opt, HCI_CMD_TIMEOUT); break;
case HCISETENCRYPT: @@ -2042,19 +1999,23 @@ int hci_dev_cmd(unsigned int cmd, void __user *arg)
if (!test_bit(HCI_AUTH, &hdev->flags)) { /* Auth must be enabled first */ - err = hci_req_sync(hdev, hci_auth_req, dr.dev_opt, - HCI_INIT_TIMEOUT, NULL); + err = __hci_cmd_sync_status(hdev, + HCI_OP_WRITE_AUTH_ENABLE, + 1, &dr.dev_opt, + HCI_CMD_TIMEOUT); if (err) break; }
- err = hci_req_sync(hdev, hci_encrypt_req, dr.dev_opt, - HCI_INIT_TIMEOUT, NULL); + err = __hci_cmd_sync_status(hdev, HCI_OP_WRITE_ENCRYPT_MODE, + 1, &dr.dev_opt, + HCI_CMD_TIMEOUT); break;
case HCISETSCAN: - err = hci_req_sync(hdev, hci_scan_req, dr.dev_opt, - HCI_INIT_TIMEOUT, NULL); + err = __hci_cmd_sync_status(hdev, HCI_OP_WRITE_SCAN_ENABLE, + 1, &dr.dev_opt, + HCI_CMD_TIMEOUT);
/* Ensure that the connectable and discoverable states * get correctly modified as this was a non-mgmt change. @@ -2064,8 +2025,11 @@ int hci_dev_cmd(unsigned int cmd, void __user *arg) break;
case HCISETLINKPOL: - err = hci_req_sync(hdev, hci_linkpol_req, dr.dev_opt, - HCI_INIT_TIMEOUT, NULL); + policy = cpu_to_le16(dr.dev_opt); + + err = __hci_cmd_sync_status(hdev, HCI_OP_WRITE_DEF_LINK_POLICY, + 2, &policy, + HCI_CMD_TIMEOUT); break;
case HCISETLINKMODE: diff --git a/net/bluetooth/hci_sync.c b/net/bluetooth/hci_sync.c index b2048287fe90..1660d607483b 100644 --- a/net/bluetooth/hci_sync.c +++ b/net/bluetooth/hci_sync.c @@ -244,6 +244,19 @@ int __hci_cmd_sync_status(struct hci_dev *hdev, u16 opcode, u32 plen, } EXPORT_SYMBOL(__hci_cmd_sync_status);
+int hci_cmd_sync_status(struct hci_dev *hdev, u16 opcode, u32 plen, + const void *param, u32 timeout) +{ + int err; + + hci_req_sync_lock(hdev); + err = __hci_cmd_sync_status(hdev, opcode, plen, param, timeout); + hci_req_sync_unlock(hdev); + + return err; +} +EXPORT_SYMBOL(hci_cmd_sync_status); + static void hci_cmd_sync_work(struct work_struct *work) { struct hci_dev *hdev = container_of(work, struct hci_dev, cmd_sync_work); diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c index c3b6a416119e..707076552f0b 100644 --- a/net/bluetooth/l2cap_core.c +++ b/net/bluetooth/l2cap_core.c @@ -7713,6 +7713,8 @@ static void l2cap_conless_channel(struct l2cap_conn *conn, __le16 psm,
BT_DBG("chan %p, len %d", chan, skb->len);
+ l2cap_chan_lock(chan); + if (chan->state != BT_BOUND && chan->state != BT_CONNECTED) goto drop;
@@ -7729,6 +7731,7 @@ static void l2cap_conless_channel(struct l2cap_conn *conn, __le16 psm, }
drop: + l2cap_chan_unlock(chan); l2cap_chan_put(chan); free_skb: kfree_skb(skb); diff --git a/net/bluetooth/l2cap_sock.c b/net/bluetooth/l2cap_sock.c index c7217d3ecd8b..51927765473b 100644 --- a/net/bluetooth/l2cap_sock.c +++ b/net/bluetooth/l2cap_sock.c @@ -1487,17 +1487,9 @@ static int l2cap_sock_recv_cb(struct l2cap_chan *chan, struct sk_buff *skb) struct sock *sk; int err;
- /* To avoid race with sock_release, a chan lock needs to be added here - * to synchronize the sock. - */ - l2cap_chan_hold(chan); - l2cap_chan_lock(chan); sk = chan->data; - if (!sk) { - l2cap_chan_unlock(chan); - l2cap_chan_put(chan); + if (!sk) return -ENXIO; - }
lock_sock(sk);
@@ -1535,8 +1527,6 @@ static int l2cap_sock_recv_cb(struct l2cap_chan *chan, struct sk_buff *skb)
done: release_sock(sk); - l2cap_chan_unlock(chan); - l2cap_chan_put(chan);
return err; }
From: Luiz Augusto von Dentz luiz.von.dentz@intel.com
mainline inclusion from mainline-v6.11-rc1 commit 87be7b189b2c50d4b51512f59e4e97db4eedee8a category: bugfix bugzilla: https://gitee.com/src-openeuler/kernel/issues/IAGEK1 CVE: CVE-2024-41062
Reference: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?i...
-------------------------------------------
__hci_cmd_sync_status shall only be used if hci_req_sync_lock is _not_ required which is not the case of hci_dev_cmd so it needs to use hci_cmd_sync_status which uses hci_req_sync_lock internally.
Fixes: f1a8f402f13f ("Bluetooth: L2CAP: Fix deadlock") Reported-by: Pauli Virtanen pav@iki.fi Signed-off-by: Luiz Augusto von Dentz luiz.von.dentz@intel.com Signed-off-by: Zhengchao Shao shaozhengchao@huawei.com --- net/bluetooth/hci_core.c | 27 ++++++++++++--------------- 1 file changed, 12 insertions(+), 15 deletions(-)
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c index c24b7c7dddbc..5754141e91a6 100644 --- a/net/bluetooth/hci_core.c +++ b/net/bluetooth/hci_core.c @@ -1987,8 +1987,8 @@ int hci_dev_cmd(unsigned int cmd, void __user *arg)
switch (cmd) { case HCISETAUTH: - err = __hci_cmd_sync_status(hdev, HCI_OP_WRITE_AUTH_ENABLE, - 1, &dr.dev_opt, HCI_CMD_TIMEOUT); + err = hci_cmd_sync_status(hdev, HCI_OP_WRITE_AUTH_ENABLE, + 1, &dr.dev_opt, HCI_CMD_TIMEOUT); break;
case HCISETENCRYPT: @@ -1999,23 +1999,21 @@ int hci_dev_cmd(unsigned int cmd, void __user *arg)
if (!test_bit(HCI_AUTH, &hdev->flags)) { /* Auth must be enabled first */ - err = __hci_cmd_sync_status(hdev, - HCI_OP_WRITE_AUTH_ENABLE, - 1, &dr.dev_opt, - HCI_CMD_TIMEOUT); + err = hci_cmd_sync_status(hdev, + HCI_OP_WRITE_AUTH_ENABLE, + 1, &dr.dev_opt, + HCI_CMD_TIMEOUT); if (err) break; }
- err = __hci_cmd_sync_status(hdev, HCI_OP_WRITE_ENCRYPT_MODE, - 1, &dr.dev_opt, - HCI_CMD_TIMEOUT); + err = hci_cmd_sync_status(hdev, HCI_OP_WRITE_ENCRYPT_MODE, + 1, &dr.dev_opt, HCI_CMD_TIMEOUT); break;
case HCISETSCAN: - err = __hci_cmd_sync_status(hdev, HCI_OP_WRITE_SCAN_ENABLE, - 1, &dr.dev_opt, - HCI_CMD_TIMEOUT); + err = hci_cmd_sync_status(hdev, HCI_OP_WRITE_SCAN_ENABLE, + 1, &dr.dev_opt, HCI_CMD_TIMEOUT);
/* Ensure that the connectable and discoverable states * get correctly modified as this was a non-mgmt change. @@ -2027,9 +2025,8 @@ int hci_dev_cmd(unsigned int cmd, void __user *arg) case HCISETLINKPOL: policy = cpu_to_le16(dr.dev_opt);
- err = __hci_cmd_sync_status(hdev, HCI_OP_WRITE_DEF_LINK_POLICY, - 2, &policy, - HCI_CMD_TIMEOUT); + err = hci_cmd_sync_status(hdev, HCI_OP_WRITE_DEF_LINK_POLICY, + 2, &policy, HCI_CMD_TIMEOUT); break;
case HCISETLINKMODE:
反馈: 您发送到kernel@openeuler.org的补丁/补丁集,已成功转换为PR! PR链接地址: https://gitee.com/openeuler/kernel/pulls/10592 邮件列表地址:https://mailweb.openeuler.org/hyperkitty/list/kernel@openeuler.org/message/G...
FeedBack: The patch(es) which you have sent to kernel@openeuler.org mailing list has been converted to a pull request successfully! Pull request link: https://gitee.com/openeuler/kernel/pulls/10592 Mailing list address: https://mailweb.openeuler.org/hyperkitty/list/kernel@openeuler.org/message/G...