Skip to content

Commit 2b2bf47

Browse files
1sealVudentz
authored andcommitted
Bluetooth: hci_event: move wake reason storage into validated event handlers
hci_store_wake_reason() is called from hci_event_packet() immediately after stripping the HCI event header but before hci_event_func() enforces the per-event minimum payload length from hci_ev_table. This means a short HCI event frame can reach bacpy() before any bounds check runs. Rather than duplicating skb parsing and per-event length checks inside hci_store_wake_reason(), move wake-address storage into the individual event handlers after their existing event-length validation has succeeded. Convert hci_store_wake_reason() into a small helper that only stores an already-validated bdaddr while the caller holds hci_dev_lock(). Use the same helper after hci_event_func() with a NULL address to preserve the existing unexpected-wake fallback semantics when no validated event handler records a wake address. Annotate the helper with __must_hold(&hdev->lock) and add lockdep_assert_held(&hdev->lock) so future call paths keep the lock contract explicit. Call the helper from hci_conn_request_evt(), hci_conn_complete_evt(), hci_sync_conn_complete_evt(), le_conn_complete_evt(), hci_le_adv_report_evt(), hci_le_ext_adv_report_evt(), hci_le_direct_adv_report_evt(), hci_le_pa_sync_established_evt(), and hci_le_past_received_evt(). Fixes: 2f20216 ("Bluetooth: Emit controller suspend and resume events") Cc: stable@vger.kernel.org Signed-off-by: Oleh Konko <security@1seal.org> Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
1 parent 8a5b013 commit 2b2bf47

1 file changed

Lines changed: 35 additions & 59 deletions

File tree

net/bluetooth/hci_event.c

Lines changed: 35 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,10 @@ static void *hci_le_ev_skb_pull(struct hci_dev *hdev, struct sk_buff *skb,
8080
return data;
8181
}
8282

83+
static void hci_store_wake_reason(struct hci_dev *hdev,
84+
const bdaddr_t *bdaddr, u8 addr_type)
85+
__must_hold(&hdev->lock);
86+
8387
static u8 hci_cc_inquiry_cancel(struct hci_dev *hdev, void *data,
8488
struct sk_buff *skb)
8589
{
@@ -3111,6 +3115,7 @@ static void hci_conn_complete_evt(struct hci_dev *hdev, void *data,
31113115
bt_dev_dbg(hdev, "status 0x%2.2x", status);
31123116

31133117
hci_dev_lock(hdev);
3118+
hci_store_wake_reason(hdev, &ev->bdaddr, BDADDR_BREDR);
31143119

31153120
/* Check for existing connection:
31163121
*
@@ -3274,6 +3279,10 @@ static void hci_conn_request_evt(struct hci_dev *hdev, void *data,
32743279

32753280
bt_dev_dbg(hdev, "bdaddr %pMR type 0x%x", &ev->bdaddr, ev->link_type);
32763281

3282+
hci_dev_lock(hdev);
3283+
hci_store_wake_reason(hdev, &ev->bdaddr, BDADDR_BREDR);
3284+
hci_dev_unlock(hdev);
3285+
32773286
/* Reject incoming connection from device with same BD ADDR against
32783287
* CVE-2020-26555
32793288
*/
@@ -5021,6 +5030,7 @@ static void hci_sync_conn_complete_evt(struct hci_dev *hdev, void *data,
50215030
bt_dev_dbg(hdev, "status 0x%2.2x", status);
50225031

50235032
hci_dev_lock(hdev);
5033+
hci_store_wake_reason(hdev, &ev->bdaddr, BDADDR_BREDR);
50245034

50255035
conn = hci_conn_hash_lookup_ba(hdev, ev->link_type, &ev->bdaddr);
50265036
if (!conn) {
@@ -5713,6 +5723,7 @@ static void le_conn_complete_evt(struct hci_dev *hdev, u8 status,
57135723
int err;
57145724

57155725
hci_dev_lock(hdev);
5726+
hci_store_wake_reason(hdev, bdaddr, bdaddr_type);
57165727

57175728
/* All controllers implicitly stop advertising in the event of a
57185729
* connection, so ensure that the state bit is cleared.
@@ -6005,6 +6016,7 @@ static void hci_le_past_received_evt(struct hci_dev *hdev, void *data,
60056016
bt_dev_dbg(hdev, "status 0x%2.2x", ev->status);
60066017

60076018
hci_dev_lock(hdev);
6019+
hci_store_wake_reason(hdev, &ev->bdaddr, ev->bdaddr_type);
60086020

60096021
hci_dev_clear_flag(hdev, HCI_PA_SYNC);
60106022

@@ -6403,6 +6415,8 @@ static void hci_le_adv_report_evt(struct hci_dev *hdev, void *data,
64036415
info->length + 1))
64046416
break;
64056417

6418+
hci_store_wake_reason(hdev, &info->bdaddr, info->bdaddr_type);
6419+
64066420
if (info->length <= max_adv_len(hdev)) {
64076421
rssi = info->data[info->length];
64086422
process_adv_report(hdev, info->type, &info->bdaddr,
@@ -6491,6 +6505,8 @@ static void hci_le_ext_adv_report_evt(struct hci_dev *hdev, void *data,
64916505
info->length))
64926506
break;
64936507

6508+
hci_store_wake_reason(hdev, &info->bdaddr, info->bdaddr_type);
6509+
64946510
evt_type = __le16_to_cpu(info->type) & LE_EXT_ADV_EVT_TYPE_MASK;
64956511
legacy_evt_type = ext_evt_type_to_legacy(hdev, evt_type);
64966512

@@ -6536,6 +6552,7 @@ static void hci_le_pa_sync_established_evt(struct hci_dev *hdev, void *data,
65366552
bt_dev_dbg(hdev, "status 0x%2.2x", ev->status);
65376553

65386554
hci_dev_lock(hdev);
6555+
hci_store_wake_reason(hdev, &ev->bdaddr, ev->bdaddr_type);
65396556

65406557
hci_dev_clear_flag(hdev, HCI_PA_SYNC);
65416558

@@ -6834,6 +6851,8 @@ static void hci_le_direct_adv_report_evt(struct hci_dev *hdev, void *data,
68346851
for (i = 0; i < ev->num; i++) {
68356852
struct hci_ev_le_direct_adv_info *info = &ev->info[i];
68366853

6854+
hci_store_wake_reason(hdev, &info->bdaddr, info->bdaddr_type);
6855+
68376856
process_adv_report(hdev, info->type, &info->bdaddr,
68386857
info->bdaddr_type, &info->direct_addr,
68396858
info->direct_addr_type, HCI_ADV_PHY_1M, 0,
@@ -7517,73 +7536,29 @@ static bool hci_get_cmd_complete(struct hci_dev *hdev, u16 opcode,
75177536
return true;
75187537
}
75197538

7520-
static void hci_store_wake_reason(struct hci_dev *hdev, u8 event,
7521-
struct sk_buff *skb)
7539+
static void hci_store_wake_reason(struct hci_dev *hdev,
7540+
const bdaddr_t *bdaddr, u8 addr_type)
7541+
__must_hold(&hdev->lock)
75227542
{
7523-
struct hci_ev_le_advertising_info *adv;
7524-
struct hci_ev_le_direct_adv_info *direct_adv;
7525-
struct hci_ev_le_ext_adv_info *ext_adv;
7526-
const struct hci_ev_conn_complete *conn_complete = (void *)skb->data;
7527-
const struct hci_ev_conn_request *conn_request = (void *)skb->data;
7528-
7529-
hci_dev_lock(hdev);
7543+
lockdep_assert_held(&hdev->lock);
75307544

75317545
/* If we are currently suspended and this is the first BT event seen,
75327546
* save the wake reason associated with the event.
75337547
*/
75347548
if (!hdev->suspended || hdev->wake_reason)
7535-
goto unlock;
7549+
return;
7550+
7551+
if (!bdaddr) {
7552+
hdev->wake_reason = MGMT_WAKE_REASON_UNEXPECTED;
7553+
return;
7554+
}
75367555

75377556
/* Default to remote wake. Values for wake_reason are documented in the
75387557
* Bluez mgmt api docs.
75397558
*/
75407559
hdev->wake_reason = MGMT_WAKE_REASON_REMOTE_WAKE;
7541-
7542-
/* Once configured for remote wakeup, we should only wake up for
7543-
* reconnections. It's useful to see which device is waking us up so
7544-
* keep track of the bdaddr of the connection event that woke us up.
7545-
*/
7546-
if (event == HCI_EV_CONN_REQUEST) {
7547-
bacpy(&hdev->wake_addr, &conn_request->bdaddr);
7548-
hdev->wake_addr_type = BDADDR_BREDR;
7549-
} else if (event == HCI_EV_CONN_COMPLETE) {
7550-
bacpy(&hdev->wake_addr, &conn_complete->bdaddr);
7551-
hdev->wake_addr_type = BDADDR_BREDR;
7552-
} else if (event == HCI_EV_LE_META) {
7553-
struct hci_ev_le_meta *le_ev = (void *)skb->data;
7554-
u8 subevent = le_ev->subevent;
7555-
u8 *ptr = &skb->data[sizeof(*le_ev)];
7556-
u8 num_reports = *ptr;
7557-
7558-
if ((subevent == HCI_EV_LE_ADVERTISING_REPORT ||
7559-
subevent == HCI_EV_LE_DIRECT_ADV_REPORT ||
7560-
subevent == HCI_EV_LE_EXT_ADV_REPORT) &&
7561-
num_reports) {
7562-
adv = (void *)(ptr + 1);
7563-
direct_adv = (void *)(ptr + 1);
7564-
ext_adv = (void *)(ptr + 1);
7565-
7566-
switch (subevent) {
7567-
case HCI_EV_LE_ADVERTISING_REPORT:
7568-
bacpy(&hdev->wake_addr, &adv->bdaddr);
7569-
hdev->wake_addr_type = adv->bdaddr_type;
7570-
break;
7571-
case HCI_EV_LE_DIRECT_ADV_REPORT:
7572-
bacpy(&hdev->wake_addr, &direct_adv->bdaddr);
7573-
hdev->wake_addr_type = direct_adv->bdaddr_type;
7574-
break;
7575-
case HCI_EV_LE_EXT_ADV_REPORT:
7576-
bacpy(&hdev->wake_addr, &ext_adv->bdaddr);
7577-
hdev->wake_addr_type = ext_adv->bdaddr_type;
7578-
break;
7579-
}
7580-
}
7581-
} else {
7582-
hdev->wake_reason = MGMT_WAKE_REASON_UNEXPECTED;
7583-
}
7584-
7585-
unlock:
7586-
hci_dev_unlock(hdev);
7560+
bacpy(&hdev->wake_addr, bdaddr);
7561+
hdev->wake_addr_type = addr_type;
75877562
}
75887563

75897564
#define HCI_EV_VL(_op, _func, _min_len, _max_len) \
@@ -7830,14 +7805,15 @@ void hci_event_packet(struct hci_dev *hdev, struct sk_buff *skb)
78307805

78317806
skb_pull(skb, HCI_EVENT_HDR_SIZE);
78327807

7833-
/* Store wake reason if we're suspended */
7834-
hci_store_wake_reason(hdev, event, skb);
7835-
78367808
bt_dev_dbg(hdev, "event 0x%2.2x", event);
78377809

78387810
hci_event_func(hdev, event, skb, &opcode, &status, &req_complete,
78397811
&req_complete_skb);
78407812

7813+
hci_dev_lock(hdev);
7814+
hci_store_wake_reason(hdev, NULL, 0);
7815+
hci_dev_unlock(hdev);
7816+
78417817
if (req_complete) {
78427818
req_complete(hdev, status, opcode);
78437819
} else if (req_complete_skb) {

0 commit comments

Comments
 (0)