Bluetooth: Controller: Fix missing conn update ind PDU validation
Fix missing validation of Connection Update Ind PDU. Ignore
invalid connection update parameters and force a silent
local connection termination.
Signed-off-by: Vinayak Kariappa Chettimada <vich@nordicsemi.no>
(cherry picked from commit 4b6d3f1e16ea8ccccbf85f1eaf7efd0caea11766)
Signed-off-by: Vinayak Kariappa Chettimada <vich@nordicsemi.no>
diff --git a/subsys/bluetooth/controller/ll_sw/ull_llcp_conn_upd.c b/subsys/bluetooth/controller/ll_sw/ull_llcp_conn_upd.c
index 381f5d3..4d20aee 100644
--- a/subsys/bluetooth/controller/ll_sw/ull_llcp_conn_upd.c
+++ b/subsys/bluetooth/controller/ll_sw/ull_llcp_conn_upd.c
@@ -196,6 +196,22 @@
}
#endif /* CONFIG_BT_CTLR_CONN_PARAM_REQ */
+static bool cu_check_conn_ind_parameters(struct ll_conn *conn, struct proc_ctx *ctx)
+{
+ const uint16_t interval_max = ctx->data.cu.interval_max; /* unit 1.25ms */
+ const uint16_t timeout = ctx->data.cu.timeout; /* unit 10ms */
+ const uint16_t latency = ctx->data.cu.latency;
+
+ /* Valid conn_update_ind parameters */
+ return (interval_max >= CONN_INTERVAL_MIN(conn)) &&
+ (interval_max <= CONN_UPDATE_CONN_INTV_4SEC) &&
+ (latency <= CONN_UPDATE_LATENCY_MAX) &&
+ (timeout >= CONN_UPDATE_TIMEOUT_100MS) &&
+ (timeout <= CONN_UPDATE_TIMEOUT_32SEC) &&
+ ((timeout * 4U) > /* *4U re. conn events is equivalent to *2U re. ms */
+ ((latency + 1U) * interval_max));
+}
+
static void cu_prepare_update_ind(struct ll_conn *conn, struct proc_ctx *ctx)
{
ctx->data.cu.win_size = 1U;
@@ -585,8 +601,20 @@
switch (evt) {
case LP_CU_EVT_CONN_UPDATE_IND:
llcp_pdu_decode_conn_update_ind(ctx, param);
+
+ /* Invalid PDU, mark the connection for termination */
+ if (!cu_check_conn_ind_parameters(conn, ctx)) {
+ llcp_rr_set_incompat(conn, INCOMPAT_NO_COLLISION);
+ conn->llcp_terminate.reason_final = BT_HCI_ERR_INVALID_LL_PARAM;
+ lp_cu_complete(conn, ctx);
+ break;
+ }
+
+ llcp_rr_set_incompat(conn, INCOMPAT_RESERVED);
+
/* Keep RX node to use for NTF */
llcp_rx_node_retain(ctx);
+
ctx->state = LP_CU_STATE_WAIT_INSTANT;
break;
case LP_CU_EVT_UNKNOWN:
@@ -1206,19 +1234,27 @@
case BT_HCI_ROLE_PERIPHERAL:
llcp_pdu_decode_conn_update_ind(ctx, param);
- if (is_instant_not_passed(ctx->data.cu.instant,
- ull_conn_event_counter(conn))) {
+ /* Valid PDU */
+ if (cu_check_conn_ind_parameters(conn, ctx)) {
+ if (is_instant_not_passed(ctx->data.cu.instant,
+ ull_conn_event_counter(conn))) {
+ /* Keep RX node to use for NTF */
+ llcp_rx_node_retain(ctx);
- llcp_rx_node_retain(ctx);
+ ctx->state = RP_CU_STATE_WAIT_INSTANT;
- ctx->state = RP_CU_STATE_WAIT_INSTANT;
- /* In case we only just received it in time */
- rp_cu_check_instant(conn, ctx, evt, param);
- } else {
+ /* In case we only just received it in time */
+ rp_cu_check_instant(conn, ctx, evt, param);
+ break;
+ }
+
conn->llcp_terminate.reason_final = BT_HCI_ERR_INSTANT_PASSED;
- llcp_rr_complete(conn);
- ctx->state = RP_CU_STATE_IDLE;
+ } else {
+ conn->llcp_terminate.reason_final = BT_HCI_ERR_INVALID_LL_PARAM;
}
+
+ llcp_rr_complete(conn);
+ ctx->state = RP_CU_STATE_IDLE;
break;
default:
/* Unknown role */
diff --git a/tests/bluetooth/controller/ctrl_conn_update/src/main.c b/tests/bluetooth/controller/ctrl_conn_update/src/main.c
index 9e0b072..4871491 100644
--- a/tests/bluetooth/controller/ctrl_conn_update/src/main.c
+++ b/tests/bluetooth/controller/ctrl_conn_update/src/main.c
@@ -4593,6 +4593,128 @@
}
#endif
+/*
+ * Central-initiated Connection Update procedure.
+ * Peripheral receives invalid Connection Update parameters.
+ *
+ * +-----+ +-------+ +-----+
+ * | UT | | LL_P | | LT |
+ * +-----+ +-------+ +-----+
+ * | | |
+ * | | LL_CONNECTION_UPDATE_IND |
+ * | |<--------------------------|
+ * | | |
+ * ~~~~~~~~~~~~~~~~~~ TERMINATE CONNECTION ~~~~~~~~~~~~~~~~~
+ * | | |
+ */
+ZTEST(periph_rem_invalid, test_conn_update_periph_rem_invalid_param)
+{
+ uint16_t interval;
+
+ /* Role */
+ test_set_role(&conn, BT_HCI_ROLE_PERIPHERAL);
+
+ /* Connect */
+ ull_cp_state_set(&conn, ULL_CP_CONNECTED);
+
+ /* Prepare */
+ event_prepare(&conn);
+
+ /* Rx */
+ interval = conn_update_ind.interval;
+ conn_update_ind.interval = 0U;
+ conn_update_ind.instant = event_counter(&conn) + 6U;
+ lt_tx(LL_CONNECTION_UPDATE_IND, &conn, &conn_update_ind);
+
+ /* Done */
+ event_done(&conn);
+
+ /* Termination 'triggered' */
+ zassert_equal(conn.llcp_terminate.reason_final, BT_HCI_ERR_INVALID_LL_PARAM,
+ "Terminate reason %d", conn.llcp_terminate.reason_final);
+
+ /* Clear termination flag for subsequent test cycle */
+ conn.llcp_terminate.reason_final = 0;
+
+ /* Restore interval for other tests */
+ conn_update_ind.interval = interval;
+}
+
+#if defined(CONFIG_BT_CTLR_CONN_PARAM_REQ)
+/*
+ * Peripheral-initiated Connection Parameters Request procedure.
+ * Peripheral requests change in LE connection parameters, central’s Host accepts.
+ * Peripheral receives invalid Connection Update parameters.
+ *
+ * +-----+ +-------+ +-----+
+ * | UT | | LL_P | | LT |
+ * +-----+ +-------+ +-----+
+ * | | |
+ * | LE Connection Update | |
+ * |-------------------------->| |
+ * | | LL_CONNECTION_PARAM_REQ |
+ * | |-------------------------->|
+ * | | |
+ * | | LL_CONNECTION_UPDATE_IND |
+ * | |<--------------------------|
+ * | | |
+ * ~~~~~~~~~~~~~~~~~~ TERMINATE CONNECTION ~~~~~~~~~~~~~~~~~
+ * | | |
+ */
+ZTEST(periph_rem_invalid, test_conn_param_req_periph_rem_invalid_param)
+{
+ struct node_tx *tx;
+ uint16_t interval;
+ uint8_t err;
+
+ /* Role */
+ test_set_role(&conn, BT_HCI_ROLE_PERIPHERAL);
+
+ /* Connect */
+ ull_cp_state_set(&conn, ULL_CP_CONNECTED);
+
+ /* Initiate a Connection Parameter Request Procedure */
+ err = ull_cp_conn_update(&conn, INTVL_MIN, INTVL_MAX, LATENCY, TIMEOUT, NULL);
+ zassert_equal(err, BT_HCI_ERR_SUCCESS);
+
+ /* Prepare */
+ event_prepare(&conn);
+ conn_param_req.reference_conn_event_count = event_counter(&conn);
+
+ /* Tx Queue should have one LL Control PDU */
+ lt_rx(LL_CONNECTION_PARAM_REQ, &conn, &tx, &conn_param_req);
+ lt_rx_q_is_empty(&conn);
+
+ /* Done */
+ event_done(&conn);
+
+ /* Release Tx */
+ ull_cp_release_tx(&conn, tx);
+
+ /* Prepare */
+ event_prepare(&conn);
+
+ /* Rx */
+ interval = conn_update_ind.interval;
+ conn_update_ind.interval = 0U;
+ conn_update_ind.instant = event_counter(&conn) + 6U;
+ lt_tx(LL_CONNECTION_UPDATE_IND, &conn, &conn_update_ind);
+
+ /* Done */
+ event_done(&conn);
+
+ /* Termination 'triggered' */
+ zassert_equal(conn.llcp_terminate.reason_final, BT_HCI_ERR_INVALID_LL_PARAM,
+ "Terminate reason %d", conn.llcp_terminate.reason_final);
+
+ /* Clear termination flag for subsequent test cycle */
+ conn.llcp_terminate.reason_final = 0;
+
+ /* Restore interval for other tests */
+ conn_update_ind.interval = interval;
+}
+#endif /* CONFIG_BT_CTLR_CONN_PARAM_REQ */
+
#if defined(CONFIG_BT_CTLR_CONN_PARAM_REQ)
ZTEST_SUITE(central_loc, NULL, NULL, conn_update_setup, NULL, NULL);
ZTEST_SUITE(central_rem, NULL, NULL, conn_update_setup, NULL, NULL);
@@ -4604,3 +4726,5 @@
ZTEST_SUITE(periph_loc_no_param_req, NULL, NULL, conn_update_setup, NULL, NULL);
ZTEST_SUITE(periph_rem_no_param_req, NULL, NULL, conn_update_setup, NULL, NULL);
#endif /* CONFIG_BT_CTLR_CONN_PARAM_REQ */
+
+ZTEST_SUITE(periph_rem_invalid, NULL, NULL, conn_update_setup, NULL, NULL);
