Re: [PATCH] Bluetooth: skip invalid hci_sync_conn_complete_evt

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

Hi Desmond,

quoted

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.

quoted

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@syzkaller.appspotmail.com
Tested-by: syzbot+66264bf2fd0476be7e6c@syzkaller.appspotmail.com
Signed-off-by: Desmond Cheong Zhi Xi <redacted>
---
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.

quoted

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

Regards

Marcel