Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bluetooth: Add a validation for le_conn_update_complete() and bt_hci_le_subrate_change_event() #79584

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions include/zephyr/bluetooth/hci_types.h
Original file line number Diff line number Diff line change
Expand Up @@ -2969,6 +2969,13 @@ struct bt_hci_evt_le_advertising_report {
struct bt_hci_evt_le_advertising_info adv_info[0];
} __packed;

/** All limits according to BT Core Spec v5.4 [Vol 4, Part E]. */
#define BT_HCI_LE_INTERVAL_MIN 0x0006
#define BT_HCI_LE_INTERVAL_MAX 0x0c80
#define BT_HCI_LE_PERIPHERAL_LATENCY_MAX 0x01f3
#define BT_HCI_LE_SUPERVISON_TIMEOUT_MIN 0x000a
#define BT_HCI_LE_SUPERVISON_TIMEOUT_MAX 0x0c80

#define BT_HCI_EVT_LE_CONN_UPDATE_COMPLETE 0x03
struct bt_hci_evt_le_conn_update_complete {
uint8_t status;
Expand Down Expand Up @@ -3338,6 +3345,11 @@ struct bt_hci_evt_le_biginfo_adv_report {
uint8_t encryption;
} __packed;

/** All limits according to BT Core Spec v5.4 [Vol 4, Part E]. */
#define BT_HCI_LE_SUBRATE_FACTOR_MIN 0x0001
#define BT_HCI_LE_SUBRATE_FACTOR_MAX 0x01f4
#define BT_HCI_LE_CONTINUATION_NUM_MAX 0x01f3

#define BT_HCI_EVT_LE_SUBRATE_CHANGE 0x23
struct bt_hci_evt_le_subrate_change {
uint8_t status;
Expand Down
33 changes: 33 additions & 0 deletions subsys/bluetooth/host/hci_core.c
Original file line number Diff line number Diff line change
Expand Up @@ -1929,6 +1929,20 @@ static void le_conn_update_complete(struct net_buf *buf)
conn->le.latency = sys_le16_to_cpu(evt->latency);
conn->le.timeout = sys_le16_to_cpu(evt->supv_timeout);

if (!IN_RANGE(conn->le.interval, BT_HCI_LE_INTERVAL_MIN,
BT_HCI_LE_INTERVAL_MAX)) {
LOG_WRN("interval exceeds the valid range 0x%04x",
conn->le.interval);
}
if (conn->le.latency > BT_HCI_LE_PERIPHERAL_LATENCY_MAX) {
LOG_WRN("latency exceeds the valid range 0x%04x", conn->le.latency);
}
if (!IN_RANGE(conn->le.timeout, BT_HCI_LE_SUPERVISON_TIMEOUT_MIN,
BT_HCI_LE_SUPERVISON_TIMEOUT_MAX)) {
LOG_WRN("supv_timeout exceeds the valid range 0x%04x",
conn->le.timeout);
}

#if defined(CONFIG_BT_GAP_AUTO_UPDATE_CONN_PARAMS)
atomic_clear_bit(conn->flags,
BT_CONN_PERIPHERAL_PARAM_AUTO_UPDATE);
Expand Down Expand Up @@ -2626,6 +2640,25 @@ void bt_hci_le_subrate_change_event(struct net_buf *buf)
conn->le.subrate.continuation_number = sys_le16_to_cpu(evt->continuation_number);
conn->le.latency = sys_le16_to_cpu(evt->peripheral_latency);
conn->le.timeout = sys_le16_to_cpu(evt->supervision_timeout);

if (!IN_RANGE(conn->le.subrate.factor, BT_HCI_LE_SUBRATE_FACTOR_MIN,
BT_HCI_LE_SUBRATE_FACTOR_MAX)) {
LOG_WRN("subrate_factor exceeds the valid range %d",
conn->le.subrate.factor);
}
if (conn->le.latency > BT_HCI_LE_PERIPHERAL_LATENCY_MAX) {
LOG_WRN("peripheral_latency exceeds the valid range 0x%04x",
conn->le.latency);
}
if (BT_HCI_LE_CONTINUATION_NUM_MAX < conn->le.subrate.continuation_number) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this not

Suggested change
if (BT_HCI_LE_CONTINUATION_NUM_MAX < conn->le.subrate.continuation_number) {
if (conn->le.subrate.continuation_number > BT_HCI_LE_CONTINUATION_NUM_MAX) {

LOG_WRN("continuation_number exceeds the valid range %d",
conn->le.subrate.continuation_number);
}
if (!IN_RANGE(conn->le.timeout, BT_HCI_LE_SUPERVISON_TIMEOUT_MIN,
BT_HCI_LE_SUPERVISON_TIMEOUT_MAX)) {
LOG_WRN("supervision_timeout exceeds the valid range 0x%04x",
conn->le.timeout);
}
}

params.status = evt->status;
Expand Down
Loading