Skip to content

Commit

Permalink
Bluetooth: HCI: Add validation for le_conn_update_complete() and bt_h…
Browse files Browse the repository at this point in the history
…ci_le_subrate_change_event()

Like 2ca179c, check and warn if incoming HCI event parameters exceed
the specification. This helps debugging controllers by detecting the
out-of-spec value that shouldn't appear.

Signed-off-by: Sungwoo Kim <iam@sung-woo.kim>
  • Loading branch information
swkim101 committed Oct 9, 2024
1 parent 9f60075 commit 29a7722
Show file tree
Hide file tree
Showing 2 changed files with 35 additions and 0 deletions.
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

Check notice on line 2975 in include/zephyr/bluetooth/hci_types.h

View workflow job for this annotation

GitHub Actions / Run compliance checks on patch series (PR)

You may want to run clang-format on this change

include/zephyr/bluetooth/hci_types.h:2975 -#define BT_HCI_LE_INTERVAL_MIN 0x0006 -#define BT_HCI_LE_INTERVAL_MAX 0x0c80 +#define BT_HCI_LE_INTERVAL_MIN 0x0006 +#define BT_HCI_LE_INTERVAL_MAX 0x0c80
#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

Check notice on line 3351 in include/zephyr/bluetooth/hci_types.h

View workflow job for this annotation

GitHub Actions / Run compliance checks on patch series (PR)

You may want to run clang-format on this change

include/zephyr/bluetooth/hci_types.h:3351 -#define BT_HCI_LE_SUBRATE_FACTOR_MIN 0x0001 -#define BT_HCI_LE_SUBRATE_FACTOR_MAX 0x01f4 +#define BT_HCI_LE_SUBRATE_FACTOR_MIN 0x0001 +#define BT_HCI_LE_SUBRATE_FACTOR_MAX 0x01f4

#define BT_HCI_EVT_LE_SUBRATE_CHANGE 0x23
struct bt_hci_evt_le_subrate_change {
uint8_t status;
Expand Down
23 changes: 23 additions & 0 deletions subsys/bluetooth/host/hci_core.c
Original file line number Diff line number Diff line change
Expand Up @@ -1929,6 +1929,16 @@ 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)) {

Check warning on line 1932 in subsys/bluetooth/host/hci_core.c

View workflow job for this annotation

GitHub Actions / Run compliance checks on patch series (PR)

LONG_LINE

subsys/bluetooth/host/hci_core.c:1932 line length of 107 exceeds 100 columns
LOG_WRN("interval exceeds the valid range 0x%04x", conn->le.interval);

Check warning on line 1933 in subsys/bluetooth/host/hci_core.c

View workflow job for this annotation

GitHub Actions / Run compliance checks on patch series (PR)

LONG_LINE

subsys/bluetooth/host/hci_core.c:1933 line length of 102 exceeds 100 columns
}
if (BT_HCI_LE_PERIPHERAL_LATENCY_MAX < conn->le.latency) {

Check warning on line 1935 in subsys/bluetooth/host/hci_core.c

View workflow job for this annotation

GitHub Actions / Run compliance checks on patch series (PR)

CONSTANT_COMPARISON

subsys/bluetooth/host/hci_core.c:1935 Comparisons should place the constant on the right side of the test
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)) {

Check warning on line 1938 in subsys/bluetooth/host/hci_core.c

View workflow job for this annotation

GitHub Actions / Run compliance checks on patch series (PR)

LONG_LINE

subsys/bluetooth/host/hci_core.c:1938 line length of 126 exceeds 100 columns
LOG_WRN("supv_timeout exceeds the valid range 0x%04x", conn->le.timeout);

Check warning on line 1939 in subsys/bluetooth/host/hci_core.c

View workflow job for this annotation

GitHub Actions / Run compliance checks on patch series (PR)

LONG_LINE

subsys/bluetooth/host/hci_core.c:1939 line length of 105 exceeds 100 columns
}

Check notice on line 1940 in subsys/bluetooth/host/hci_core.c

View workflow job for this annotation

GitHub Actions / Run compliance checks on patch series (PR)

You may want to run clang-format on this change

subsys/bluetooth/host/hci_core.c:1940 - 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 (!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 (BT_HCI_LE_PERIPHERAL_LATENCY_MAX < conn->le.latency) { 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 (!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 +2636,19 @@ 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)) {

Check warning on line 2640 in subsys/bluetooth/host/hci_core.c

View workflow job for this annotation

GitHub Actions / Run compliance checks on patch series (PR)

LONG_LINE

subsys/bluetooth/host/hci_core.c:2640 line length of 117 exceeds 100 columns
LOG_WRN("subrate_factor exceeds the valid range %d", conn->le.subrate.factor);

Check warning on line 2641 in subsys/bluetooth/host/hci_core.c

View workflow job for this annotation

GitHub Actions / Run compliance checks on patch series (PR)

LONG_LINE

subsys/bluetooth/host/hci_core.c:2641 line length of 102 exceeds 100 columns
}
if (BT_HCI_LE_PERIPHERAL_LATENCY_MAX < conn->le.peripheral_latency) {
LOG_WRN("peripheral_latency exceeds the valid range 0x%04x", conn->le.peripheral_latency);

Check warning on line 2644 in subsys/bluetooth/host/hci_core.c

View workflow job for this annotation

GitHub Actions / Run compliance checks on patch series (PR)

LONG_LINE

subsys/bluetooth/host/hci_core.c:2644 line length of 114 exceeds 100 columns
}
if (BT_HCI_LE_CONTINUATION_NUM_MAX < conn->le.continuation_number) {
LOG_WRN("continuation_number exceeds the valid range %d", conn->le.continuation_number);

Check warning on line 2647 in subsys/bluetooth/host/hci_core.c

View workflow job for this annotation

GitHub Actions / Run compliance checks on patch series (PR)

LONG_LINE

subsys/bluetooth/host/hci_core.c:2647 line length of 112 exceeds 100 columns
}
if (!IN_RANGE(conn->le.timeout, BT_HCI_LE_SUPERVISON_TIMEOUT_MIN, BT_HCI_LE_SUPERVISON_TIMEOUT_MAX)) {

Check warning on line 2649 in subsys/bluetooth/host/hci_core.c

View workflow job for this annotation

GitHub Actions / Run compliance checks on patch series (PR)

LONG_LINE

subsys/bluetooth/host/hci_core.c:2649 line length of 118 exceeds 100 columns
LOG_WRN("supervision_timeout exceeds the valid range 0x%04x", conn->le.timeout);
}

Check notice on line 2651 in subsys/bluetooth/host/hci_core.c

View workflow job for this annotation

GitHub Actions / Run compliance checks on patch series (PR)

You may want to run clang-format on this change

subsys/bluetooth/host/hci_core.c:2651 - 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 (!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 (BT_HCI_LE_PERIPHERAL_LATENCY_MAX < conn->le.peripheral_latency) { - LOG_WRN("peripheral_latency exceeds the valid range 0x%04x", conn->le.peripheral_latency); + LOG_WRN("peripheral_latency exceeds the valid range 0x%04x", + conn->le.peripheral_latency); } if (BT_HCI_LE_CONTINUATION_NUM_MAX < conn->le.continuation_number) { - LOG_WRN("continuation_number exceeds the valid range %d", conn->le.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); + LOG_WRN("continuation_number exceeds the valid range %d", + conn->le.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

0 comments on commit 29a7722

Please sign in to comment.