From 21b39fa2051084611221c6aa80ba0615c1200c61 Mon Sep 17 00:00:00 2001 From: Greg Kroah-Hartman Date: Wed, 13 Nov 2024 15:42:18 +0100 Subject: [PATCH 1/3] Revert "Bluetooth: fix use-after-free in accessing skb after sending it" MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This reverts commit 715264ad09fd4004e347cdb79fa58a4f2344f13f which is commit 947ec0d002dce8577b655793dcc6fc78d67b7cb6 upstream. It is reported to cause regressions in the 6.1.y tree, so revert it for now. Link: https://lore.kernel.org/all/CADRbXaDqx6S+7tzdDPPEpRu9eDLrHQkqoWTTGfKJSRxY=hT5MQ@mail.gmail.com/ Reported-by: Jeremy Lainé Cc: Salvatore Bonaccorso Cc: Mike Cc: Marcel Holtmann Cc: Johan Hedberg Cc: Paul Menzel Cc: Pauli Virtanen Cc: Luiz Augusto von Dentz Cc: Sasha Levin Signed-off-by: Greg Kroah-Hartman --- net/bluetooth/hci_core.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c index 993b98257bc2..796d9258787c 100644 --- a/net/bluetooth/hci_core.c +++ b/net/bluetooth/hci_core.c @@ -4146,7 +4146,7 @@ static void hci_send_cmd_sync(struct hci_dev *hdev, struct sk_buff *skb) if (hci_req_status_pend(hdev) && !hci_dev_test_and_set_flag(hdev, HCI_CMD_PENDING)) { kfree_skb(hdev->req_skb); - hdev->req_skb = skb_clone(hdev->sent_cmd, GFP_KERNEL); + hdev->req_skb = skb_clone(skb, GFP_KERNEL); } atomic_dec(&hdev->cmd_cnt); From 0337fb092873a4146aa854acb554675b49c8f6b5 Mon Sep 17 00:00:00 2001 From: Greg Kroah-Hartman Date: Wed, 13 Nov 2024 15:42:48 +0100 Subject: [PATCH 2/3] Revert "Bluetooth: hci_sync: Fix overwriting request callback" MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This reverts commit da77c1d39bc527b31890bfa0405763c82828defb which is commit 2615fd9a7c2507eb3be3fbe49dcec88a2f56454a upstream. It is reported to cause regressions in the 6.1.y tree, so revert it for now. Link: https://lore.kernel.org/all/CADRbXaDqx6S+7tzdDPPEpRu9eDLrHQkqoWTTGfKJSRxY=hT5MQ@mail.gmail.com/ Reported-by: Jeremy Lainé Cc: Salvatore Bonaccorso Cc: Mike Cc: Marcel Holtmann Cc: Johan Hedberg Cc: Paul Menzel Cc: Pauli Virtanen Cc: Luiz Augusto von Dentz Cc: Sasha Levin Signed-off-by: Greg Kroah-Hartman --- include/net/bluetooth/hci_core.h | 1 - net/bluetooth/hci_conn.c | 2 +- net/bluetooth/hci_core.c | 46 ++++++++++---------------------- net/bluetooth/hci_event.c | 18 ++++++------- net/bluetooth/hci_sync.c | 21 +++------------ 5 files changed, 27 insertions(+), 61 deletions(-) diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h index 215b56dc26df..ad1efeb135d6 100644 --- a/include/net/bluetooth/hci_core.h +++ b/include/net/bluetooth/hci_core.h @@ -544,7 +544,6 @@ struct hci_dev { __u32 req_status; __u32 req_result; struct sk_buff *req_skb; - struct sk_buff *req_rsp; void *smp_data; void *smp_bredr_data; diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c index 5ec2160108a1..ba708974fa51 100644 --- a/net/bluetooth/hci_conn.c +++ b/net/bluetooth/hci_conn.c @@ -2816,7 +2816,7 @@ int hci_abort_conn(struct hci_conn *conn, u8 reason) case HCI_EV_LE_CONN_COMPLETE: case HCI_EV_LE_ENHANCED_CONN_COMPLETE: case HCI_EVT_LE_CIS_ESTABLISHED: - hci_cmd_sync_cancel(hdev, ECANCELED); + hci_cmd_sync_cancel(hdev, -ECANCELED); break; } } diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c index 796d9258787c..baa1fb498cf1 100644 --- a/net/bluetooth/hci_core.c +++ b/net/bluetooth/hci_core.c @@ -1452,8 +1452,8 @@ static void hci_cmd_timeout(struct work_struct *work) struct hci_dev *hdev = container_of(work, struct hci_dev, cmd_timer.work); - if (hdev->req_skb) { - u16 opcode = hci_skb_opcode(hdev->req_skb); + if (hdev->sent_cmd) { + u16 opcode = hci_skb_opcode(hdev->sent_cmd); bt_dev_err(hdev, "command 0x%4.4x tx timeout", opcode); @@ -2762,7 +2762,6 @@ void hci_release_dev(struct hci_dev *hdev) ida_simple_remove(&hci_index_ida, hdev->id); kfree_skb(hdev->sent_cmd); - kfree_skb(hdev->req_skb); kfree_skb(hdev->recv_event); kfree(hdev); } @@ -3092,33 +3091,21 @@ int __hci_cmd_send(struct hci_dev *hdev, u16 opcode, u32 plen, EXPORT_SYMBOL(__hci_cmd_send); /* Get data from the previously sent command */ -static void *hci_cmd_data(struct sk_buff *skb, __u16 opcode) +void *hci_sent_cmd_data(struct hci_dev *hdev, __u16 opcode) { struct hci_command_hdr *hdr; - if (!skb || skb->len < HCI_COMMAND_HDR_SIZE) + if (!hdev->sent_cmd) return NULL; - hdr = (void *)skb->data; + hdr = (void *) hdev->sent_cmd->data; if (hdr->opcode != cpu_to_le16(opcode)) return NULL; - return skb->data + HCI_COMMAND_HDR_SIZE; -} + BT_DBG("%s opcode 0x%4.4x", hdev->name, opcode); -/* Get data from the previously sent command */ -void *hci_sent_cmd_data(struct hci_dev *hdev, __u16 opcode) -{ - void *data; - - /* Check if opcode matches last sent command */ - data = hci_cmd_data(hdev->sent_cmd, opcode); - if (!data) - /* Check if opcode matches last request */ - data = hci_cmd_data(hdev->req_skb, opcode); - - return data; + return hdev->sent_cmd->data + HCI_COMMAND_HDR_SIZE; } /* Get data from last received event */ @@ -4014,19 +4001,17 @@ void hci_req_cmd_complete(struct hci_dev *hdev, u16 opcode, u8 status, if (!status && !hci_req_is_complete(hdev)) return; - skb = hdev->req_skb; - /* If this was the last command in a request the complete - * callback would be found in hdev->req_skb instead of the + * callback would be found in hdev->sent_cmd instead of the * command queue (hdev->cmd_q). */ - if (skb && bt_cb(skb)->hci.req_flags & HCI_REQ_SKB) { - *req_complete_skb = bt_cb(skb)->hci.req_complete_skb; + if (bt_cb(hdev->sent_cmd)->hci.req_flags & HCI_REQ_SKB) { + *req_complete_skb = bt_cb(hdev->sent_cmd)->hci.req_complete_skb; return; } - if (skb && bt_cb(skb)->hci.req_complete) { - *req_complete = bt_cb(skb)->hci.req_complete; + if (bt_cb(hdev->sent_cmd)->hci.req_complete) { + *req_complete = bt_cb(hdev->sent_cmd)->hci.req_complete; return; } @@ -4143,11 +4128,8 @@ static void hci_send_cmd_sync(struct hci_dev *hdev, struct sk_buff *skb) return; } - if (hci_req_status_pend(hdev) && - !hci_dev_test_and_set_flag(hdev, HCI_CMD_PENDING)) { - kfree_skb(hdev->req_skb); - hdev->req_skb = skb_clone(skb, GFP_KERNEL); - } + if (hci_req_status_pend(hdev)) + hci_dev_set_flag(hdev, HCI_CMD_PENDING); atomic_dec(&hdev->cmd_cnt); } diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c index 0c0141c59fd1..ac9bdfac773f 100644 --- a/net/bluetooth/hci_event.c +++ b/net/bluetooth/hci_event.c @@ -4354,7 +4354,7 @@ static void hci_cmd_status_evt(struct hci_dev *hdev, void *data, * (since for this kind of commands there will not be a command * complete event). */ - if (ev->status || (hdev->req_skb && !hci_skb_event(hdev->req_skb))) { + if (ev->status || (hdev->sent_cmd && !hci_skb_event(hdev->sent_cmd))) { hci_req_cmd_complete(hdev, *opcode, ev->status, req_complete, req_complete_skb); if (hci_dev_test_flag(hdev, HCI_CMD_PENDING)) { @@ -7171,10 +7171,10 @@ static void hci_le_meta_evt(struct hci_dev *hdev, void *data, bt_dev_dbg(hdev, "subevent 0x%2.2x", ev->subevent); /* Only match event if command OGF is for LE */ - if (hdev->req_skb && - hci_opcode_ogf(hci_skb_opcode(hdev->req_skb)) == 0x08 && - hci_skb_event(hdev->req_skb) == ev->subevent) { - *opcode = hci_skb_opcode(hdev->req_skb); + if (hdev->sent_cmd && + hci_opcode_ogf(hci_skb_opcode(hdev->sent_cmd)) == 0x08 && + hci_skb_event(hdev->sent_cmd) == ev->subevent) { + *opcode = hci_skb_opcode(hdev->sent_cmd); hci_req_cmd_complete(hdev, *opcode, 0x00, req_complete, req_complete_skb); } @@ -7561,10 +7561,10 @@ void hci_event_packet(struct hci_dev *hdev, struct sk_buff *skb) } /* Only match event if command OGF is not for LE */ - if (hdev->req_skb && - hci_opcode_ogf(hci_skb_opcode(hdev->req_skb)) != 0x08 && - hci_skb_event(hdev->req_skb) == event) { - hci_req_cmd_complete(hdev, hci_skb_opcode(hdev->req_skb), + if (hdev->sent_cmd && + hci_opcode_ogf(hci_skb_opcode(hdev->sent_cmd)) != 0x08 && + hci_skb_event(hdev->sent_cmd) == event) { + hci_req_cmd_complete(hdev, hci_skb_opcode(hdev->sent_cmd), status, &req_complete, &req_complete_skb); req_evt = event; } diff --git a/net/bluetooth/hci_sync.c b/net/bluetooth/hci_sync.c index c368235202b2..5a6d70d6424f 100644 --- a/net/bluetooth/hci_sync.c +++ b/net/bluetooth/hci_sync.c @@ -31,10 +31,6 @@ static void hci_cmd_sync_complete(struct hci_dev *hdev, u8 result, u16 opcode, hdev->req_result = result; hdev->req_status = HCI_REQ_DONE; - /* Free the request command so it is not used as response */ - kfree_skb(hdev->req_skb); - hdev->req_skb = NULL; - if (skb) { struct sock *sk = hci_skb_sk(skb); @@ -42,7 +38,7 @@ static void hci_cmd_sync_complete(struct hci_dev *hdev, u8 result, u16 opcode, if (sk) sock_put(sk); - hdev->req_rsp = skb_get(skb); + hdev->req_skb = skb_get(skb); } wake_up_interruptible(&hdev->req_wait_q); @@ -190,8 +186,8 @@ struct sk_buff *__hci_cmd_sync_sk(struct hci_dev *hdev, u16 opcode, u32 plen, hdev->req_status = 0; hdev->req_result = 0; - skb = hdev->req_rsp; - hdev->req_rsp = NULL; + skb = hdev->req_skb; + hdev->req_skb = NULL; bt_dev_dbg(hdev, "end: err %d", err); @@ -4941,11 +4937,6 @@ int hci_dev_open_sync(struct hci_dev *hdev) hdev->sent_cmd = NULL; } - if (hdev->req_skb) { - kfree_skb(hdev->req_skb); - hdev->req_skb = NULL; - } - clear_bit(HCI_RUNNING, &hdev->flags); hci_sock_dev_event(hdev, HCI_DEV_CLOSE); @@ -5107,12 +5098,6 @@ int hci_dev_close_sync(struct hci_dev *hdev) hdev->sent_cmd = NULL; } - /* Drop last request */ - if (hdev->req_skb) { - kfree_skb(hdev->req_skb); - hdev->req_skb = NULL; - } - clear_bit(HCI_RUNNING, &hdev->flags); hci_sock_dev_event(hdev, HCI_DEV_CLOSE); From 0625d7c240b307b78640dcd823cb738cb900a8ba Mon Sep 17 00:00:00 2001 From: Greg Kroah-Hartman Date: Wed, 13 Nov 2024 15:43:55 +0100 Subject: [PATCH 3/3] Revert "Bluetooth: af_bluetooth: Fix deadlock" MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This reverts commit cb8adca52f306563d958a863bb0cbae9c184d1ae which is commit f7b94bdc1ec107c92262716b073b3e816d4784fb upstream. It is reported to cause regressions in the 6.1.y tree, so revert it for now. Link: https://lore.kernel.org/all/CADRbXaDqx6S+7tzdDPPEpRu9eDLrHQkqoWTTGfKJSRxY=hT5MQ@mail.gmail.com/ Reported-by: Jeremy Lainé Cc: Salvatore Bonaccorso Cc: Mike Cc: Marcel Holtmann Cc: Johan Hedberg Cc: Paul Menzel Cc: Pauli Virtanen Cc: Luiz Augusto von Dentz Cc: Sasha Levin Signed-off-by: Greg Kroah-Hartman --- net/bluetooth/af_bluetooth.c | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/net/bluetooth/af_bluetooth.c b/net/bluetooth/af_bluetooth.c index 4c3bd0516038..7960fd514e5a 100644 --- a/net/bluetooth/af_bluetooth.c +++ b/net/bluetooth/af_bluetooth.c @@ -307,11 +307,14 @@ int bt_sock_recvmsg(struct socket *sock, struct msghdr *msg, size_t len, if (flags & MSG_OOB) return -EOPNOTSUPP; + lock_sock(sk); + skb = skb_recv_datagram(sk, flags, &err); if (!skb) { if (sk->sk_shutdown & RCV_SHUTDOWN) err = 0; + release_sock(sk); return err; } @@ -337,6 +340,8 @@ int bt_sock_recvmsg(struct socket *sock, struct msghdr *msg, size_t len, skb_free_datagram(sk, skb); + release_sock(sk); + if (flags & MSG_TRUNC) copied = skblen; @@ -559,11 +564,10 @@ int bt_sock_ioctl(struct socket *sock, unsigned int cmd, unsigned long arg) if (sk->sk_state == BT_LISTEN) return -EINVAL; - spin_lock(&sk->sk_receive_queue.lock); + lock_sock(sk); skb = skb_peek(&sk->sk_receive_queue); amount = skb ? skb->len : 0; - spin_unlock(&sk->sk_receive_queue.lock); - + release_sock(sk); err = put_user(amount, (int __user *)arg); break;