2021-04-16 22:09:33

by Luiz Augusto von Dentz

[permalink] [raw]
Subject: [PATCH 00/10] Bluetooth: HCI: Use skb_pull to parse events

From: Luiz Augusto von Dentz <[email protected]>

This set ensures events received have the minimum required length using
skb_pull to advance on packet, it also rework some of events to take
advantage flex_array_size for events that can have variable size.

This should fix issues found by szybot like:

[syzbot] KMSAN: uninit-value in hci_event_packet

Luiz Augusto von Dentz (10):
Bluetooth: HCI: Use skb_pull to parse BR/EDR events
Bluetooth: HCI: Use skb_pull to parse Command Complete event
Bluetooth: HCI: Use skb_pull to parse Number of Complete Packets event
Bluetooth: HCI: Use skb_pull to parse Inquiry Result event
Bluetooth: HCI: Use skb_pull to parse Inquiry Result with RSSI event
Bluetooth: HCI: Use skb_pull to parse Extended Inquiry Result event
Bluetooth: HCI: Use skb_pull to parse LE Metaevents
Bluetooth: HCI: Use skb_pull to parse LE Advertising Report event
Bluetooth: HCI: Use skb_pull to parse LE Extended Advertising Report
event
Bluetooth: HCI: Use skb_pull to parse LE Direct Advertising Report
event

include/net/bluetooth/hci.h | 59 +-
net/bluetooth/hci_event.c | 1312 +++++++++++++++++++++++++++--------
2 files changed, 1052 insertions(+), 319 deletions(-)

--
2.30.2


2021-04-16 22:09:52

by Luiz Augusto von Dentz

[permalink] [raw]
Subject: [PATCH 09/10] Bluetooth: HCI: Use skb_pull to parse LE Extended Advertising Report event

From: Luiz Augusto von Dentz <[email protected]>

This uses skb_pull to check the LE Extended Advertising Report events
received have the minimum required length.

Signed-off-by: Luiz Augusto von Dentz <[email protected]>
---
include/net/bluetooth/hci.h | 17 +++++++++++------
net/bluetooth/hci_event.c | 36 +++++++++++++++++++++++++-----------
2 files changed, 36 insertions(+), 17 deletions(-)

diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
index 3ec8e07f1724..9600cc6ad952 100644
--- a/include/net/bluetooth/hci.h
+++ b/include/net/bluetooth/hci.h
@@ -2399,8 +2399,8 @@ struct hci_ev_le_phy_update_complete {
} __packed;

#define HCI_EV_LE_EXT_ADV_REPORT 0x0d
-struct hci_ev_le_ext_adv_report {
- __le16 evt_type;
+struct hci_ev_le_ext_adv_info {
+ __le16 type;
__u8 bdaddr_type;
bdaddr_t bdaddr;
__u8 primary_phy;
@@ -2408,11 +2408,16 @@ struct hci_ev_le_ext_adv_report {
__u8 sid;
__u8 tx_power;
__s8 rssi;
- __le16 interval;
- __u8 direct_addr_type;
+ __le16 interval;
+ __u8 direct_addr_type;
bdaddr_t direct_addr;
- __u8 length;
- __u8 data[];
+ __u8 length;
+ __u8 data[];
+} __packed;
+
+struct hci_ev_le_ext_adv_report {
+ __u8 num;
+ struct hci_ev_le_ext_adv_info info[];
} __packed;

#define HCI_EV_LE_ENHANCED_CONN_COMPLETE 0x0a
diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index becc6319c8c5..3fdab3fe427d 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -6360,26 +6360,40 @@ static u8 ext_evt_type_to_legacy(struct hci_dev *hdev, u16 evt_type)

static void hci_le_ext_adv_report_evt(struct hci_dev *hdev, struct sk_buff *skb)
{
- u8 num_reports = skb->data[0];
- void *ptr = &skb->data[1];
+ struct hci_ev_le_ext_adv_report *ev;
+
+ ev = hci_le_ev_skb_pull(hdev, skb, HCI_EV_LE_EXT_ADV_REPORT,
+ sizeof(*ev));
+ if (!ev)
+ return;
+
+ if (!ev->num)
+ return;

hci_dev_lock(hdev);

- while (num_reports--) {
- struct hci_ev_le_ext_adv_report *ev = ptr;
+ while (ev->num--) {
+ struct hci_ev_le_ext_adv_info *info;
u8 legacy_evt_type;
u16 evt_type;

- evt_type = __le16_to_cpu(ev->evt_type);
+ info = hci_le_ev_skb_pull(hdev, skb, HCI_EV_LE_EXT_ADV_REPORT,
+ sizeof(*info));
+ if (!info)
+ break;
+
+ if (!hci_le_ev_skb_pull(hdev, skb, HCI_EV_LE_EXT_ADV_REPORT,
+ info->length))
+ break;
+
+ evt_type = __le16_to_cpu(info->type);
legacy_evt_type = ext_evt_type_to_legacy(hdev, evt_type);
if (legacy_evt_type != LE_ADV_INVALID) {
- process_adv_report(hdev, legacy_evt_type, &ev->bdaddr,
- ev->bdaddr_type, NULL, 0, ev->rssi,
- ev->data, ev->length,
+ process_adv_report(hdev, legacy_evt_type, &info->bdaddr,
+ info->bdaddr_type, NULL, 0,
+ info->rssi, info->data, info->length,
!(evt_type & LE_EXT_ADV_LEGACY_PDU));
}
-
- ptr += sizeof(*ev) + ev->length;
}

hci_dev_unlock(hdev);
@@ -6730,7 +6744,7 @@ static void hci_store_wake_reason(struct hci_dev *hdev, u8 event,
{
struct hci_ev_le_advertising_info *adv;
struct hci_ev_le_direct_adv_info *direct_adv;
- struct hci_ev_le_ext_adv_report *ext_adv;
+ struct hci_ev_le_ext_adv_info *ext_adv;
const struct hci_ev_conn_complete *conn_complete = (void *)skb->data;
const struct hci_ev_conn_request *conn_request = (void *)skb->data;

--
2.30.2