Skip to content

Commit

Permalink
Bluetooth: Controller: Fix missing conn update ind PDU validation
Browse files Browse the repository at this point in the history 
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]>
  • Loading branch information
@cvinayak @carlescufi
cvinayak authored and carlescufi committed May 31, 2024
1 parent b50e1d5 commit 4b6d3f1
Show file tree
Hide file tree
Showing 2 changed files with 168 additions and 9 deletions.
53 changes: 44 additions & 9 deletions subsys/bluetooth/controller/ll_sw/ull_llcp_conn_upd.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -196,6 +196,22 @@ static bool cu_check_conn_parameters(struct ll_conn *conn, struct proc_ctx *ctx)
}
#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;
Expand Down Expand Up @@ -585,9 +601,20 @@ static void lp_cu_st_wait_rx_conn_update_ind(struct ll_conn *conn, struct proc_c
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:
Expand Down Expand Up @@ -1202,19 +1229,27 @@ static void rp_cu_st_wait_rx_conn_update_ind(struct ll_conn *conn, struct proc_c
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;

/* In case we only just received it in time */
rp_cu_check_instant(conn, ctx, evt, param);
break;
}

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 {
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 */
Expand Down
124 changes: 124 additions & 0 deletions tests/bluetooth/controller/ctrl_conn_update/src/main.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4859,6 +4859,128 @@ ZTEST(periph_loc_no_param_req, test_conn_update_periph_loc_disallowed_no_param_r
}
#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);
Expand All @@ -4870,3 +4992,5 @@ ZTEST_SUITE(central_rem_no_param_req, NULL, NULL, conn_update_setup, NULL, NULL)
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);

0 comments on commit 4b6d3f1

Please sign in to comment.