Re: [PATCH] Bluetooth: skip invalid hci_sync_conn_complete_evt

From: Desmond Cheong Zhi Xi
Date: Wed Jul 28 2021 - 02:47:35 EST


On 22/7/21 10:39 pm, Marcel Holtmann wrote:
Hi Desmond,

Syzbot reported a corrupted list in kobject_add_internal [1]. This
happens when multiple HCI_EV_SYNC_CONN_COMPLETE event packets with
status 0 are sent for the same HCI connection. This causes us to
register the device more than once which corrupts the kset list.

and that is actually forbidden by the spec. So we need to complain loudly that such a device is misbehaving.

To fix this, in hci_sync_conn_complete_evt, we check whether we're
trying to process the same HCI_EV_SYNC_CONN_COMPLETE event multiple
times for one connection. If that's the case, the event is invalid, so
we skip further processing and exit.

Link: https://syzkaller.appspot.com/bug?extid=66264bf2fd0476be7e6c [1]
Reported-by: syzbot+66264bf2fd0476be7e6c@xxxxxxxxxxxxxxxxxxxxxxxxx
Tested-by: syzbot+66264bf2fd0476be7e6c@xxxxxxxxxxxxxxxxxxxxxxxxx
Signed-off-by: Desmond Cheong Zhi Xi <desmondcheongzx@xxxxxxxxx>
---
net/bluetooth/hci_event.c | 2 ++
1 file changed, 2 insertions(+)

diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index 016b2999f219..091a92338492 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -4373,6 +4373,8 @@ static void hci_sync_conn_complete_evt(struct hci_dev *hdev,

switch (ev->status) {
case 0x00:
+ if (conn->state == BT_CONNECTED)
+ goto unlock; /* Already connected, event not valid */

The comment has go above and be a lot more details since this is not expected behavior from valid hardware and we should add a bt_dev_err as well.


Hi Marcel,

Apologies for the delayed response.

Thanks for the feedback, I'll add more elaboration for the new check and add a bt_dev_err in a v2 patch.

conn->handle = __le16_to_cpu(ev->handle);
conn->state = BT_CONNECTED;
conn->type = ev->link_type;

Regards

Marcel