Skip to content

Commit 996a1da

Browse files
cvinayakanangl
cvinayak
authored and
anangl
committed
[nrf fromtree] 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 <[email protected]> (cherry picked from commit 4b6d3f1) Signed-off-by: Vinayak Kariappa Chettimada <[email protected]>
1 parent 4f182bc commit 996a1da

File tree

2 files changed

+168
-9
lines changed

2 files changed

+168
-9
lines changed

‎subsys/bluetooth/controller/ll_sw/ull_llcp_conn_upd.c‎

Lines changed: 44 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -196,6 +196,22 @@ static bool cu_check_conn_parameters(struct ll_conn *conn, struct proc_ctx *ctx)
196196
}
197197
#endif /* CONFIG_BT_CTLR_CONN_PARAM_REQ */
198198

199+
static bool cu_check_conn_ind_parameters(struct ll_conn *conn, struct proc_ctx *ctx)
200+
{
201+
const uint16_t interval_max = ctx->data.cu.interval_max; /* unit 1.25ms */
202+
const uint16_t timeout = ctx->data.cu.timeout; /* unit 10ms */
203+
const uint16_t latency = ctx->data.cu.latency;
204+
205+
/* Valid conn_update_ind parameters */
206+
return (interval_max >= CONN_INTERVAL_MIN(conn)) &&
207+
(interval_max <= CONN_UPDATE_CONN_INTV_4SEC) &&
208+
(latency <= CONN_UPDATE_LATENCY_MAX) &&
209+
(timeout >= CONN_UPDATE_TIMEOUT_100MS) &&
210+
(timeout <= CONN_UPDATE_TIMEOUT_32SEC) &&
211+
((timeout * 4U) > /* *4U re. conn events is equivalent to *2U re. ms */
212+
((latency + 1U) * interval_max));
213+
}
214+
199215
static void cu_prepare_update_ind(struct ll_conn *conn, struct proc_ctx *ctx)
200216
{
201217
ctx->data.cu.win_size = 1U;
@@ -585,9 +601,20 @@ static void lp_cu_st_wait_rx_conn_update_ind(struct ll_conn *conn, struct proc_c
585601
switch (evt) {
586602
case LP_CU_EVT_CONN_UPDATE_IND:
587603
llcp_pdu_decode_conn_update_ind(ctx, param);
604+
605+
/* Invalid PDU, mark the connection for termination */
606+
if (!cu_check_conn_ind_parameters(conn, ctx)) {
607+
llcp_rr_set_incompat(conn, INCOMPAT_NO_COLLISION);
608+
conn->llcp_terminate.reason_final = BT_HCI_ERR_INVALID_LL_PARAM;
609+
lp_cu_complete(conn, ctx);
610+
break;
611+
}
612+
588613
llcp_rr_set_incompat(conn, INCOMPAT_RESERVED);
614+
589615
/* Keep RX node to use for NTF */
590616
llcp_rx_node_retain(ctx);
617+
591618
ctx->state = LP_CU_STATE_WAIT_INSTANT;
592619
break;
593620
case LP_CU_EVT_UNKNOWN:
@@ -1202,19 +1229,27 @@ static void rp_cu_st_wait_rx_conn_update_ind(struct ll_conn *conn, struct proc_c
12021229
case BT_HCI_ROLE_PERIPHERAL:
12031230
llcp_pdu_decode_conn_update_ind(ctx, param);
12041231

1205-
if (is_instant_not_passed(ctx->data.cu.instant,
1206-
ull_conn_event_counter(conn))) {
1232+
/* Valid PDU */
1233+
if (cu_check_conn_ind_parameters(conn, ctx)) {
1234+
if (is_instant_not_passed(ctx->data.cu.instant,
1235+
ull_conn_event_counter(conn))) {
1236+
/* Keep RX node to use for NTF */
1237+
llcp_rx_node_retain(ctx);
12071238

1208-
llcp_rx_node_retain(ctx);
1239+
ctx->state = RP_CU_STATE_WAIT_INSTANT;
1240+
1241+
/* In case we only just received it in time */
1242+
rp_cu_check_instant(conn, ctx, evt, param);
1243+
break;
1244+
}
12091245

1210-
ctx->state = RP_CU_STATE_WAIT_INSTANT;
1211-
/* In case we only just received it in time */
1212-
rp_cu_check_instant(conn, ctx, evt, param);
1213-
} else {
12141246
conn->llcp_terminate.reason_final = BT_HCI_ERR_INSTANT_PASSED;
1215-
llcp_rr_complete(conn);
1216-
ctx->state = RP_CU_STATE_IDLE;
1247+
} else {
1248+
conn->llcp_terminate.reason_final = BT_HCI_ERR_INVALID_LL_PARAM;
12171249
}
1250+
1251+
llcp_rr_complete(conn);
1252+
ctx->state = RP_CU_STATE_IDLE;
12181253
break;
12191254
default:
12201255
/* Unknown role */

‎tests/bluetooth/controller/ctrl_conn_update/src/main.c‎

Lines changed: 124 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4859,6 +4859,128 @@ ZTEST(periph_loc_no_param_req, test_conn_update_periph_loc_disallowed_no_param_r
48594859
}
48604860
#endif
48614861

4862+
/*
4863+
* Central-initiated Connection Update procedure.
4864+
* Peripheral receives invalid Connection Update parameters.
4865+
*
4866+
* +-----+ +-------+ +-----+
4867+
* | UT | | LL_P | | LT |
4868+
* +-----+ +-------+ +-----+
4869+
* | | |
4870+
* | | LL_CONNECTION_UPDATE_IND |
4871+
* | |<--------------------------|
4872+
* | | |
4873+
* ~~~~~~~~~~~~~~~~~~ TERMINATE CONNECTION ~~~~~~~~~~~~~~~~~
4874+
* | | |
4875+
*/
4876+
ZTEST(periph_rem_invalid, test_conn_update_periph_rem_invalid_param)
4877+
{
4878+
uint16_t interval;
4879+
4880+
/* Role */
4881+
test_set_role(&conn, BT_HCI_ROLE_PERIPHERAL);
4882+
4883+
/* Connect */
4884+
ull_cp_state_set(&conn, ULL_CP_CONNECTED);
4885+
4886+
/* Prepare */
4887+
event_prepare(&conn);
4888+
4889+
/* Rx */
4890+
interval = conn_update_ind.interval;
4891+
conn_update_ind.interval = 0U;
4892+
conn_update_ind.instant = event_counter(&conn) + 6U;
4893+
lt_tx(LL_CONNECTION_UPDATE_IND, &conn, &conn_update_ind);
4894+
4895+
/* Done */
4896+
event_done(&conn);
4897+
4898+
/* Termination 'triggered' */
4899+
zassert_equal(conn.llcp_terminate.reason_final, BT_HCI_ERR_INVALID_LL_PARAM,
4900+
"Terminate reason %d", conn.llcp_terminate.reason_final);
4901+
4902+
/* Clear termination flag for subsequent test cycle */
4903+
conn.llcp_terminate.reason_final = 0;
4904+
4905+
/* Restore interval for other tests */
4906+
conn_update_ind.interval = interval;
4907+
}
4908+
4909+
#if defined(CONFIG_BT_CTLR_CONN_PARAM_REQ)
4910+
/*
4911+
* Peripheral-initiated Connection Parameters Request procedure.
4912+
* Peripheral requests change in LE connection parameters, central’s Host accepts.
4913+
* Peripheral receives invalid Connection Update parameters.
4914+
*
4915+
* +-----+ +-------+ +-----+
4916+
* | UT | | LL_P | | LT |
4917+
* +-----+ +-------+ +-----+
4918+
* | | |
4919+
* | LE Connection Update | |
4920+
* |-------------------------->| |
4921+
* | | LL_CONNECTION_PARAM_REQ |
4922+
* | |-------------------------->|
4923+
* | | |
4924+
* | | LL_CONNECTION_UPDATE_IND |
4925+
* | |<--------------------------|
4926+
* | | |
4927+
* ~~~~~~~~~~~~~~~~~~ TERMINATE CONNECTION ~~~~~~~~~~~~~~~~~
4928+
* | | |
4929+
*/
4930+
ZTEST(periph_rem_invalid, test_conn_param_req_periph_rem_invalid_param)
4931+
{
4932+
struct node_tx *tx;
4933+
uint16_t interval;
4934+
uint8_t err;
4935+
4936+
/* Role */
4937+
test_set_role(&conn, BT_HCI_ROLE_PERIPHERAL);
4938+
4939+
/* Connect */
4940+
ull_cp_state_set(&conn, ULL_CP_CONNECTED);
4941+
4942+
/* Initiate a Connection Parameter Request Procedure */
4943+
err = ull_cp_conn_update(&conn, INTVL_MIN, INTVL_MAX, LATENCY, TIMEOUT, NULL);
4944+
zassert_equal(err, BT_HCI_ERR_SUCCESS);
4945+
4946+
/* Prepare */
4947+
event_prepare(&conn);
4948+
conn_param_req.reference_conn_event_count = event_counter(&conn);
4949+
4950+
/* Tx Queue should have one LL Control PDU */
4951+
lt_rx(LL_CONNECTION_PARAM_REQ, &conn, &tx, &conn_param_req);
4952+
lt_rx_q_is_empty(&conn);
4953+
4954+
/* Done */
4955+
event_done(&conn);
4956+
4957+
/* Release Tx */
4958+
ull_cp_release_tx(&conn, tx);
4959+
4960+
/* Prepare */
4961+
event_prepare(&conn);
4962+
4963+
/* Rx */
4964+
interval = conn_update_ind.interval;
4965+
conn_update_ind.interval = 0U;
4966+
conn_update_ind.instant = event_counter(&conn) + 6U;
4967+
lt_tx(LL_CONNECTION_UPDATE_IND, &conn, &conn_update_ind);
4968+
4969+
/* Done */
4970+
event_done(&conn);
4971+
4972+
/* Termination 'triggered' */
4973+
zassert_equal(conn.llcp_terminate.reason_final, BT_HCI_ERR_INVALID_LL_PARAM,
4974+
"Terminate reason %d", conn.llcp_terminate.reason_final);
4975+
4976+
/* Clear termination flag for subsequent test cycle */
4977+
conn.llcp_terminate.reason_final = 0;
4978+
4979+
/* Restore interval for other tests */
4980+
conn_update_ind.interval = interval;
4981+
}
4982+
#endif /* CONFIG_BT_CTLR_CONN_PARAM_REQ */
4983+
48624984
#if defined(CONFIG_BT_CTLR_CONN_PARAM_REQ)
48634985
ZTEST_SUITE(central_loc, NULL, NULL, conn_update_setup, NULL, NULL);
48644986
ZTEST_SUITE(central_rem, NULL, NULL, conn_update_setup, NULL, NULL);
@@ -4870,3 +4992,5 @@ ZTEST_SUITE(central_rem_no_param_req, NULL, NULL, conn_update_setup, NULL, NULL)
48704992
ZTEST_SUITE(periph_loc_no_param_req, NULL, NULL, conn_update_setup, NULL, NULL);
48714993
ZTEST_SUITE(periph_rem_no_param_req, NULL, NULL, conn_update_setup, NULL, NULL);
48724994
#endif /* CONFIG_BT_CTLR_CONN_PARAM_REQ */
4995+
4996+
ZTEST_SUITE(periph_rem_invalid, NULL, NULL, conn_update_setup, NULL, NULL);

0 commit comments

Comments
 (0)